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()
(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...
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
