Bug 51972 - sendRedirect does not support protocol relative URLs
sendRedirect does not support protocol relative URLs
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Catalina
7.0.22
All All
: P2 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2011-10-05 21:31 UTC by Heikki Vesalainen
Modified: 2011-10-10 12:22 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Heikki Vesalainen 2011-10-05 21:31:50 UTC
The following URL is a protocol relative URL: "//server.com:8080/foo/bar/kala.html"
where as this is server relative: "/foo/bar/kala.html"

org/apache/catalina/connector/Response.java breaks on protocol relative URLs in that toAbsolute(String) expects everything that begins with a '/' to be server relative. toAbsolute adds the protocol, server and port to the URL, resulting in URLs like http://foo.com:80//server.com:8080/foo/bar/kala.html (instead of the correct http://server.com:8080/foo/bar/kala.html)
Comment 1 Mark Thomas 2011-10-05 22:12:49 UTC
The JavaDoc for sendRedirect requires that anything that starts with '/' is treated as server relative. I'm not sure if that is what was intended but it is certainly what the spec currently requires. I don't see any reason not to allow protocol relative URLs. I'll see if I can get some clarification from the Servlet EG.
Comment 2 Mark Thomas 2011-10-07 22:31:07 UTC
Discussion on this topic within the Servlet EG can be followed here:
http://java.net/jira/browse/SERVLET_SPEC-27

Given that I am in favour of supporting protocol relative URLs, I will look at adding this support to Tomcat 7 ready for the next release.
Comment 3 Heikki Vesalainen 2011-10-08 08:57:00 UTC
I would also suggest using the java.net.URI to resolve things in place of any home-brewn code.

private String toAbsolute(String href) {
  URI base = URI.create(requestURI); // original request URI, including protocol
  return base.resolve(URI.create(href)).toASCIIString();
}

And not just in this case, but all over tomcat where URIs are handled.
Comment 4 Mark Thomas 2011-10-10 11:56:08 UTC
The home-brew code is used for a good reason - performance. It is currently ~2x faster than the alternative proposed.
Comment 5 Heikki Vesalainen 2011-10-10 12:13:11 UTC
100% difference sounds dramatic, but how many percent of the total response time is spent in this code? How bad would it make things if the slower (but more reliable) version was used? Is it like the home-brewn code is 0,1% of total time and the URI takes 0,2% of total time? That would sound trivial and not worth the optimization.
Comment 6 Mark Thomas 2011-10-10 12:22:04 UTC
The amount of time spend in the sendRedirect code is likely to be only a small proportion of the overall request time but there are lots of places where URIs are manipulated in the Tomcat codebase. Switching all of them to URI is very likely to impact performance.

If you want to argue for this change then you'll need to present some hard numbers to the dev list (that cover both request processing time and GC) that show that there is no noticeable impact in making this change.
Comment 7 Mark Thomas 2011-10-10 12:22:43 UTC
This has been fixed in trunk and 7.0.x and will be included in 7.0.23 onwards.