Author: markt Date: Fri Dec 20 09:43:33 2013 New Revision: 1552565 URL: http://svn.apache.org/r1552565 Log: Better adherence to RFC2616 for content-length headers.
Modified: tomcat/tc6.0.x/trunk/STATUS.txt tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Modified: tomcat/tc6.0.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1552565&r1=1552564&r2=1552565&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Dec 20 09:43:33 2013 @@ -49,24 +49,6 @@ PATCHES PROPOSED TO BACKPORT: +1: kkolinko (a typo in changelog: s/associated/associate/) -1: -* Better adherence to RFC2616 for content-length headers. - http://people.apache.org/~markt/patches/2013-09-11-tc6-content-length.patch - +1: markt - +0: schultz: I don't see anywhere in RFC2616 that suggests that multiple - Content-Length headers yields an invalid request. Another option - for response code in this case might be 411 Length Required. - There are some conditions under which Content-Type "RFC MUST" be - ignored, and I don't see those cases in any of the *Processor - classes. - markt: RFC2616 states that multiple headers of the same name may be - merged in to a single, multi-valued header without any change in - meaning. Content-Length is clearly defined as a single value - header. If multiple values are not permitted then neither are - multiple headers. - +1: schultz: Changing my vote to +1. - +1: kkolinko - -1: - * Add support for limiting the size of chunk extensions when using chunked encoding. http://people.apache.org/~markt/patches/2013-12-17-chunk-extensions-tc6-v2.patch Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java?rev=1552565&r1=1552564&r2=1552565&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java Fri Dec 20 09:43:33 2013 @@ -26,6 +26,8 @@ import java.security.NoSuchProviderExcep import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; +import javax.servlet.http.HttpServletResponse; + import org.apache.coyote.ActionCode; import org.apache.coyote.ActionHook; import org.apache.coyote.Adapter; @@ -695,6 +697,7 @@ public class AjpAprProcessor implements // 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; @@ -731,8 +734,15 @@ public class AjpAprProcessor implements (hId == -1 && tmpMB.equalsIgnoreCase("Content-Length"))) { // just read the content-length header, so set it long cl = vMB.getLong(); - if(cl < Integer.MAX_VALUE) - request.setContentLength( (int)cl ); + if (contentLengthSet) { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + error = true; + } else { + contentLengthSet = true; + // Set the content-length header for the request + if(cl < Integer.MAX_VALUE) + request.setContentLength( (int)cl ); + } } else if (hId == Constants.SC_REQ_CONTENT_TYPE || (hId == -1 && tmpMB.equalsIgnoreCase("Content-Type"))) { // just read the content-type header, so set it Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1552565&r1=1552564&r2=1552565&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Fri Dec 20 09:43:33 2013 @@ -28,6 +28,8 @@ import java.security.NoSuchProviderExcep import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; +import javax.servlet.http.HttpServletResponse; + import org.apache.coyote.ActionCode; import org.apache.coyote.ActionHook; import org.apache.coyote.Adapter; @@ -700,6 +702,7 @@ public class AjpProcessor implements Act // 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; @@ -736,8 +739,15 @@ public class AjpProcessor implements Act (hId == -1 && tmpMB.equalsIgnoreCase("Content-Length"))) { // just read the content-length header, so set it long cl = vMB.getLong(); - if(cl < Integer.MAX_VALUE) - request.setContentLength( (int)cl ); + if (contentLengthSet) { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + error = true; + } else { + contentLengthSet = true; + // Set the content-length header for the request + if(cl < Integer.MAX_VALUE) + request.setContentLength( (int)cl ); + } } else if (hId == Constants.SC_REQ_CONTENT_TYPE || (hId == -1 && tmpMB.equalsIgnoreCase("Content-Type"))) { // just read the content-type header, so set it Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1552565&r1=1552564&r2=1552565&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Fri Dec 20 09:43:33 2013 @@ -1443,10 +1443,20 @@ public class Http11AprProcessor implemen // Parse content-length header long contentLength = request.getContentLengthLong(); - if (contentLength >= 0 && !contentDelimitation) { - inputBuffer.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 { + inputBuffer.addActiveFilter + (inputFilters[Constants.IDENTITY_FILTER]); + contentDelimitation = true; + } } MessageBytes valueMB = headers.getValue("host"); Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1552565&r1=1552564&r2=1552565&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Fri Dec 20 09:43:33 2013 @@ -1438,10 +1438,20 @@ public class Http11NioProcessor implemen // Parse content-length header long contentLength = request.getContentLengthLong(); - if (contentLength >= 0 && !contentDelimitation) { - inputBuffer.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 { + inputBuffer.addActiveFilter + (inputFilters[Constants.IDENTITY_FILTER]); + contentDelimitation = true; + } } MessageBytes valueMB = headers.getValue("host"); Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1552565&r1=1552564&r2=1552565&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java Fri Dec 20 09:43:33 2013 @@ -1320,10 +1320,20 @@ public class Http11Processor implements // Parse content-length header long contentLength = request.getContentLengthLong(); - if (contentLength >= 0 && !contentDelimitation) { - inputBuffer.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 { + inputBuffer.addActiveFilter + (inputFilters[Constants.IDENTITY_FILTER]); + contentDelimitation = true; + } } MessageBytes valueMB = headers.getValue("host"); Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1552565&r1=1552564&r2=1552565&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Fri Dec 20 09:43:33 2013 @@ -99,6 +99,9 @@ <bug>55228</bug>: Allow web applications to set a HTTP Date header. (markt) </fix> + <fix> + Better adherence to RFC2616 for content-length headers. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org