This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new d7d4374 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63710 d7d4374 is described below commit d7d4374d3f6726758e2bb61a553aa783ebfd565b Author: Mark Thomas <ma...@apache.org> AuthorDate: Thu Sep 12 21:05:11 2019 +0100 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63710 No content-length on 304 responses (and other status codes) --- java/org/apache/coyote/http2/StreamProcessor.java | 23 +++++++---- .../apache/coyote/http2/TestStreamProcessor.java | 47 ++++++++++++++++++++++ webapps/docs/changelog.xml | 5 +++ 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/java/org/apache/coyote/http2/StreamProcessor.java b/java/org/apache/coyote/http2/StreamProcessor.java index 1e8f6a7..c550f19 100644 --- a/java/org/apache/coyote/http2/StreamProcessor.java +++ b/java/org/apache/coyote/http2/StreamProcessor.java @@ -145,7 +145,7 @@ class StreamProcessor extends AbstractProcessor { headers.addValue(":status").setString(Integer.toString(statusCode)); // Check to see if a response body is present - if (!(statusCode < 200 || statusCode == 205 || statusCode == 304)) { + if (!(statusCode < 200 || statusCode == 204 || statusCode == 205 || statusCode == 304)) { String contentType = coyoteResponse.getContentType(); if (contentType != null) { headers.setValue("content-type").setString(contentType); @@ -154,13 +154,20 @@ class StreamProcessor extends AbstractProcessor { if (contentLanguage != null) { headers.setValue("content-language").setString(contentLanguage); } - } - - // Add a content-length header if a content length has been set unless - // the application has already added one - long contentLength = coyoteResponse.getContentLengthLong(); - if (contentLength != -1 && headers.getValue("content-length") == null) { - headers.addValue("content-length").setLong(contentLength); + // Add a content-length header if a content length has been set unless + // the application has already added one + long contentLength = coyoteResponse.getContentLengthLong(); + if (contentLength != -1 && headers.getValue("content-length") == null) { + headers.addValue("content-length").setLong(contentLength); + } + } else { + if (statusCode == 205) { + // RFC 7231 requires the server to explicitly signal an empty + // response in this case + coyoteResponse.setContentLength(0); + } else { + coyoteResponse.setContentLength(-1); + } } // Add date header unless it is an informational response or the diff --git a/test/org/apache/coyote/http2/TestStreamProcessor.java b/test/org/apache/coyote/http2/TestStreamProcessor.java index 6c85992..821fa40 100644 --- a/test/org/apache/coyote/http2/TestStreamProcessor.java +++ b/test/org/apache/coyote/http2/TestStreamProcessor.java @@ -16,9 +16,12 @@ */ package org.apache.coyote.http2; +import java.io.File; import java.io.IOException; import java.io.PrintWriter; import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; import javax.servlet.AsyncContext; import javax.servlet.ServletException; @@ -32,6 +35,7 @@ import org.junit.Test; import org.apache.catalina.Context; import org.apache.catalina.Wrapper; import org.apache.catalina.startup.Tomcat; +import org.apache.tomcat.util.http.FastHttpDateFormat; public class TestStreamProcessor extends Http2TestBase { @@ -111,6 +115,49 @@ public class TestStreamProcessor extends Http2TestBase { } + @Test + public void testPrepareHeaders() throws Exception { + enableHttp2(); + + Tomcat tomcat = getTomcatInstance(); + + File appDir = new File("test/webapp"); + Context ctxt = tomcat.addWebapp(null, "", appDir.getAbsolutePath()); + + Tomcat.addServlet(ctxt, "simple", new SimpleServlet()); + ctxt.addServletMappingDecoded("/simple", "simple"); + + tomcat.start(); + + openClientConnection(); + doHttpUpgrade(); + sendClientPreface(); + validateHttp2InitialResponse(); + + byte[] frameHeader = new byte[9]; + ByteBuffer headersPayload = ByteBuffer.allocate(128); + + List<Header> headers = new ArrayList<>(3); + headers.add(new Header(":method", "GET")); + headers.add(new Header(":scheme", "http")); + headers.add(new Header(":path", "/index.html")); + headers.add(new Header(":authority", "localhost:" + getPort())); + headers.add(new Header("if-modified-since", FastHttpDateFormat.getCurrentDate())); + + buildGetRequest(frameHeader, headersPayload, null, headers, 3); + + writeFrame(frameHeader, headersPayload); + + parser.readFrame(true); + + Assert.assertEquals("3-HeadersStart\n" + + "3-Header-[:status]-[304]\n" + + "3-Header-[etag]-[W/\"934-1567674491000\"]\n" + + "3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n" + + "3-HeadersEnd\n", output.getTrace()); + } + + private static final class AsyncComplete extends HttpServlet { private static final long serialVersionUID = 1L; diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index cb4ea48..7bdf0e7 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -92,6 +92,11 @@ connection is consistent with the increased value. (markt) </fix> <fix> + <bug>63710</bug>: When using HTTP/2, ensure that a + <code>content-length</code> header is not set for those responses with + status codes that do not permit one. (markt) + </fix> + <fix> <bug>63737</bug>: Correct various issues when parsing the <code>accept-encoding</code> header to determine if gzip encoding is supported including only parsing the first header found. (markt) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org