Author: markt Date: Mon Jul 9 19:11:54 2012 New Revision: 1359342 URL: http://svn.apache.org/viewvc?rev=1359342&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53062 Correctly handle case where redirect URL includes a query string (with test cases)
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:r1359340 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=1359342&r1=1359341&r2=1359342&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 Mon Jul 9 19:11:54 2012 @@ -1752,6 +1752,18 @@ 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); + } + if (cc.endsWith("/.") || cc.endsWith("/..")) { try { cc.append('/'); @@ -1810,6 +1822,15 @@ public class Response cc.setEnd(end); index = index2; } + + // Add the query string (if present) back in + if (queryCC != null) { + try { + cc.append(queryCC, 0, queryCC.length); + } catch (IOException ioe) { + throw new IllegalArgumentException(ioe); + } + } } private void copyChars(char[] c, int dest, int src, int len) { 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=1359342&r1=1359341&r2=1359342&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 Mon Jul 9 19:11:54 2012 @@ -222,6 +222,88 @@ public class TestResponse extends Tomcat } + @Test + public void testBug53062f() 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 testBug53062g() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + String result = resp.toAbsolute("bar.html?x=/../"); + + Assert.assertEquals( + "http://localhost:8080/level1/level2/bar.html?x=/../", result); + } + + + @Test + public void testBug53062h() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + String result = resp.toAbsolute("bar.html?x=/../../"); + + Assert.assertEquals( + "http://localhost:8080/level1/level2/bar.html?x=/../../", + result); + } + + + @Test + public void testBug53062i() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + String result = resp.toAbsolute("./.?x=/../../"); + + Assert.assertEquals( + "http://localhost:8080/level1/level2/?x=/../../", + result); + } + + + @Test + public void testBug53062j() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + String result = resp.toAbsolute("./..?x=/../../"); + + Assert.assertEquals( + "http://localhost:8080/level1/?x=/../../", + result); + } + + + @Test + public void testBug53062k() throws Exception { + Request req = new TesterMockRequest(); + Response resp = new Response(); + resp.setRequest(req); + + String result = resp.toAbsolute("./..?x=/../.."); + + Assert.assertEquals( + "http://localhost:8080/level1/?x=/../..", + 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=1359342&r1=1359341&r2=1359342&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Mon Jul 9 19:11:54 2012 @@ -66,6 +66,11 @@ <bug>53498</bug>: Fix atomicity bugs in use of concurrent collections. Based on a patch by Yu Lin. (markt) </fix> + <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) + </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