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/08/29 22:26:31 UTC
svn commit: r1378702 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/coyote/http11/filters/ test/org/apache/catalina/startup/
test/org/apache/coyote/http11/filters/ webapps/docs/
Author: markt
Date: Wed Aug 29 20:26:30 2012
New Revision: 1378702
URL: http://svn.apache.org/viewvc?rev=1378702&view=rev
Log:
Resolve a FIXME and expand unit tests to cover CRLF vs LF checking.
Modified:
tomcat/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1378699
Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java?rev=1378702&r1=1378701&r2=1378702&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java Wed Aug 29 20:26:30 2012
@@ -144,7 +144,7 @@ public class ChunkedInputFilter implemen
if(needCRLFParse) {
needCRLFParse = false;
- parseCRLF();
+ parseCRLF(false);
}
if (remaining <= 0) {
@@ -179,7 +179,7 @@ public class ChunkedInputFilter implemen
//so we defer it to the next call BZ 11117
needCRLFParse = true;
} else {
- parseCRLF(); //parse the CRLF immediately
+ parseCRLF(false); //parse the CRLF immediately
}
}
@@ -303,9 +303,8 @@ public class ChunkedInputFilter implemen
return false;
}
- if (buf[pos] == Constants.CR) {
- // FIXME: Improve parsing to check for CRLF
- } else if (buf[pos] == Constants.LF) {
+ if (buf[pos] == Constants.CR || buf[pos] == Constants.LF) {
+ parseCRLF(false);
eol = true;
} else if (buf[pos] == Constants.SEMI_COLON) {
trailer = true;
@@ -323,7 +322,10 @@ public class ChunkedInputFilter implemen
}
}
- pos++;
+ // Parsing the CRLF increments pos
+ if (!eol) {
+ pos++;
+ }
}
@@ -344,9 +346,21 @@ public class ChunkedInputFilter implemen
/**
* Parse CRLF at end of chunk.
+ * @deprecated Use {@link #parseCRLF(boolean)}
*/
- protected boolean parseCRLF()
- throws IOException {
+ @Deprecated
+ protected boolean parseCRLF() throws IOException {
+ return parseCRLF(false);
+ }
+
+ /**
+ * Parse CRLF at end of chunk.
+ *
+ * @param tolerant Should tolerant parsing (LF and CRLF) be used? This
+ * is recommended (RFC2616, section 19.3) for message
+ * headers.
+ */
+ protected boolean parseCRLF(boolean tolerant) throws IOException {
boolean eol = false;
boolean crfound = false;
@@ -362,7 +376,9 @@ public class ChunkedInputFilter implemen
if (crfound) throw new IOException("Invalid CRLF, two CR characters encountered.");
crfound = true;
} else if (buf[pos] == Constants.LF) {
- if (!crfound) throw new IOException("Invalid CRLF, no CR character encountered.");
+ if (!tolerant && !crfound) {
+ throw new IOException("Invalid CRLF, no CR character encountered.");
+ }
eol = true;
} else {
throw new IOException("Invalid CRLF");
@@ -394,26 +410,19 @@ public class ChunkedInputFilter implemen
MimeHeaders headers = request.getMimeHeaders();
byte chr = 0;
- while (true) {
- // Read new bytes if needed
- if (pos >= lastValid) {
- if (readBytes() <0)
- throw new EOFException("Unexpected end of stream whilst reading trailer headers for chunked request");
- }
- chr = buf[pos];
-
- if ((chr == Constants.CR) || (chr == Constants.LF)) {
- if (chr == Constants.LF) {
- pos++;
- return false;
- }
- } else {
- break;
- }
+ // Read new bytes if needed
+ if (pos >= lastValid) {
+ if (readBytes() <0)
+ throw new EOFException("Unexpected end of stream whilst reading trailer headers for chunked request");
+ }
- pos++;
+ chr = buf[pos];
+ // CRLF terminates the request
+ if (chr == Constants.CR || chr == Constants.LF) {
+ parseCRLF(true);
+ return false;
}
// Mark the current buffer position
@@ -493,9 +502,8 @@ public class ChunkedInputFilter implemen
}
chr = buf[pos];
- if (chr == Constants.CR) {
- // Skip
- } else if (chr == Constants.LF) {
+ if (chr == Constants.CR || chr == Constants.LF) {
+ parseCRLF(true);
eol = true;
} else if (chr == Constants.SP) {
trailingHeaders.append(chr);
Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java?rev=1378702&r1=1378701&r2=1378702&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java Wed Aug 29 20:26:30 2012
@@ -201,7 +201,13 @@ public abstract class SimpleHttpClient {
line = readLine();
while (line != null) {
builder.append(line);
- line = readLine();
+ try {
+ line = readLine();
+ } catch (IOException ioe) {
+ // The server probably closed the connection due to an
+ // error
+ line = null;
+ }
}
}
}
Modified: tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java?rev=1378702&r1=1378701&r2=1378702&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java Wed Aug 29 20:26:30 2012
@@ -38,8 +38,52 @@ import org.apache.catalina.startup.Tomca
public class TestChunkedInputFilter extends TomcatBaseTest {
+ private static final String LF = "\n";
+
+ @Test
+ public void testChunkHeaderCRLF() throws Exception {
+ doTestChunkingCRLF(true, true, true, true, true);
+ }
+
+ @Test
+ public void testChunkHeaderLF() throws Exception {
+ doTestChunkingCRLF(false, true, true, true, false);
+ }
+
+ @Test
+ public void testChunkCRLF() throws Exception {
+ doTestChunkingCRLF(true, true, true, true, true);
+ }
+
+ @Test
+ public void testChunkLF() throws Exception {
+ doTestChunkingCRLF(true, false, true, true, false);
+ }
+
+ @Test
+ public void testTrailingHeadersCRLF() throws Exception {
+ doTestChunkingCRLF(true, true, true, true, true);
+ }
+
+ @Test
+ public void testTrailingHeadersLF() throws Exception {
+ doTestChunkingCRLF(true, true, false, true, true);
+ }
+
@Test
- public void testTrailingHeaders() throws Exception {
+ public void testEndCRLF() throws Exception {
+ doTestChunkingCRLF(true, true, true, true, true);
+ }
+
+ @Test
+ public void testEndLF() throws Exception {
+ doTestChunkingCRLF(true, true, true, false, false);
+ }
+
+ private void doTestChunkingCRLF(boolean chunkHeaderUsesCRLF,
+ boolean chunkUsesCRLF, boolean headerUsesCRLF,
+ boolean endUsesCRLF, boolean expectPass) throws Exception {
+
// Setup Tomcat instance
Tomcat tomcat = getTomcatInstance();
@@ -60,13 +104,14 @@ public class TestChunkedInputFilter exte
SimpleHttpClient.CRLF +
"Connection: close" + SimpleHttpClient.CRLF +
SimpleHttpClient.CRLF +
- "3" + SimpleHttpClient.CRLF +
- "a=0" + SimpleHttpClient.CRLF +
+ "3" + (chunkHeaderUsesCRLF ? SimpleHttpClient.CRLF : LF) +
+ "a=0" + (chunkUsesCRLF ? SimpleHttpClient.CRLF : LF) +
"4" + SimpleHttpClient.CRLF +
"&b=1" + SimpleHttpClient.CRLF +
"0" + SimpleHttpClient.CRLF +
- "x-trailer: Test", "TestTest0123456789abcdefghijABCDEFGHIJopqrstuvwxyz" + SimpleHttpClient.CRLF +
- SimpleHttpClient.CRLF };
+ "x-trailer: Test", "TestTest0123456789abcdefghijABCDEFGHIJopqrstuvwxyz" +
+ (headerUsesCRLF ? SimpleHttpClient.CRLF : LF)+
+ (endUsesCRLF ? SimpleHttpClient.CRLF : LF) };
TrailerClient client =
new TrailerClient(tomcat.getConnector().getLocalPort());
@@ -74,7 +119,13 @@ public class TestChunkedInputFilter exte
client.connect();
client.processRequest();
- assertEquals("null7TestTestTest0123456789abcdefghijABCDEFGHIJopqrstuvwxyz", client.getResponseBody());
+
+ if (expectPass) {
+ assertTrue(client.isResponse200());
+ assertEquals("null7TestTestTest0123456789abcdefghijABCDEFGHIJopqrstuvwxyz", client.getResponseBody());
+ } else {
+ assertTrue(client.getResponseLine(), client.isResponse500());
+ }
}
@Test
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=1378702&r1=1378701&r2=1378702&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Wed Aug 29 20:26:30 2012
@@ -214,6 +214,10 @@
<bug>53725</bug>: Fix possible corruption of GZIP'd output.
(markt/kkolinko)
</fix>
+ <fix>
+ Better parsing of line-terminators for requests using chunked encoding.
+ (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org