Hi Am 14.01.2014 um 12:37 schrieb Alexander Klimetschek <[email protected]>:
> 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 Right > > 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. Yes, and see below for more on this > > The question is: > Is it ok to always redecode query strings (in addition to the parsing done by > the servlet container)? I would imagine that we would grab request.getQueryString() and parse and decode it (in two goes — we first have to find whether there is a _charset_ parameter) > > 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. For inside the Sling Engine, this is already taken care of by the Authentication Handlers actually getting a wrapped request object which hooks into the Sling Engine's parameter support. For outside the sling engine, this is not the case yet. I think, once we are in control of parsing the parameters, we don't let the servlet container do it. In fact, I could as another option, even imagine that the ParameterSupport is registered as a filter, which does not even allow the servlet container to do anything — at least for the Http Service inside of Sling. Regards Felix > 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
