Author: markt
Date: Wed Aug 29 20:21:01 2012
New Revision: 1378699
URL: http://svn.apache.org/viewvc?rev=1378699&view=rev
Log:
Resolve a FIXME and expand unit tests to cover CRLF vs LF checking.
Modified:
tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
tomcat/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
Modified:
tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java?rev=1378699&r1=1378698&r2=1378699&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
Wed Aug 29 20:21:01 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
}
}
@@ -305,9 +305,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;
@@ -325,7 +324,10 @@ public class ChunkedInputFilter implemen
}
}
- pos++;
+ // Parsing the CRLF increments pos
+ if (!eol) {
+ pos++;
+ }
}
@@ -346,9 +348,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;
@@ -364,7 +378,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");
@@ -396,26 +412,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
@@ -495,9 +504,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/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java?rev=1378699&r1=1378698&r2=1378699&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java Wed Aug
29 20:21:01 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/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java?rev=1378699&r1=1378698&r2=1378699&view=diff
==============================================================================
---
tomcat/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
(original)
+++
tomcat/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
Wed Aug 29 20:21:01 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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
