tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject 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/
Date Wed, 29 Aug 2012 20:26:31 GMT
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


Mime
View raw message