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

Reply via email to