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

Reply via email to