Author: markt
Date: Thu Aug 30 13:08:35 2012
New Revision: 1378918

URL: http://svn.apache.org/viewvc?rev=1378918&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 
r1378699)
- 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/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=1378918&r1=1378917&r2=1378918&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java 
Thu Aug 30 13:08:35 2012
@@ -411,7 +411,7 @@ public class ChunkedInputFilter implemen
 
         // CRLF terminates the request
         if (chr == Constants.CR || chr == Constants.LF) {
-            parseCRLF(true);
+            parseCRLF(false);
             return false;
         }
 
@@ -502,8 +502,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/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java?rev=1378918&r1=1378917&r2=1378918&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java 
(original)
+++ tomcat/trunk/test/org/apache/catalina/startup/SimpleHttpClient.java Thu Aug 
30 13:08:35 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/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=1378918&r1=1378917&r2=1378918&view=diff
==============================================================================
--- 
tomcat/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java 
(original)
+++ 
tomcat/trunk/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java 
Thu Aug 30 13:08:35 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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to