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/30 15:12:14 UTC
svn commit: r1378921 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
test/org/apache/catalina/startup/SimpleHttpClient.java
test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
Author: markt
Date: Thu Aug 30 13:12:13 2012
New Revision: 1378921
URL: http://svn.apache.org/viewvc?rev=1378921&view=rev
Log:
More chunked encoding improvements
- Expand unit tests for chunked encoding
- Fix a parsing error at eol when multiple headers are present (regression in r1378702)
- Make parsing of terminating CRLF non-tolerant (RFC2616 only suggests to be tolerant of LF at the end of headers)
- Revert previous unnecessary change to SimpleHttpClient
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
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1378868,1378918
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=1378921&r1=1378920&r2=1378921&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 Thu Aug 30 13:12:13 2012
@@ -350,7 +350,8 @@ public class ChunkedInputFilter implemen
*/
@Deprecated
protected boolean parseCRLF() throws IOException {
- return parseCRLF(false);
+ parseCRLF(false);
+ return true;
}
/**
@@ -360,7 +361,7 @@ public class ChunkedInputFilter implemen
* is recommended (RFC2616, section 19.3) for message
* headers.
*/
- protected boolean parseCRLF(boolean tolerant) throws IOException {
+ protected void parseCRLF(boolean tolerant) throws IOException {
boolean eol = false;
boolean crfound = false;
@@ -387,9 +388,6 @@ public class ChunkedInputFilter implemen
pos++;
}
-
- return true;
-
}
@@ -421,7 +419,7 @@ public class ChunkedInputFilter implemen
// CRLF terminates the request
if (chr == Constants.CR || chr == Constants.LF) {
- parseCRLF(true);
+ parseCRLF(false);
return false;
}
@@ -512,8 +510,9 @@ public class ChunkedInputFilter implemen
lastSignificantChar = trailingHeaders.getEnd();
}
- pos++;
-
+ if (!eol) {
+ pos++;
+ }
}
// Checking the first character of the new line. If the character
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=1378921&r1=1378920&r2=1378921&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 Thu Aug 30 13:12:13 2012
@@ -201,13 +201,7 @@ public abstract class SimpleHttpClient {
line = readLine();
while (line != null) {
builder.append(line);
- try {
- line = readLine();
- } catch (IOException ioe) {
- // The server probably closed the connection due to an
- // error
- line = null;
- }
+ line = readLine();
}
}
}
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=1378921&r1=1378920&r2=1378921&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 Thu Aug 30 13:12:13 2012
@@ -42,47 +42,58 @@ public class TestChunkedInputFilter exte
@Test
public void testChunkHeaderCRLF() throws Exception {
- doTestChunkingCRLF(true, true, true, true, true);
+ doTestChunkingCRLF(true, true, true, true, true, true);
}
@Test
public void testChunkHeaderLF() throws Exception {
- doTestChunkingCRLF(false, true, true, true, false);
+ doTestChunkingCRLF(false, true, true, true, true, false);
}
@Test
public void testChunkCRLF() throws Exception {
- doTestChunkingCRLF(true, true, true, true, true);
+ doTestChunkingCRLF(true, true, true, true, true, true);
}
@Test
public void testChunkLF() throws Exception {
- doTestChunkingCRLF(true, false, true, true, false);
+ doTestChunkingCRLF(true, false, true, true, true, false);
}
@Test
- public void testTrailingHeadersCRLF() throws Exception {
- doTestChunkingCRLF(true, true, true, true, true);
+ public void testFirstTrailingHeadersCRLF() throws Exception {
+ doTestChunkingCRLF(true, true, true, true, true, true);
}
@Test
- public void testTrailingHeadersLF() throws Exception {
- doTestChunkingCRLF(true, true, false, true, true);
+ public void testFirstTrailingHeadersLF() throws Exception {
+ doTestChunkingCRLF(true, true, false, true, true, true);
+ }
+
+ @Test
+ public void testSecondTrailingHeadersCRLF() throws Exception {
+ doTestChunkingCRLF(true, true, true, true, true, true);
+ }
+
+ @Test
+ public void testSecondTrailingHeadersLF() throws Exception {
+ doTestChunkingCRLF(true, true, true, false, true, true);
}
@Test
public void testEndCRLF() throws Exception {
- doTestChunkingCRLF(true, true, true, true, true);
+ doTestChunkingCRLF(true, true, true, true, true, true);
}
@Test
public void testEndLF() throws Exception {
- doTestChunkingCRLF(true, true, true, false, false);
+ doTestChunkingCRLF(true, true, true, true, false, false);
}
private void doTestChunkingCRLF(boolean chunkHeaderUsesCRLF,
- boolean chunkUsesCRLF, boolean headerUsesCRLF,
- boolean endUsesCRLF, boolean expectPass) throws Exception {
+ boolean chunkUsesCRLF, boolean firstheaderUsesCRLF,
+ boolean secondheaderUsesCRLF, boolean endUsesCRLF,
+ boolean expectPass) throws Exception {
// Setup Tomcat instance
Tomcat tomcat = getTomcatInstance();
@@ -109,8 +120,10 @@ public class TestChunkedInputFilter exte
"4" + SimpleHttpClient.CRLF +
"&b=1" + SimpleHttpClient.CRLF +
"0" + SimpleHttpClient.CRLF +
- "x-trailer: Test", "TestTest0123456789abcdefghijABCDEFGHIJopqrstuvwxyz" +
- (headerUsesCRLF ? SimpleHttpClient.CRLF : LF)+
+ "x-trailer1: Test", "Value1" +
+ (firstheaderUsesCRLF ? SimpleHttpClient.CRLF : LF) +
+ "x-trailer2: TestValue2" +
+ (secondheaderUsesCRLF ? SimpleHttpClient.CRLF : LF) +
(endUsesCRLF ? SimpleHttpClient.CRLF : LF) };
TrailerClient client =
@@ -122,7 +135,8 @@ public class TestChunkedInputFilter exte
if (expectPass) {
assertTrue(client.isResponse200());
- assertEquals("null7TestTestTest0123456789abcdefghijABCDEFGHIJopqrstuvwxyz", client.getResponseBody());
+ assertEquals("nullnull7TestValue1TestValue2",
+ client.getResponseBody());
} else {
assertTrue(client.getResponseLine(), client.isResponse500());
}
@@ -206,7 +220,7 @@ public class TestChunkedInputFilter exte
client.connect();
client.processRequest();
- assertEquals("null7null", client.getResponseBody());
+ assertEquals("nullnull7nullnull", client.getResponseBody());
}
private static class EchoHeaderServlet extends HttpServlet {
@@ -217,12 +231,9 @@ public class TestChunkedInputFilter exte
throws ServletException, IOException {
resp.setContentType("text/plain");
PrintWriter pw = resp.getWriter();
- // Header not visible yet, body not processed
- String value = req.getHeader("x-trailer");
- if (value == null) {
- value = "null";
- }
- pw.write(value);
+ // Headers not visible yet, body not processed
+ dumpHeader("x-trailer1", req, pw);
+ dumpHeader("x-trailer2", req, pw);
// Read the body - quick and dirty
InputStream is = req.getInputStream();
@@ -233,8 +244,14 @@ public class TestChunkedInputFilter exte
pw.write(Integer.valueOf(count).toString());
- // Header should be visible now
- value = req.getHeader("x-trailer");
+ // Headers should be visible now
+ dumpHeader("x-trailer1", req, pw);
+ dumpHeader("x-trailer2", req, pw);
+ }
+
+ private void dumpHeader(String headerName, HttpServletRequest req,
+ PrintWriter pw) {
+ String value = req.getHeader(headerName);
if (value == null) {
value = "null";
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1378921 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
test/org/apache/catalina/startup/SimpleHttpClient.java test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
Posted by Mark Thomas <ma...@apache.org>.
On 30/08/2012 14:12, markt@apache.org wrote:
> Author: markt
> Date: Thu Aug 30 13:12:13 2012
> New Revision: 1378921
>
> URL: http://svn.apache.org/viewvc?rev=1378921&view=rev
> Log:
> More chunked encoding improvements
With this change I get consistent test failures on Windows for 7.0.x and
intermittent failures for trunk.
I haven't see issues on Linux.
My best guess is that the loop-back adaptor implementation on Windows is
clearing the TCP buffers and closing the socket before the client has a
chance to read all the data. Adding a short sleep just before the socket
close fixes the problem. Adding one after the close does not.
My plan is to refactor the unit tests to not rely on receiving a 500
response in error conditions.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org