On 13.01.2014, at 21:17, Felix Meschberger <[email protected]> wrote:
> Having said this, I could imagine taking a hybrid approach: > > (1) For url-encoded POST request, check whether InputStream is available > (1a) if available: > - decode query string > - decode input stream > (1b) if not available > - take all parameters from servlet container > (2) For form-data POST request > - decode query string > - decode POST input (as done today) Sounds good - some additions: The GET case is missing: (0) GET request - decode query string And I think in a normal Sling setup, (1b) should never be the case, as nobody else should read the input stream beforehand (or does the OSGi http service allow for earlier "hooks"?). So that's ok. The question is: Is it ok to always redecode query strings (in addition to the parsing done by the servlet container)? For POST bodies, it can happen only once, as done for form-data today already (afaics), by carefully not triggering the servlet container parsing with the first call to getParameter() etc. If redecoding is not ok, and should only happen when a servlet needs it, we probably have to handle authentication handlers separately from once the servlet has been selected and the per-servlet-config is clear. This means a mixed approach, for which I see two options: (I) avoid for GET only - do (1) and (2) early on already (it has to be read by Sling anyway) - but (0) (the GET query redecoding) only happens later, so the request parameter map must be updated once the servlet config says "order params" (II) avoid always - (1) and (2) will not redecode the query string, but keep decoding the input stream (with order) and remember that part - (0) GET query redecoding happens later - later (1) and (2) will be done again, by redecoding the query string and merging it with the existing decoded input stream This minimizes performance impact (if implemented efficiently), but will give you no full order for a POST with a query and a body in the early stage; it's very rare, so nothing we need to optimize for, and authentication handlers would never care. IMO it is important to not add a performance penalty for _all_ existing servlets. Cheers, Alex
