Bug 53062 - Tomcat doesn't normalize absolute urls for redirect
Summary: Tomcat doesn't normalize absolute urls for redirect
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Servlet & JSP API (show other bugs)
Version: trunk
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-11 12:42 UTC by mgrigorov
Modified: 2012-08-14 22:21 UTC (History)
1 user (show)



Attachments
A demo app that demonstrates the problem. Issue a request to /serv1 and check the produced 'Location' response header (2.68 KB, application/x-compressed-tar)
2012-04-11 12:42 UTC, mgrigorov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description mgrigorov 2012-04-11 12:42:29 UTC
Created attachment 28582 [details]
A demo app that demonstrates the problem. Issue a request to /serv1 and check the produced 'Location' response header

Issuing a redirect like:

  response.sendRedirect("./serv2");

will lead to a response header like:

  Location:http://localhost:8080/./serv2

and this causes problems for some not that smart user agents like Internet Explorer and JMeter.

The problem has been reported few times in Apache Wicket's Jira.
Wicket works only with relative urls and lets the web container to make them absolute when a redirect is needed. But it seems only Tomcat produces absolute urls with '../' and/or './' inside and let the user agent to normalize them. Other web containers normalize the url at the server side and make user agents life easier.

See 
https://issues.apache.org/jira/browse/WICKET-2732
https://issues.apache.org/jira/browse/WICKET-4260
Comment 1 mgrigorov 2012-04-11 12:44:53 UTC
The demo application provides a workaround by using a custom HttpServletResponseWrapper that solves the problem but I believe this should be handled by Tomcat itself.
It uses the same approach as described in https://issues.apache.org/bugzilla/show_bug.cgi?id=51972
Comment 2 Mark Thomas 2012-05-09 20:29:21 UTC
The specification says relative URLs must be translated to "fully qualified URL". My understanding of that is that it means an absolute URL. The specification says nothing about normalizing or not normalizing the URL.

Given that Tomcat has worked this way for so long, I am surprised there haven't been more complaints given the statement that IE does not handle non-normalized redirects.

I'm not against normalizing them in principle but I want to take a look at the code to see what would be involved.
Comment 3 Mark Thomas 2012-05-09 20:59:58 UTC
Hmm. Interesting. RFC2396 says /./ and /../ are only special in relative URLs. It implies (but does not make explicitly clear) that /./ and /../ are to be treated literally in absolute URLs. That certainly isn't what any web server I am aware of does. They would get treated the same way they would in relative URLs and be normalized.

There is certainly an argument, based on removing ambiguity, to normalize URLs generated by sendRedirect and friends.
Comment 4 mgrigorov 2012-05-10 08:53:03 UTC
I have noticed that WebLogic (not sure which version though) also produce such absolute urls.
If you ask me IE users should suffer just for the reason they use this browser, but except IE+Tomcat_virtual_host combination we see that other user agents like JMeter are also confused by the existence of ./ and ../ in absolute urls.
Comment 5 Mark Thomas 2012-05-10 20:10:28 UTC
Normalization added to trunk and 7.0.x and will be included in 7.0.28 onwards.
Comment 6 Konstantin Preißer 2012-07-09 17:00:35 UTC
Hi,

it seems that the URL normalization which has been added to Tomcat 7.0.28 includes the querystring part of the URL in the normalization process.

I'm not 100% sure if the character '/' is allowed to appear unencoded in the query string part, but according to some sites which reference RFC 3986 [1], it is.

Although most commonly used URL-encoding methods (like java.net.URLEncoder.encode()) encode the '/' character as "%2F", it maybe possible that some applications use that char directly in a querystring, which is then given to response.sendRedirect().

Imaging a servlet available at URL

    http://localhost/Test/SomeServlet

calls

    response.sendRedirect("OtherServlet?someText=A/../B");

then the resulting HTTP 302 header will be:

    Location: http://localhost/Test/B

instead of

    Location: http://localhost/Test/OtherServlet?someText=A/../B

so the querystring part is unintentionally modified. Maybe this needs to be fixed?


[1] http://www.456bereastreet.com/archive/201008/what_characters_are_allowed_unencoded_in_query_strings/
Comment 7 Mark Thomas 2012-07-09 17:36:41 UTC
Looking at the code, that certainly looks as if it is the case. The query string (if any) should be excluded from the normalization process. I'll add some more unit tests to confirm this is happening and then make any fixes necessary.
Comment 8 Mark Thomas 2012-07-09 19:13:22 UTC
Fixed in trunk and 7.0.x and will be included in 7.0.30 onwards.

I think I have covered all the edge cases but if you spot an edge case you think still isn't correct, it should be easy to extend the unit tests to confirm this.
Comment 9 wanshoupu 2012-08-14 19:22:25 UTC
(In reply to comment #8)
> Fixed in trunk and 7.0.x and will be included in 7.0.30 onwards.
> 
> I think I have covered all the edge cases but if you spot an edge case you
> think still isn't correct, it should be easy to extend the unit tests to
> confirm this.

Thanks for the explanation and quick fix. May I get an estimated release date for 7.0.30 please? Our server is currently blocked on this issue. Many thanks!
Comment 10 Mark Thomas 2012-08-14 22:21:38 UTC
7.0.30 will be released when it is ready. As a volunteer organization, the ASF does not commit to release schedules.

What I can say is that I try to do a Tomcat 7 release every month. The first task is clearing the current bug back-log and I am currently working on that. Once that is complete the TCKs will need to be run (takes most of a working day) and then the release candidate needs to be built (few minutes). Once we have a release candidate there will need to be a release vote and that takes a minimum of 3 days. This, of course, assumes all goes well.

I would guess (based on my own schedule over the next few weeks) that 7.0.30 is at least 2-3 weeks away.

If you want 7.0.30 sooner, then providing patches to the open bugs that do not have them is the best way to help out.