You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2012/05/10 22:06:16 UTC
svn commit: r1336864 - in /tomcat/trunk:
java/org/apache/catalina/connector/Response.java
java/org/apache/tomcat/util/buf/CharChunk.java
test/org/apache/catalina/connector/TestResponse.java
test/org/apache/catalina/connector/TestResponsePerformance.java
Author: markt
Date: Thu May 10 20:06:15 2012
New Revision: 1336864
URL: http://svn.apache.org/viewvc?rev=1336864&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53062
Normalize redirect URLs constructed from relative paths
Modified:
tomcat/trunk/java/org/apache/catalina/connector/Response.java
tomcat/trunk/java/org/apache/tomcat/util/buf/CharChunk.java
tomcat/trunk/test/org/apache/catalina/connector/TestResponse.java
tomcat/trunk/test/org/apache/catalina/connector/TestResponsePerformance.java
Modified: tomcat/trunk/java/org/apache/catalina/connector/Response.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Response.java?rev=1336864&r1=1336863&r2=1336864&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Response.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Response.java Thu May 10 20:06:15 2012
@@ -1560,7 +1560,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());
@@ -1619,6 +1619,8 @@ public class Response
redirectURLCC.append('/');
}
redirectURLCC.append(location, 0, location.length());
+
+ normalize(redirectURLCC);
} catch (IOException e) {
IllegalArgumentException iae =
new IllegalArgumentException(location);
@@ -1636,6 +1638,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/trunk/java/org/apache/tomcat/util/buf/CharChunk.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/buf/CharChunk.java?rev=1336864&r1=1336863&r2=1336864&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/buf/CharChunk.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/buf/CharChunk.java Thu May 10 20:06:15 2012
@@ -557,6 +557,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/trunk/test/org/apache/catalina/connector/TestResponse.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/connector/TestResponse.java?rev=1336864&r1=1336863&r2=1336864&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/connector/TestResponse.java (original)
+++ tomcat/trunk/test/org/apache/catalina/connector/TestResponse.java Thu May 10 20:06:15 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/trunk/test/org/apache/catalina/connector/TestResponsePerformance.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/connector/TestResponsePerformance.java?rev=1336864&r1=1336863&r2=1336864&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/connector/TestResponsePerformance.java (original)
+++ tomcat/trunk/test/org/apache/catalina/connector/TestResponsePerformance.java Thu May 10 20:06:15 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";
- }
- }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org