Hi

Am 14.01.2014 um 18:44 schrieb Alexander Klimetschek <[email protected]>:

> I forgot 2 important points ;) (see below for responses to the existing 
> discussion)
> 
> (1)
> 
> Just using an ordered map instead of a hash map doesn't solve the problem, as 
> multi-value parameters still get merged and their invidual position lost. For 
> example, an URL like this (which is similar to my case):
> 
>    /do?move=10steps&turnLeft=45deg&move=20steps
> 
> Would become this ordered map:
>    move => 10steps,20steps
>    turnLeft => 45deg
> 
> but I really want to get the original order.
> 
> This could be easily solved by extending Sling's RequestParameterMap [0], 
> e.g. with a method returning an key-value iterator:
> 
>    Iterator<KeyValuePair> getParameters()

Well, that is a bit esoteric IMHO and I would not go as far as this.

> 
> (2)
> 
> OTOH, I notice that the code I am looking at does its own parameter parsing 
> anyway, and so basically requires the query string as input, even if it is a 
> POST - for that it currently does request.getReader() and converts it into a 
> string in a filter. It doesn't use any request parameter API. Changing it 
> might lead to issues, I am not sure if it uses the exact same parsing logic 
> that servlet engines are supposed to do…


> 
> So another solution would be to allow access to the request body as string 
> even if already parsed for parameters. I guess Sling doesn't store it, so 
> adding this might give a memory usage penalty by storing that possibly long 
> string (again, because you'd have to always do it before you even know if a 
> servlet asks for it). Not sure…

In the case of a multipart/form-data the input is in general not a string and 
can get quite big, particularly if files are involved.

I am also not sure about the benefit of this and the use case.

Regards
Felix


> 
> On 14.01.2014, at 16:27, Felix Meschberger <[email protected]> wrote:
> 
>>> 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)
> 
> Right, the _charset_ needs to be checked first.
> 
>> 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.
> 
> Good.
> 
>> For outside the sling engine, this is not the case yet.
> 
> I don't care :)
> 
>> I think, once we are in control of parsing the parameters, we don't let the 
>> servlet container do it.
> 
> I was under the assumption that servlet containers will parse the query 
> string in any case and only do POST body parsing on demand (i.e. when a 
> getParameter etc. gets called). But that was probably wrong, Jetty for 
> example does it lazily [1].
> 
> This simplifies it a lot and we can always ensure the right order for all 
> servlets in sling without sacrifying performance.
> 
>> 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.
> 
> I would not do it as a filter but as part of the sling engine i.e. the core 
> sling servlet request impl which wraps the original request anyway. Sling 
> already does decoding to some degree, we just improve it with keeping the 
> order.
> 
> We would need to update theAPI docs then to say that the 
> getRequestParameterMap() (sling specific) and getParameterMap() (from servlet 
> api) return an ordered map.
> 
> [0] 
> http://sling.apache.org/apidocs/sling5/org/apache/sling/api/request/RequestParameterMap.html
> [1] 
> http://download.eclipse.org/jetty/stable-9/xref/org/eclipse/jetty/server/Request.html#244
> 
> Cheers,
> Alex
> 

Reply via email to