Author: markt Date: Wed Jul 11 08:27:31 2012 New Revision: 1360060 URL: http://svn.apache.org/viewvc?rev=1360060&view=rev Log: Handle fragments as well as query strings in normalisation of redirects
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/test/org/apache/catalina/connector/TestResponse.java tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Propchange: tomcat/tc7.0.x/trunk/ ------------------------------------------------------------------------------ Merged /tomcat/trunk:r1360059 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=1360060&r1=1360059&r2=1360060&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 Wed Jul 11 08:27:31 2012 @@ -28,6 +28,7 @@ import java.security.PrivilegedActionExc import java.security.PrivilegedExceptionAction; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Enumeration; import java.util.List; @@ -1752,16 +1753,17 @@ public class Response * Code borrowed heavily from CoyoteAdapter.normalize() */ private void normalize(CharChunk cc) { - // Strip query string first (doing it this way makes the logic a lot - // simpler) - int query = cc.indexOf('?'); - char[] queryCC = null; - if (query > -1) { - queryCC = new char[cc.getEnd() - query]; - for (int i = query; i < cc.getEnd(); i++) { - queryCC[i - query] = cc.charAt(i); - } - cc.setEnd(query); + // Strip query string and/or fragment first as doing it this way makes + // the normalization logic a lot simpler + int truncate = cc.indexOf('?'); + if (truncate == -1) { + truncate = cc.indexOf('#'); + } + char[] truncateCC = null; + if (truncate > -1) { + truncateCC = Arrays.copyOfRange(cc.getBuffer(), + cc.getStart() + truncate, cc.getEnd()); + cc.setEnd(cc.getStart() + truncate); } if (cc.endsWith("/.") || cc.endsWith("/..")) { @@ -1824,9 +1826,9 @@ public class Response } // Add the query string (if present) back in - if (queryCC != null) { + if (truncateCC != null) { try { - cc.append(queryCC, 0, queryCC.length); + cc.append(truncateCC, 0, truncateCC.length); } catch (IOException ioe) { throw new IllegalArgumentException(ioe); } 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=1360060&r1=1360059&r2=1360060&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 Wed Jul 11 08:27:31 2012 @@ -271,8 +271,7 @@ public class TestResponse extends Tomcat String result = resp.toAbsolute("./.?x=/../../"); Assert.assertEquals( - "http://localhost:8080/level1/level2/?x=/../../", - result); + "http://localhost:8080/level1/level2/?x=/../../", result); } @@ -284,9 +283,7 @@ public class TestResponse extends Tomcat String result = resp.toAbsolute("./..?x=/../../"); - Assert.assertEquals( - "http://localhost:8080/level1/?x=/../../", - result); + Assert.assertEquals("http://localhost:8080/level1/?x=/../../", result); } @@ -304,6 +301,69 @@ public class TestResponse extends Tomcat } + @Test + public void testBug53062l() 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 testBug53062m() 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 testBug53062n() 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 testBug53062o() 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 testBug53062p() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + String result = resp.toAbsolute("./..#/../.."); + + Assert.assertEquals("http://localhost:8080/level1/#/../..", result); + } + + private static final class Bug52811Servlet extends HttpServlet { private static final long serialVersionUID = 1L; 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=1360060&r1=1360059&r2=1360060&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Jul 11 08:27:31 2012 @@ -69,7 +69,7 @@ <fix> Correct a regression in the previous fix for <bug>53062</bug> that did not always correctly normalize redirect URLs when the redirect URL - included a query string component. (markt) + included a query string or fragment component. (markt) </fix> </changelog> </subsection> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org