Parameters of nice URL's pages with 'sensitive' characters

• Key: WICKET-40
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Critical
• Assignee: Jean-Baptiste Quenot
• Reporter: Jan Bareš
• Component/s: wicket
• Affects Version/s: 1.2.3
• Fix Version/s: 1.3.0-beta2

Description

Wicket uses HttpServletRequest.getPathInfo() to get the the URL. The returned string is already URL decoded, so when the request parameter pair contains %2F, it will be returned as '/', so the request pair will be broken (the same applies to other characters like '+' etc). This was cseen with Jetty 6 and Tomcat 5.5.
Wicket should use HttpServletRequest.getRequestURI() or getRequestURL() as this seems to return URL as it was passed to the server.

Comments

If you don't mind, here's another one for ya

[Johan Compagner]

this should be rewritten anyway.
Now it is a mess. We do encoding everywhere some & -> $amp; and then urlencode on params.
We should remove those everywhere. And use Request.encodeUrl or something like that
to really encode the url correctly at once.

[Jean-Baptiste Quenot]

Jan, can you please test it again after updating your SVN working copy. If you can still reproduce the problem, can you please be more specific and describe your usecase in details? Providing a quickstart, unit test or a patch can be useful. Thanks in advance!

[Jan Bareš]

I don't have the possibility to test fresh SVN copy now, but I tried on wicket-examples:
http://www.wicket-library.com/wicket-examples/niceurl/path/to/page2/param2/sla%2Fsh/param1/sss

The param2 should display "sla/sh" but instead wicket crashes.

[Jean-Baptiste Quenot]

What about this:

http://wicketstuff.org/wicket13/niceurl/path/to/page2/param2/sla%3Ash/param1/sss

Please use wicketstuff.org from now on for getting access to a running Wicket system.

[Jean-Baptiste Quenot]

I think we can close the issue now, I'm sure it has been fixed by WICKET-358

[Jan Bareš]

Thanks, I tried, but the problem still persist, please mind the %2F, which is '/' character, so the result should be "sla/sh". The ":" doesn't clash with parameters, but "/" does.

http://wicketstuff.org/wicket13/niceurl/path/to/page2/param2/sla%2Fsh/param1/sss

[Jean-Baptiste Quenot]

Using a slash in one of the parameter's key or value is another story because the URI is already decoded by the servlet container or web server. So it leads to unmatched key pair error because request.getURI() already returns a decoded URI like
/niceurl/path/to/page2/param2/sla/sh/param1/sss

You have to encode the encoded slash, like this:

http://localhost:8080/wicket-examples/niceurl/path/to/page2/param2/sla%252Fsh/param1/sss

On the older wicket snapshost, triple encode seems to be needed:

http://www.wicket-library.com/wicket-examples/niceurl/path/to/page2/param2/sla%25252Fsh/param1/sss

In other words, the servlet API does not offer a facility for parsing query parameters in this form using slashes, only using the query string beginning with "?" so you'd better avoid encoding slashes in the bookmarkable page parameters. Wicket does its best to parse the slashes-delimited query parameters, but it's far from being perfect.

[Jan Bareš]

Thanks for the explanation, you can close the issue.

[Jean-Baptiste Quenot]

Mmm actually, looking at the very description of this issue, you are right:

The Javadoc for getRequestURI() states that the URI is *not* decoded:
http://java.sun.com/j2ee/sdk_1.3/techdocs/api/javax/servlet/http/HttpServletRequest.html#getRequestURI()

Whereas it is for getPathInfo(). So we may want to take a look at this issue, indeed.

[Jean-Baptiste Quenot]

So the culprit must be:


In ServletWebRequest I find:

public String getPath()
{
return ((WebApplication)Application.get()).getWicketFilter().getRelativePath(
httpServletRequest);
}

And in WicketFilter:

public String getRelativePath(HttpServletRequest request) {
String path = request.getServletPath();
                ...

So WicketFilter is the culprit, as getServletPath() decodes the URL, see:
http://java.sun.com/j2ee/sdk_1.3/techdocs/api/javax/servlet/http/HttpServletRequest.html#getServletPath()

[Jean-Baptiste Quenot]

The attached patch changes the way WicketFilter computes the relative URL by avoiding by any means calling getServletPath() or getPathInfo(), only using getRequestURI(), getContextPath() and getServletPath().

There is also a temporary patch to wicket-examples to be able to reproduce the issue. Go to http://localhost:8080/wicket-examples/helloservlet/ for the servlet case and http://localhost:8080/wicket-examples/helloworld/ for the filter case.

[Jean-Baptiste Quenot]

I also tested this patch successfully with:

* "/wicket-examples" servlet context path and /* servlet mapping
* "/wicket-examples" servlet context path and /* filter mapping
* "" servlet context path and /* servlet mapping
* "" servlet context path and /* filter mapping
* "" servlet context path and /helloservlet/* servlet mapping
* "" servlet context path and /helloworld/* filter mapping

[Jean-Baptiste Quenot]

Just added some httpunit-based unit tests in wicket-examples, see revision 534932. Just need to apply the patch to WicketFilter now and rename bugTestXXX() methods to testXXX()

[Alastair Maw]

I can see what you're trying to achieve with this, but it's breaking some unit tests in wicket-examples. I'm looking into fixing this right now.

[Jean-Baptiste Quenot]

WicketFilter changed, and tests fixed.