Author: markt Date: Wed Sep 11 13:41:58 2013 New Revision: 1521829 URL: http://svn.apache.org/r1521829 Log: Better adherence to RFC2616 for content-length headers
Modified: tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java Modified: tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java?rev=1521829&r1=1521828&r2=1521829&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java Wed Sep 11 13:41:58 2013 @@ -29,6 +29,7 @@ import java.util.concurrent.LinkedBlocki import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.RequestDispatcher; +import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpUpgradeHandler; import org.apache.coyote.AbstractProcessor; @@ -1110,6 +1111,7 @@ public abstract class AbstractAjpProcess // Set this every time in case limit has been changed via JMX headers.setLimit(endpoint.getMaxHeaderCount()); + boolean contentLengthSet = false; int hCount = requestHeaderMessage.getInt(); for(int i = 0 ; i < hCount ; i++) { String hName = null; @@ -1144,9 +1146,15 @@ public abstract class AbstractAjpProcess if (hId == Constants.SC_REQ_CONTENT_LENGTH || (hId == -1 && tmpMB.equalsIgnoreCase("Content-Length"))) { - // just read the content-length header, so set it long cl = vMB.getLong(); - request.setContentLength(cl); + if (contentLengthSet) { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + error = true; + } else { + contentLengthSet = true; + // Set the content-length header for the request + request.setContentLength(cl); + } if (cl != 0) { bodyPresent = true; } Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1521829&r1=1521828&r2=1521829&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Wed Sep 11 13:41:58 2013 @@ -1263,10 +1263,20 @@ public abstract class AbstractHttp11Proc // Parse content-length header long contentLength = request.getContentLengthLong(); - if (contentLength >= 0 && !contentDelimitation) { - getInputBuffer().addActiveFilter - (inputFilters[Constants.IDENTITY_FILTER]); - contentDelimitation = true; + if (contentLength >= 0) { + if (contentDelimitation) { + // contentDelimitation being true at this point indicates that + // chunked encoding is being used but chunked encoding should + // not be used with a content length. RFC 2616, section 4.4, + // bullet 3 states Content-Length must be ignored in this case - + // so remove it. + headers.removeHeader("content-length"); + request.setContentLength(-1); + } else { + getInputBuffer().addActiveFilter + (inputFilters[Constants.IDENTITY_FILTER]); + contentDelimitation = true; + } } MessageBytes valueMB = headers.getValue("host"); Modified: tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java?rev=1521829&r1=1521828&r2=1521829&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java (original) +++ tomcat/trunk/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java Wed Sep 11 13:41:58 2013 @@ -98,9 +98,20 @@ public class TestAbstractAjpProcessor ex ajpClient.disconnect(); } + @Test + public void testPost() throws Exception { + doTestPost(false, HttpServletResponse.SC_OK); + } + @Test - public void testSimplePost() throws Exception { + public void testPostMultipleContentLength() throws Exception { + // Multiple content lengths + doTestPost(true, HttpServletResponse.SC_BAD_REQUEST); + } + + + public void doTestPost(boolean multipleCL, int expectedStatus) throws Exception { Tomcat tomcat = getTomcatInstance(); @@ -119,6 +130,9 @@ public class TestAbstractAjpProcessor ex TesterAjpMessage forwardMessage = ajpClient.createForwardMessage("/echo-params.jsp", 4); forwardMessage.addHeader(0xA008, "9"); + if (multipleCL) { + forwardMessage.addHeader(0xA008, "99"); + } forwardMessage.addHeader(0xA007, "application/x-www-form-urlencoded"); forwardMessage.end(); @@ -128,15 +142,20 @@ public class TestAbstractAjpProcessor ex TesterAjpMessage responseHeaders = ajpClient.sendMessage(forwardMessage, bodyMessage); - // Expect 3 messages: headers, body, end - validateResponseHeaders(responseHeaders, 200); - // Skip the body - TesterAjpMessage responseBody = ajpClient.readMessage(); - validateResponseBody(responseBody, "test - data"); - validateResponseEnd(ajpClient.readMessage(), true); + validateResponseHeaders(responseHeaders, expectedStatus); + if (expectedStatus == HttpServletResponse.SC_OK) { + // Expect 3 messages: headers, body, end for a valid request + TesterAjpMessage responseBody = ajpClient.readMessage(); + validateResponseBody(responseBody, "test - data"); + validateResponseEnd(ajpClient.readMessage(), true); + + // Double check the connection is still open + validateCpong(ajpClient.cping()); + } else { + // Expect 2 messages: headers, end for an invalid request + validateResponseEnd(ajpClient.readMessage(), false); + } - // Double check the connection is still open - validateCpong(ajpClient.cping()); ajpClient.disconnect(); } Modified: tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java?rev=1521829&r1=1521828&r2=1521829&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java (original) +++ tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java Wed Sep 11 13:41:58 2013 @@ -103,6 +103,20 @@ public class TestAbstractHttp11Processor @Test public void testWithTEChunked() throws Exception { + doTestWithTEChunked(false); + } + + + @Test + public void testWithTEChunkedWithCL() throws Exception { + // Should be ignored + doTestWithTEChunked(true); + } + + + private void doTestWithTEChunked(boolean withCL) + throws Exception { + Tomcat tomcat = getTomcatInstance(); // Use the normal Tomcat ROOT context @@ -114,6 +128,7 @@ public class TestAbstractHttp11Processor String request = "POST /echo-params.jsp HTTP/1.1" + SimpleHttpClient.CRLF + "Host: any" + SimpleHttpClient.CRLF + + (withCL ? "Content-length: 1" + SimpleHttpClient.CRLF : "") + "Transfer-encoding: chunked" + SimpleHttpClient.CRLF + "Content-Type: application/x-www-form-urlencoded" + SimpleHttpClient.CRLF + --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org