This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push: new b9198b0e35 Update the default HEAD response to exclude payload headers b9198b0e35 is described below commit b9198b0e35cc46a080c4d29fc9e0d743ba80dec5 Author: Mark Thomas <ma...@apache.org> AuthorDate: Thu Jan 19 20:40:10 2023 +0000 Update the default HEAD response to exclude payload headers First explicitly allowed in RFC 7231 and also in the current RFC 9110. Servlet 6.0 references RFC 7231 Fixes BZ https://bz.apache.org/bugzilla/show_bug.cgi?id=69379 --- java/org/apache/coyote/http11/Http11Processor.java | 11 ++++++++- java/org/apache/coyote/http2/StreamProcessor.java | 6 +++++ .../servlet/http/HttpServletDoHeadBaseTest.java | 27 +++++++++++++++++----- test/jakarta/servlet/http/TestHttpServlet.java | 20 ++++++++++++++-- webapps/docs/changelog.xml | 4 ++++ 5 files changed, 59 insertions(+), 9 deletions(-) diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java index aaec6e440e..c6df8f5798 100644 --- a/java/org/apache/coyote/http11/Http11Processor.java +++ b/java/org/apache/coyote/http11/Http11Processor.java @@ -905,7 +905,9 @@ public class Http11Processor extends AbstractProcessor { } } - if (request.method().equals("HEAD")) { + MessageBytes methodMB = request.method(); + boolean head = methodMB.equals("HEAD"); + if (head) { // No entity body outputBuffer.addActiveFilter(outputFilters[Constants.VOID_FILTER]); contentDelimitation = true; @@ -1045,6 +1047,13 @@ public class Http11Processor extends AbstractProcessor { headers.setValue("Server").setString(server); } + if (head) { + headers.removeHeader("content-length"); + headers.removeHeader("content-range"); + headers.removeHeader("trailer"); + headers.removeHeader("transfer-encoding"); + } + writeHeaders(response.getStatus(), headers); outputBuffer.commit(); diff --git a/java/org/apache/coyote/http2/StreamProcessor.java b/java/org/apache/coyote/http2/StreamProcessor.java index caa3debea3..3d07567b3c 100644 --- a/java/org/apache/coyote/http2/StreamProcessor.java +++ b/java/org/apache/coyote/http2/StreamProcessor.java @@ -255,6 +255,12 @@ class StreamProcessor extends AbstractProcessor { headers.setValue("Server").setString(server); } } + + // Remove payload headers for HEAD requests + if (coyoteRequest != null && "HEAD".equals(coyoteRequest.method().toString())) { + headers.removeHeader("content-length"); + headers.removeHeader("content-range"); + } } diff --git a/test/jakarta/servlet/http/HttpServletDoHeadBaseTest.java b/test/jakarta/servlet/http/HttpServletDoHeadBaseTest.java index 4fce606aea..eef18ae69f 100644 --- a/test/jakarta/servlet/http/HttpServletDoHeadBaseTest.java +++ b/test/jakarta/servlet/http/HttpServletDoHeadBaseTest.java @@ -95,7 +95,15 @@ public class HttpServletDoHeadBaseTest extends Http2TestBase { rc = headUrl(path, out, headHeaders); Assert.assertEquals(HttpServletResponse.SC_OK, rc); - // Headers should be the same (apart from Date) + // Headers should be the same part from: + // - Date header may be different + // - HEAD requests don't include payload headers + // (RFC 7231, section 4.3.2) + getHeaders.remove("content-length"); + getHeaders.remove("content-range"); + getHeaders.remove("trailer"); + getHeaders.remove("transfer-encoding"); + Assert.assertEquals(getHeaders.size(), headHeaders.size()); for (Map.Entry<String, List<String>> getHeader : getHeaders.entrySet()) { String headerName = getHeader.getKey(); @@ -158,15 +166,22 @@ public class HttpServletDoHeadBaseTest extends Http2TestBase { String[] headHeaders = traceHead.split("\n"); int i = 0; + int j = 0; for (; i < getHeaders.length; i++) { - // Headers should be the same, ignoring the first character which is the steam ID - Assert.assertEquals(getHeaders[i] + "\n" + traceGet + traceHead, '3', getHeaders[i].charAt(0)); - Assert.assertEquals(headHeaders[i] + "\n" + traceGet + traceHead, '5', headHeaders[i].charAt(0)); - Assert.assertEquals(traceGet + traceHead, getHeaders[i].substring(1), headHeaders[i].substring(1)); + // Ignore payload headers + if (getHeaders[i].contains("content-length") || getHeaders[i].contains("content-range") ) { + // Skip + } else { + // Headers should be the same, ignoring the first character which is the steam ID + Assert.assertEquals(getHeaders[i] + "\n" + traceGet + traceHead, '3', getHeaders[i].charAt(0)); + Assert.assertEquals(headHeaders[j] + "\n" + traceGet + traceHead, '5', headHeaders[j].charAt(0)); + Assert.assertEquals(traceGet + traceHead, getHeaders[i].substring(1), headHeaders[j].substring(1)); + j++; + } } // Stream 5 should have one more trace entry - Assert.assertEquals("5-EndOfStream", headHeaders[i]); + Assert.assertEquals("5-EndOfStream", headHeaders[j]); } catch (Exception t) { System.out.println(debug.toString()); throw t; diff --git a/test/jakarta/servlet/http/TestHttpServlet.java b/test/jakarta/servlet/http/TestHttpServlet.java index 05484ab303..e07f74095e 100644 --- a/test/jakarta/servlet/http/TestHttpServlet.java +++ b/test/jakarta/servlet/http/TestHttpServlet.java @@ -45,6 +45,10 @@ import org.apache.tomcat.util.net.TesterSupport.SimpleServlet; public class TestHttpServlet extends TomcatBaseTest { + /* + * Nature of test has changed from original bug report since content-length + * is no longer returned for HEAD requests as allowed by RFC 7231. + */ @Test public void testBug53454() throws Exception { Tomcat tomcat = getTomcatInstance(); @@ -64,8 +68,7 @@ public class TestHttpServlet extends TomcatBaseTest { resHeaders); Assert.assertEquals(HttpServletResponse.SC_OK, rc); - Assert.assertEquals(LargeBodyServlet.RESPONSE_LENGTH, - resHeaders.get("Content-Length").get(0)); + Assert.assertNull(resHeaders.get("Content-Length")); } @@ -178,6 +181,7 @@ public class TestHttpServlet extends TomcatBaseTest { int rc = getUrl(path, out, getHeaders); Assert.assertEquals(HttpServletResponse.SC_OK, rc); + removePayloadHeaders(getHeaders); out.recycle(); Map<String,List<String>> headHeaders = new HashMap<>(); @@ -204,6 +208,18 @@ public class TestHttpServlet extends TomcatBaseTest { } + /* + * Removes headers that are not expected to appear in the response to the + * equivalent HEAD request. + */ + private void removePayloadHeaders(Map<String,List<String>> headers) { + headers.remove("content-length"); + headers.remove("content-range"); + headers.remove("trailer"); + headers.remove("transfer-encoding"); + } + + @Test public void testDoOptions() throws Exception { doTestDoOptions(new OptionsServlet(), "GET, HEAD, OPTIONS"); diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 590dd918d0..48fd8464e4 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -260,6 +260,10 @@ documented by <code>javax.net.ssl.SSLContext.init</code>. This makes error handling more consistent. (remm) </fix> + <fix> + <bug>69379</bug>: The default HEAD response no longer includes the + payload HTTP header fields as per section 9.3.2 of RFC 9110. (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