Author: markt Date: Thu May 10 20:09:23 2012 New Revision: 1336870 URL: http://svn.apache.org/viewvc?rev=1336870&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53062 Normalize redirect URLs constructed from relative paths
Added: tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TesterMockRequest.java - copied unchanged from r1336868, tomcat/trunk/test/org/apache/catalina/connector/TesterMockRequest.java Modified: tomcat/tc7.0.x/trunk/ (props changed) tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/CharChunk.java tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponse.java tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponsePerformance.java tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Propchange: tomcat/tc7.0.x/trunk/ ------------------------------------------------------------------------------ Merged /tomcat/trunk:r1336864,1336868 Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java?rev=1336870&r1=1336869&r2=1336870&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Response.java Thu May 10 20:09:23 2012 @@ -1672,7 +1672,7 @@ public class Response redirectURLCC.recycle(); // Add the scheme String scheme = request.getScheme(); - try { + try { redirectURLCC.append(scheme, 0, scheme.length()); redirectURLCC.append(':'); redirectURLCC.append(location, 0, location.length()); @@ -1731,6 +1731,8 @@ public class Response redirectURLCC.append('/'); } redirectURLCC.append(location, 0, location.length()); + + normalize(redirectURLCC); } catch (IOException e) { IllegalArgumentException iae = new IllegalArgumentException(location); @@ -1748,6 +1750,77 @@ public class Response } + /* + * Removes /./ and /../ sequences from absolute URLs. + * Code borrowed heavily from CoyoteAdapter.normalize() + */ + private void normalize(CharChunk cc) { + if (cc.endsWith("/.") || cc.endsWith("/..")) { + try { + cc.append('/'); + } catch (IOException e) { + throw new IllegalArgumentException(cc.toString(), e); + } + } + + char[] c = cc.getChars(); + int start = cc.getStart(); + int end = cc.getEnd(); + int index = 0; + int startIndex = 0; + + // Advance past the first three / characters (should place index just + // scheme://host[:port] + + for (int i = 0; i < 3; i++) { + startIndex = cc.indexOf('/', startIndex + 1); + } + + // Remove /./ + index = startIndex; + while (true) { + index = cc.indexOf("/./", 0, 3, index); + if (index < 0) { + break; + } + copyChars(c, start + index, start + index + 2, + end - start - index - 2); + end = end - 2; + cc.setEnd(end); + } + + // Remove /../ + index = startIndex; + int pos; + while (true) { + index = cc.indexOf("/../", 0, 4, index); + if (index < 0) { + break; + } + // Prevent from going outside our context + if (index == startIndex) { + throw new IllegalArgumentException(); + } + int index2 = -1; + for (pos = start + index - 1; (pos >= 0) && (index2 < 0); pos --) { + if (c[pos] == (byte) '/') { + index2 = pos; + } + } + copyChars(c, start + index2, start + index + 3, + end - start - index - 3); + end = end + index2 - index - 3; + cc.setEnd(end); + index = index2; + } + } + + private void copyChars(char[] c, int dest, int src, int len) { + for (int pos = 0; pos < len; pos++) { + c[pos + dest] = c[pos + src]; + } + } + /** * Determine if an absolute URL has a path component Modified: tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/CharChunk.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/CharChunk.java?rev=1336870&r1=1336869&r2=1336870&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/CharChunk.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/buf/CharChunk.java Thu May 10 20:09:23 2012 @@ -670,6 +670,25 @@ public final class CharChunk implements } + /** + * Returns true if the message bytes end with the specified string. + * @param s the string + */ + public boolean endsWith(String s) { + char[] c = buff; + int len = s.length(); + if (c == null || len > end-start) { + return false; + } + int off = end - len; + for (int i = 0; i < len; i++) { + if (c[off++] != s.charAt(i)) { + return false; + } + } + return true; + } + // -------------------- Hash code -------------------- // normal hash. Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponse.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponse.java?rev=1336870&r1=1336869&r2=1336870&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponse.java (original) +++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponse.java Thu May 10 20:09:23 2012 @@ -33,6 +33,7 @@ import javax.servlet.http.HttpSession; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import org.junit.Assert; import org.junit.Test; import org.apache.catalina.Context; @@ -161,6 +162,66 @@ public class TestResponse extends Tomcat assertEquals("OK", bc.toString()); } + + @Test + public void testBug53062a() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + String result = resp.toAbsolute("./bar.html"); + + Assert.assertEquals("http://localhost:8080/level1/level2/bar.html", + result); + } + + + @Test + public void testBug53062b() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + String result = resp.toAbsolute("."); + + Assert.assertEquals("http://localhost:8080/level1/level2/", result); + } + + + @Test + public void testBug53062c() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + String result = resp.toAbsolute(".."); + + Assert.assertEquals("http://localhost:8080/level1/", result); + } + + + @Test + public void testBug53062d() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + String result = resp.toAbsolute(".././.."); + + Assert.assertEquals("http://localhost:8080/", result); + } + + + @Test(expected=IllegalArgumentException.class) + public void testBug53062e() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + resp.toAbsolute("../../.."); + } + + private static final class Bug52811Servlet extends HttpServlet { private static final long serialVersionUID = 1L; Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponsePerformance.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponsePerformance.java?rev=1336870&r1=1336869&r2=1336870&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponsePerformance.java (original) +++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/connector/TestResponsePerformance.java Thu May 10 20:09:23 2012 @@ -24,7 +24,7 @@ import org.junit.Test; public class TestResponsePerformance { @Test public void testToAbsolutePerformance() throws Exception { - Request req = new TesterToAbsoluteRequest(); + Request req = new TesterMockRequest(); Response resp = new Response(); resp.setRequest(req); @@ -36,7 +36,8 @@ public class TestResponsePerformance { start = System.currentTimeMillis(); for (int i = 0; i < 100000; i++) { - URI base = URI.create("http://localhost:8080/foo.html"); + URI base = URI.create( + "http://localhost:8080/level1/level2/foo.html"); base.resolve(URI.create("bar.html")).toASCIIString(); } long uri = System.currentTimeMillis() - start; @@ -45,28 +46,4 @@ public class TestResponsePerformance { "ms, Using URI: " + uri + "ms"); assertTrue(homebrew < uri); } - - - private static class TesterToAbsoluteRequest extends Request { - - @Override - public String getScheme() { - return "http"; - } - - @Override - public String getServerName() { - return "localhost"; - } - - @Override - public int getServerPort() { - return 8080; - } - - @Override - public String getDecodedRequestURI() { - return "/foo.html"; - } - } } Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1336870&r1=1336869&r2=1336870&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Thu May 10 20:09:23 2012 @@ -104,6 +104,10 @@ Ensure that <code>maxParameterCount</code> applies to multi-part requests handled via the Servlet 3 file upload API. (markt) </add> + <fix> + <bug>53062</bug>: When constructing absolute URLs for redirects from + relative URLs ensure that the resulting URLs are normalized. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org