This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 51e43f4e7b Fix BZ 69466 - rework handling of HEAD requests
51e43f4e7b is described below

commit 51e43f4e7bd2fa51abc9f893766b2e4c6e376142
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Nov 21 16:14:49 2024 +0000

    Fix BZ 69466 - rework handling of HEAD requests
    
    Headers explicitly set by users will not be removed and any header
    present in a HEAD request will also be present in the equivalent GET
    request. There may be some headers, as per RFC 9110, section 9.3.2, that
    are present in a GET request that are not present in the equivalent HEAD
    request.
    
    https://bz.apache.org/bugzilla/show_bug.cgi?id=69466
---
 .../apache/catalina/connector/OutputBuffer.java    |  13 +-
 java/org/apache/coyote/http11/Http11Processor.java |  13 +-
 .../servlet/http/HttpServletDoHeadBaseTest.java    | 206 +++++++++++++++------
 test/javax/servlet/http/TestHttpServlet.java       |  21 ++-
 .../http/TestHttpServletDoHeadValidWrite0.java     |  19 +-
 .../http/TestHttpServletDoHeadValidWrite1.java     |  19 +-
 .../http/TestHttpServletDoHeadValidWrite1023.java  |  19 +-
 .../http/TestHttpServletDoHeadValidWrite1024.java  |  19 +-
 .../http/TestHttpServletDoHeadValidWrite1025.java  |  19 +-
 .../http/TestHttpServletDoHeadValidWrite511.java   |  19 +-
 .../http/TestHttpServletDoHeadValidWrite512.java   |  19 +-
 .../http/TestHttpServletDoHeadValidWrite513.java   |  19 +-
 test/javax/servlet/http/TesterConstants.java       |  23 +++
 .../coyote/http11/TestHttp11ProcessorDoHead.java   | 175 +++++++++++++++++
 test/org/apache/coyote/http2/Http2TestBase.java    |  32 +++-
 webapps/docs/changelog.xml                         |   7 +
 16 files changed, 511 insertions(+), 131 deletions(-)

diff --git a/java/org/apache/catalina/connector/OutputBuffer.java 
b/java/org/apache/catalina/connector/OutputBuffer.java
index 6776fbe82a..dc6fb3cc53 100644
--- a/java/org/apache/catalina/connector/OutputBuffer.java
+++ b/java/org/apache/catalina/connector/OutputBuffer.java
@@ -232,13 +232,14 @@ public class OutputBuffer extends Writer {
             flushCharBuffer();
         }
 
+        // Content length can be calculated if:
+        // - the response has not been committed
+        // AND
+        // - the content length has not been explicitly set
+        // AND
+        // - some content has been written OR this is NOT a HEAD request
         if ((!coyoteResponse.isCommitted()) && 
(coyoteResponse.getContentLengthLong() == -1) &&
-                !coyoteResponse.getRequest().method().equals("HEAD")) {
-            // If this didn't cause a commit of the response, the final content
-            // length can be calculated. Only do this if this is not a HEAD
-            // request since in that case no body should have been written and
-            // setting a value of zero here will result in an explicit content
-            // length of zero being set on the response.
+                ((bb.remaining() > 0 || 
!coyoteResponse.getRequest().method().equals("HEAD")))) {
             coyoteResponse.setContentLength(bb.remaining());
         }
 
diff --git a/java/org/apache/coyote/http11/Http11Processor.java 
b/java/org/apache/coyote/http11/Http11Processor.java
index e33ff1fa6e..45042578ea 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -904,8 +904,10 @@ public class Http11Processor extends AbstractProcessor {
             }
         }
 
-        if (request.method().equals("HEAD")) {
-            // No entity body
+        MessageBytes methodMB = request.method();
+        boolean head = methodMB.equals("HEAD");
+        if (head) {
+            // Any entity body, if present, should not be sent
             outputBuffer.addActiveFilter(outputFilters[Constants.VOID_FILTER]);
             contentDelimitation = true;
         }
@@ -945,6 +947,13 @@ public class Http11Processor extends AbstractProcessor {
             headers.setValue("Content-Length").setLong(contentLength);
             
outputBuffer.addActiveFilter(outputFilters[Constants.IDENTITY_FILTER]);
             contentDelimitation = true;
+        } else if (head) {
+            /*
+             * The OutputBuffer can't differentiate between a zero length 
response because GET would have produced a
+             * zero length body and a zero length response because HEAD didn't 
write any content. Therefore skip setting
+             * the "transfer-encoding: chunked" header as that may not be 
consistent with what would be seen with the
+             * equivalent GET.
+             */
         } else {
             // If the response code supports an entity body and we're on
             // HTTP 1.1 then we chunk unless we have a Connection: close header
diff --git a/test/javax/servlet/http/HttpServletDoHeadBaseTest.java 
b/test/javax/servlet/http/HttpServletDoHeadBaseTest.java
index c49b6fea53..0856a03227 100644
--- a/test/javax/servlet/http/HttpServletDoHeadBaseTest.java
+++ b/test/javax/servlet/http/HttpServletDoHeadBaseTest.java
@@ -22,7 +22,6 @@ import java.io.PrintWriter;
 import java.lang.reflect.Field;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -32,12 +31,13 @@ import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runners.Parameterized.Parameter;
 
+import org.apache.catalina.LifecycleException;
 import org.apache.catalina.connector.OutputBuffer;
 import org.apache.catalina.connector.Response;
 import org.apache.catalina.connector.ResponseFacade;
 import org.apache.catalina.core.StandardContext;
 import org.apache.catalina.startup.Tomcat;
-import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.coyote.http2.Http2TestBase;
 import org.apache.tomcat.util.buf.ByteChunk;
 import org.apache.tomcat.util.collections.CaseInsensitiveKeyMap;
 import org.apache.tomcat.util.compat.JreCompat;
@@ -46,7 +46,7 @@ import org.apache.tomcat.util.compat.JreCompat;
  * Split into multiple tests as a single test takes so long it impacts the time
  * of an entire test run.
  */
-public class HttpServletDoHeadBaseTest extends TomcatBaseTest {
+public class HttpServletDoHeadBaseTest extends Http2TestBase {
 
     // Tomcat has a minimum output buffer size of 8 * 1024.
     // (8 * 1024) /16 = 512
@@ -54,37 +54,31 @@ public class HttpServletDoHeadBaseTest extends 
TomcatBaseTest {
     private static final String VALID = "** valid data **";
     private static final String INVALID = "* invalid data *";
 
-    protected static final Integer BUFFERS[] = new Integer[] { Integer.valueOf 
(16), Integer.valueOf(8 * 1024), Integer.valueOf(16 * 1024) };
+    protected static final Integer BUFFERS[] =
+            new Integer[] { Integer.valueOf(16), Integer.valueOf(8 * 1024), 
Integer.valueOf(16 * 1024) };
 
-    protected static final Integer COUNTS[] = new Integer[] { 
Integer.valueOf(0), Integer.valueOf(1),
-            Integer.valueOf(511), Integer.valueOf(512), Integer.valueOf(513),
-            Integer.valueOf(1023), Integer.valueOf(1024), 
Integer.valueOf(1025) };
+    protected static final Integer COUNTS[] =
+            new Integer[] { Integer.valueOf(0), Integer.valueOf(1), 
Integer.valueOf(511), Integer.valueOf(512),
+                    Integer.valueOf(513), Integer.valueOf(1023), 
Integer.valueOf(1024), Integer.valueOf(1025) };
 
-    @Parameter(0)
+    @Parameter(2)
     public int bufferSize;
-    @Parameter(1)
+    @Parameter(3)
     public boolean useWriter;
-    @Parameter(2)
+    @Parameter(4)
     public int invalidWriteCount;
-    @Parameter(3)
+    @Parameter(5)
     public ResetType resetType;
-    @Parameter(4)
+    @Parameter(6)
     public int validWriteCount;
-    @Parameter(5)
+    @Parameter(7)
     public boolean explicitFlush;
 
     @Test
     public void testDoHead() throws Exception {
         Tomcat tomcat = getTomcatInstance();
 
-        // No file system docBase required
-        StandardContext ctx = (StandardContext) tomcat.addContext("", null);
-
-        HeadTestServlet s = new HeadTestServlet(bufferSize, useWriter, 
invalidWriteCount, resetType, validWriteCount, explicitFlush);
-        Tomcat.addServlet(ctx, "HeadTestServlet", s);
-        ctx.addServletMappingDecoded("/test", "HeadTestServlet");
-
-        tomcat.start();
+        configureAndStartWebApplication();
 
         Map<String,List<String>> getHeaders = new CaseInsensitiveKeyMap<>();
         String path = "http://localhost:"; + getPort() + "/test";
@@ -94,17 +88,26 @@ public class HttpServletDoHeadBaseTest extends 
TomcatBaseTest {
         Assert.assertEquals(HttpServletResponse.SC_OK, rc);
         out.recycle();
 
-        Map<String,List<String>> headHeaders = new HashMap<>();
+        Map<String,List<String>> headHeaders = new CaseInsensitiveKeyMap<>();
         rc = headUrl(path, out, headHeaders);
         Assert.assertEquals(HttpServletResponse.SC_OK, rc);
 
-        // Headers should be the same (apart from Date)
+        // Date header is likely to be different so just remove it from both 
GET and HEAD.
+        getHeaders.remove("date");
+        headHeaders.remove("date");
+        /*
+         * There are some headers that are optional for HEAD. See RFC 9110, 
section 9.3.2. If present, they must be the
+         * same for both GET and HEAD. If not present in HEAD, remove them 
from GET.
+         */
+        for (String header : TesterConstants.OPTIONAL_HEADERS_WITH_HEAD) {
+            if (!headHeaders.containsKey(header)) {
+                getHeaders.remove(header);
+            }
+        }
+
         Assert.assertEquals(getHeaders.size(), headHeaders.size());
-        for (Map.Entry<String, List<String>> getHeader : 
getHeaders.entrySet()) {
+        for (Map.Entry<String,List<String>> getHeader : getHeaders.entrySet()) 
{
             String headerName = getHeader.getKey();
-            if ("date".equalsIgnoreCase(headerName)) {
-                continue;
-            }
             Assert.assertTrue(headerName, headHeaders.containsKey(headerName));
             List<String> getValues = getHeader.getValue();
             List<String> headValues = headHeaders.get(headerName);
@@ -118,6 +121,102 @@ public class HttpServletDoHeadBaseTest extends 
TomcatBaseTest {
     }
 
 
+    @Test
+    public void testDoHeadHttp2() throws Exception {
+        StringBuilder debug = new StringBuilder();
+        try {
+            http2Connect();
+
+            // Get request
+            byte[] frameHeaderGet = new byte[9];
+            ByteBuffer headersPayloadGet = ByteBuffer.allocate(128);
+            buildGetRequest(frameHeaderGet, headersPayloadGet, null, 3, 
"/test");
+            writeFrame(frameHeaderGet, headersPayloadGet);
+
+            // Want the headers frame for stream 3
+            parser.readFrame();
+            while (!output.getTrace().startsWith("3-HeadersStart\n")) {
+                debug.append(output.getTrace());
+                output.clearTrace();
+                parser.readFrame();
+            }
+            String traceGet = output.getTrace();
+            debug.append(output.getTrace());
+            output.clearTrace();
+
+            // Head request
+            byte[] frameHeaderHead = new byte[9];
+            ByteBuffer headersPayloadHead = ByteBuffer.allocate(128);
+            buildHeadRequest(frameHeaderHead, headersPayloadHead, 5, "/test");
+            writeFrame(frameHeaderHead, headersPayloadHead);
+
+            // Want the headers frame for stream 5
+            parser.readFrame();
+            while (!output.getTrace().startsWith("5-HeadersStart\n")) {
+                debug.append(output.getTrace());
+                output.clearTrace();
+                parser.readFrame();
+            }
+            String traceHead = output.getTrace();
+            debug.append(output.getTrace());
+
+            String[] getHeaders = traceGet.split("\n");
+            String[] headHeaders = traceHead.split("\n");
+
+            int i = 0;
+            int j = 0;
+            for (; i < getHeaders.length; i++) {
+                /*
+                 * There are some headers that are optional for HEAD. See RFC 
9110, section 9.3.2. If present, they must
+                 * be the same for both GET and HEAD. If not present in HEAD, 
ignore them in GET.
+                 */
+                boolean skip = false;
+                for (String optional : 
TesterConstants.OPTIONAL_HEADERS_WITH_HEAD) {
+                    if (getHeaders[i].contains(optional)) {
+                        if (!headHeaders[i].contains(optional)) {
+                            // Skip it
+                            skip = true;
+                        }
+                        // Found it. Don't need to check the remaining headers
+                        break;
+                    }
+                }
+                if (skip) {
+                    continue;
+                }
+
+                // Remaining 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++;
+            }
+        } catch (Exception t) {
+            System.out.println(debug.toString());
+            throw t;
+        }
+    }
+
+
+    @Override
+    protected void configureAndStartWebApplication() throws LifecycleException 
{
+        Tomcat tomcat = getTomcatInstance();
+
+        // No file system docBase required
+        StandardContext ctxt = (StandardContext) getProgrammaticRootContext();
+
+        Tomcat.addServlet(ctxt, "simple", new SimpleServlet());
+        ctxt.addServletMappingDecoded("/simple", "simple");
+
+        HeadTestServlet s = new HeadTestServlet(bufferSize, useWriter, 
invalidWriteCount, resetType, validWriteCount,
+                explicitFlush);
+        Tomcat.addServlet(ctxt, "HeadTestServlet", s);
+        ctxt.addServletMappingDecoded("/test", "HeadTestServlet");
+
+        tomcat.start();
+    }
+
+
     private static class HeadTestServlet extends HttpServlet {
 
         private static final long serialVersionUID = 1L;
@@ -149,43 +248,32 @@ public class HttpServletDoHeadBaseTest extends 
TomcatBaseTest {
             if (JreCompat.isJre19Available() && "HEAD".equals(req.getMethod()) 
&& useWriter &&
                     resetType != ResetType.NONE) {
                 /*
-                 * Using legacy (non-legacy isn't available until Servlet 6.0 /
-                 * Tomcat 10.1.x) HEAD handling with a Writer on Java 19+.
-                 * HttpServlet wraps the response. The test is sensitive to
-                 * buffer sizes. The size of the buffer HttpServlet uses varies
-                 * with Java version. For the tests to pass the number of
-                 * characters that can be written before the response is
-                 * committed needs to be the same.
+                 * Using legacy (non-legacy isn't available until Servlet 6.0 
/ Tomcat 10.1.x) HEAD handling with a
+                 * Writer on Java 19+.
+                 *
+                 * HttpServlet wraps the response. The test is sensitive to 
buffer sizes. For the tests to pass the
+                 * number of characters that can be written before the 
response is committed needs to be the same.
                  *
-                 * Internally, the Tomcat response buffer defaults to 8192
-                 * bytes. When a Writer is used, an additional 8192 byte buffer
-                 * is used for char->byte.
+                 * Internally, the Tomcat response buffer defaults to 8192 
bytes. When a Writer is used, an additional
+                 * 8192 byte buffer is used for char->byte.
                  *
-                 * With Java <19, the char->byte buffer used by HttpServlet
-                 * processing HEAD requests in legacy mode is created as a 8192
-                 * byte buffer which is consistent with the buffer Tomcat uses
-                 * internally.
+                 * With Java <19, the char->byte buffer used by HttpServlet 
processing HEAD requests in legacy mode is
+                 * created as a 8192 byte buffer which is consistent with the 
buffer Tomcat uses internally.
                  *
-                 * With Java 19+, the char->byte buffer used by HttpServlet
-                 * processing HEAD requests in legacy mode is created as a 512
-                 * byte buffer.
+                 * With Java 19+, the char->byte buffer used by HttpServlet 
processing HEAD requests in legacy mode is
+                 * created as a 512 byte buffer.
                  *
-                 * If the response isn't reset, none of this matters as it is
-                 * just the size of the response buffer and the size of the
-                 * response that determines if chunked encoding is used.
-                 * However, if the response is reset then things get 
interesting
-                 * as the amount of response data that can be written before
-                 * committing the response is the combination of the char->byte
-                 * buffer and the response buffer. Because the char->byte 
buffer
-                 * size in legacy mode varies with Java version, the size of 
the
-                 * response buffer needs to be adjusted to compensate so that
-                 * both GET and HEAD can write the same amount of data before
-                 * committing the response. To make matters worse, to obtain 
the
-                 * correct behaviour the response buffer size needs to be reset
-                 * back to 8192 after the reset.
+                 * If the response isn't reset, none of this matters as it is 
just the size of the response buffer and
+                 * the size of the response that determines if chunked 
encoding is used. However, if the response is
+                 * reset then things get interesting as the amount of response 
data that can be written before
+                 * committing the response is the combination of the 
char->byte buffer and the response buffer. Because
+                 * the char->byte buffer size in legacy mode varies with Java 
version, the size of the response buffer
+                 * needs to be adjusted to compensate so that both GET and 
HEAD can write the same amount of data before
+                 * committing the response. To make matters worse, to obtain 
the correct behaviour the response buffer
+                 * size needs to be reset back to 8192 after the reset.
                  *
-                 * This is why the legacy mode is problematic and the new
-                 * approach of the container handling HEAD is better.
+                 * This is why the legacy mode is problematic and the new 
approach of the container handling HEAD is
+                 * better.
                  */
 
                 // Response buffer is always no smaller than originalBufferSize
diff --git a/test/javax/servlet/http/TestHttpServlet.java 
b/test/javax/servlet/http/TestHttpServlet.java
index cb9f633132..a842572994 100644
--- a/test/javax/servlet/http/TestHttpServlet.java
+++ b/test/javax/servlet/http/TestHttpServlet.java
@@ -64,8 +64,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.assertEquals(LargeBodyServlet.RESPONSE_LENGTH, 
resHeaders.get("Content-Length").get(0));
     }
 
 
@@ -180,17 +179,27 @@ public class TestHttpServlet extends TomcatBaseTest {
         Assert.assertEquals(HttpServletResponse.SC_OK, rc);
         out.recycle();
 
-        Map<String,List<String>> headHeaders = new HashMap<>();
+        Map<String,List<String>> headHeaders = new CaseInsensitiveKeyMap<>();
         rc = headUrl(path, out, headHeaders);
         Assert.assertEquals(HttpServletResponse.SC_OK, rc);
 
+        // Date header is likely to be different so just remove it from both 
GET and HEAD.
+        getHeaders.remove("date");
+        headHeaders.remove("date");
+        /*
+         * There are some headers that are optional for HEAD. See RFC 9110, 
section 9.3.2. If present, they must be the
+         * same for both GET and HEAD. If not present in HEAD, remove them 
from GET.
+         */
+        for (String header : TesterConstants.OPTIONAL_HEADERS_WITH_HEAD) {
+            if (!headHeaders.containsKey(header)) {
+                getHeaders.remove(header);
+            }
+        }
+
         // Headers should be the same (apart from Date)
         Assert.assertEquals(getHeaders.size(), headHeaders.size());
         for (Map.Entry<String, List<String>> getHeader : 
getHeaders.entrySet()) {
             String headerName = getHeader.getKey();
-            if ("date".equalsIgnoreCase(headerName)) {
-                continue;
-            }
             Assert.assertTrue(headerName, headHeaders.containsKey(headerName));
             List<String> getValues = getHeader.getValue();
             List<String> headValues = headHeaders.get(headerName);
diff --git a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite0.java 
b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite0.java
index 355b52cdde..d0bbfd07a8 100644
--- a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite0.java
+++ b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite0.java
@@ -30,16 +30,21 @@ import org.junit.runners.Parameterized;
 @RunWith(Parameterized.class)
 public class TestHttpServletDoHeadValidWrite0 extends 
HttpServletDoHeadBaseTest {
 
-    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5}")
+    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5} {6} 
{7}")
     public static Collection<Object[]> parameters() {
+        Collection<Object[]> baseData = data();
 
         List<Object[]> parameterSets = new ArrayList<>();
-        for (Integer buf : BUFFERS) {
-            for (Boolean w : booleans) {
-                for (Integer c1 : COUNTS) {
-                    for (ResetType rt : ResetType.values()) {
-                        for (Boolean f : booleans) {
-                            parameterSets.add(new Object[] { buf, w, c1, rt, 
Integer.valueOf(0), f });
+        for (Object[] base : baseData) {
+            for (Integer buf : BUFFERS) {
+                for (Boolean w : booleans) {
+                    for (Integer c1 : COUNTS) {
+                        for (ResetType rt : ResetType.values()) {
+                            for (Boolean f : booleans) {
+                                        parameterSets.add(new Object[] {
+                                                base[0], base[1],
+                                                buf, w, c1, rt, 
Integer.valueOf(0), f });
+                            }
                         }
                     }
                 }
diff --git a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1.java 
b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1.java
index 73ee6c52bc..f57c08c18b 100644
--- a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1.java
+++ b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1.java
@@ -30,16 +30,21 @@ import org.junit.runners.Parameterized;
 @RunWith(Parameterized.class)
 public class TestHttpServletDoHeadValidWrite1 extends 
HttpServletDoHeadBaseTest {
 
-    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5}")
+    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5} {6} 
{7}")
     public static Collection<Object[]> parameters() {
+        Collection<Object[]> baseData = data();
 
         List<Object[]> parameterSets = new ArrayList<>();
-        for (Integer buf : BUFFERS) {
-            for (Boolean w : booleans) {
-                for (Integer c1 : COUNTS) {
-                    for (ResetType rt : ResetType.values()) {
-                        for (Boolean f : booleans) {
-                            parameterSets.add(new Object[] { buf, w, c1, rt, 
Integer.valueOf(1), f });
+        for (Object[] base : baseData) {
+            for (Integer buf : BUFFERS) {
+                for (Boolean w : booleans) {
+                    for (Integer c1 : COUNTS) {
+                        for (ResetType rt : ResetType.values()) {
+                            for (Boolean f : booleans) {
+                                        parameterSets.add(new Object[] {
+                                                base[0], base[1],
+                                                buf, w, c1, rt, 
Integer.valueOf(1), f });
+                            }
                         }
                     }
                 }
diff --git a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1023.java 
b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1023.java
index a261ee27cb..a801f926e1 100644
--- a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1023.java
+++ b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1023.java
@@ -30,16 +30,21 @@ import org.junit.runners.Parameterized;
 @RunWith(Parameterized.class)
 public class TestHttpServletDoHeadValidWrite1023 extends 
HttpServletDoHeadBaseTest {
 
-    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5}")
+    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5} {6} 
{7}")
     public static Collection<Object[]> parameters() {
+        Collection<Object[]> baseData = data();
 
         List<Object[]> parameterSets = new ArrayList<>();
-        for (Integer buf : BUFFERS) {
-            for (Boolean w : booleans) {
-                for (Integer c1 : COUNTS) {
-                    for (ResetType rt : ResetType.values()) {
-                        for (Boolean f : booleans) {
-                            parameterSets.add(new Object[] { buf, w, c1, rt, 
Integer.valueOf(1023), f });
+        for (Object[] base : baseData) {
+            for (Integer buf : BUFFERS) {
+                for (Boolean w : booleans) {
+                    for (Integer c1 : COUNTS) {
+                        for (ResetType rt : ResetType.values()) {
+                            for (Boolean f : booleans) {
+                                        parameterSets.add(new Object[] {
+                                                base[0], base[1],
+                                                buf, w, c1, rt, 
Integer.valueOf(1023), f });
+                            }
                         }
                     }
                 }
diff --git a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1024.java 
b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1024.java
index 2679c033dd..fea621d1f8 100644
--- a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1024.java
+++ b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1024.java
@@ -30,16 +30,21 @@ import org.junit.runners.Parameterized;
 @RunWith(Parameterized.class)
 public class TestHttpServletDoHeadValidWrite1024 extends 
HttpServletDoHeadBaseTest {
 
-    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5}")
+    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5} {6} 
{7}")
     public static Collection<Object[]> parameters() {
+        Collection<Object[]> baseData = data();
 
         List<Object[]> parameterSets = new ArrayList<>();
-        for (Integer buf : BUFFERS) {
-            for (Boolean w : booleans) {
-                for (Integer c1 : COUNTS) {
-                    for (ResetType rt : ResetType.values()) {
-                        for (Boolean f : booleans) {
-                            parameterSets.add(new Object[] { buf, w, c1, rt, 
Integer.valueOf(1024), f });
+        for (Object[] base : baseData) {
+            for (Integer buf : BUFFERS) {
+                for (Boolean w : booleans) {
+                    for (Integer c1 : COUNTS) {
+                        for (ResetType rt : ResetType.values()) {
+                            for (Boolean f : booleans) {
+                                        parameterSets.add(new Object[] {
+                                                base[0], base[1],
+                                                buf, w, c1, rt, 
Integer.valueOf(1024), f });
+                            }
                         }
                     }
                 }
diff --git a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1025.java 
b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1025.java
index 286cab27d1..8cd295e729 100644
--- a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1025.java
+++ b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite1025.java
@@ -30,16 +30,21 @@ import org.junit.runners.Parameterized;
 @RunWith(Parameterized.class)
 public class TestHttpServletDoHeadValidWrite1025 extends 
HttpServletDoHeadBaseTest {
 
-    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5}")
+    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5} {6} 
{7}")
     public static Collection<Object[]> parameters() {
+        Collection<Object[]> baseData = data();
 
         List<Object[]> parameterSets = new ArrayList<>();
-        for (Integer buf : BUFFERS) {
-            for (Boolean w : booleans) {
-                for (Integer c1 : COUNTS) {
-                    for (ResetType rt : ResetType.values()) {
-                        for (Boolean f : booleans) {
-                            parameterSets.add(new Object[] { buf, w, c1, rt, 
Integer.valueOf(1025), f });
+        for (Object[] base : baseData) {
+            for (Integer buf : BUFFERS) {
+                for (Boolean w : booleans) {
+                    for (Integer c1 : COUNTS) {
+                        for (ResetType rt : ResetType.values()) {
+                            for (Boolean f : booleans) {
+                                        parameterSets.add(new Object[] {
+                                                base[0], base[1],
+                                                buf, w, c1, rt, 
Integer.valueOf(1025), f });
+                            }
                         }
                     }
                 }
diff --git a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite511.java 
b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite511.java
index 46012916ad..8d56c5cc69 100644
--- a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite511.java
+++ b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite511.java
@@ -30,16 +30,21 @@ import org.junit.runners.Parameterized;
 @RunWith(Parameterized.class)
 public class TestHttpServletDoHeadValidWrite511 extends 
HttpServletDoHeadBaseTest {
 
-    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5}")
+    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5} {6} 
{7}")
     public static Collection<Object[]> parameters() {
+        Collection<Object[]> baseData = data();
 
         List<Object[]> parameterSets = new ArrayList<>();
-        for (Integer buf : BUFFERS) {
-            for (Boolean w : booleans) {
-                for (Integer c1 : COUNTS) {
-                    for (ResetType rt : ResetType.values()) {
-                        for (Boolean f : booleans) {
-                            parameterSets.add(new Object[] { buf, w, c1, rt, 
Integer.valueOf(511), f });
+        for (Object[] base : baseData) {
+            for (Integer buf : BUFFERS) {
+                for (Boolean w : booleans) {
+                    for (Integer c1 : COUNTS) {
+                        for (ResetType rt : ResetType.values()) {
+                            for (Boolean f : booleans) {
+                                        parameterSets.add(new Object[] {
+                                                base[0], base[1],
+                                                buf, w, c1, rt, 
Integer.valueOf(511), f });
+                            }
                         }
                     }
                 }
diff --git a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite512.java 
b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite512.java
index d314546f9b..9f923c146e 100644
--- a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite512.java
+++ b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite512.java
@@ -30,16 +30,21 @@ import org.junit.runners.Parameterized;
 @RunWith(Parameterized.class)
 public class TestHttpServletDoHeadValidWrite512 extends 
HttpServletDoHeadBaseTest {
 
-    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5}")
+    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5} {6} 
{7}")
     public static Collection<Object[]> parameters() {
+        Collection<Object[]> baseData = data();
 
         List<Object[]> parameterSets = new ArrayList<>();
-        for (Integer buf : BUFFERS) {
-            for (Boolean w : booleans) {
-                for (Integer c1 : COUNTS) {
-                    for (ResetType rt : ResetType.values()) {
-                        for (Boolean f : booleans) {
-                            parameterSets.add(new Object[] { buf, w, c1, rt, 
Integer.valueOf(512), f });
+        for (Object[] base : baseData) {
+            for (Integer buf : BUFFERS) {
+                for (Boolean w : booleans) {
+                    for (Integer c1 : COUNTS) {
+                        for (ResetType rt : ResetType.values()) {
+                            for (Boolean f : booleans) {
+                                        parameterSets.add(new Object[] {
+                                                base[0], base[1],
+                                                buf, w, c1, rt, 
Integer.valueOf(512), f });
+                            }
                         }
                     }
                 }
diff --git a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite513.java 
b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite513.java
index c512bb7332..274ca0fe84 100644
--- a/test/javax/servlet/http/TestHttpServletDoHeadValidWrite513.java
+++ b/test/javax/servlet/http/TestHttpServletDoHeadValidWrite513.java
@@ -30,16 +30,21 @@ import org.junit.runners.Parameterized;
 @RunWith(Parameterized.class)
 public class TestHttpServletDoHeadValidWrite513 extends 
HttpServletDoHeadBaseTest {
 
-    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5}")
+    @Parameterized.Parameters(name = "{index}: {0} {1} {2} {3} {4} {5} {6} 
{7}")
     public static Collection<Object[]> parameters() {
+        Collection<Object[]> baseData = data();
 
         List<Object[]> parameterSets = new ArrayList<>();
-        for (Integer buf : BUFFERS) {
-            for (Boolean w : booleans) {
-                for (Integer c1 : COUNTS) {
-                    for (ResetType rt : ResetType.values()) {
-                        for (Boolean f : booleans) {
-                            parameterSets.add(new Object[] { buf, w, c1, rt, 
Integer.valueOf(513), f });
+        for (Object[] base : baseData) {
+            for (Integer buf : BUFFERS) {
+                for (Boolean w : booleans) {
+                    for (Integer c1 : COUNTS) {
+                        for (ResetType rt : ResetType.values()) {
+                            for (Boolean f : booleans) {
+                                        parameterSets.add(new Object[] {
+                                                base[0], base[1],
+                                                buf, w, c1, rt, 
Integer.valueOf(513), f });
+                            }
                         }
                     }
                 }
diff --git a/test/javax/servlet/http/TesterConstants.java 
b/test/javax/servlet/http/TesterConstants.java
new file mode 100644
index 0000000000..e68283b6f3
--- /dev/null
+++ b/test/javax/servlet/http/TesterConstants.java
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package javax.servlet.http;
+
+public class TesterConstants {
+
+    public static final String[] OPTIONAL_HEADERS_WITH_HEAD =
+            new String[] { "content-length", "vary", "transfer-encoding", 
"trailers" };
+}
diff --git a/test/org/apache/coyote/http11/TestHttp11ProcessorDoHead.java 
b/test/org/apache/coyote/http11/TestHttp11ProcessorDoHead.java
new file mode 100644
index 0000000000..ef35a867b5
--- /dev/null
+++ b/test/org/apache/coyote/http11/TestHttp11ProcessorDoHead.java
@@ -0,0 +1,175 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.coyote.http11;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+@RunWith(Parameterized.class)
+public class TestHttp11ProcessorDoHead extends TomcatBaseTest {
+
+    private static final int DEFAULT_BUFFER_SIZE = 8192;
+
+    @Parameterized.Parameters(name = "{index}: explicit-cl[{0}], cl[{1}], 
useContainerHead[{2}], expected-cl[{3}]")
+    public static Collection<Object[]> parameters() {
+
+        List<Object[]> parameterSets = new ArrayList<>();
+
+        int[] contentLengths = new int[] { DEFAULT_BUFFER_SIZE / 2, 
DEFAULT_BUFFER_SIZE -1 , DEFAULT_BUFFER_SIZE, DEFAULT_BUFFER_SIZE + 1, 
DEFAULT_BUFFER_SIZE * 2};
+
+        for (Boolean explicitContentLength : booleans) {
+            for (int contentLength : contentLengths) {
+                for (HeadSource headSource : HeadSource.values()) {
+                    Integer expectedContentLength = null;
+                    /*
+                     * Tomcat should send a content-length for a head request 
when the same value would be sent for a
+                     * equivalent GET request. Those circumstances are:
+                     * - The Servlet sets an explicit content-length
+                     * - When the container is providing the HEAD response and 
the content-length is less than the
+                     *   buffer size (so the container would normally set the 
content-length on a GET)
+                     */
+                    if (explicitContentLength.booleanValue() ||
+                            !HeadSource.SERVLET.equals(headSource) && 
contentLength <= DEFAULT_BUFFER_SIZE) {
+                        expectedContentLength = Integer.valueOf(contentLength);
+                    }
+                    parameterSets.add(new Object[] { explicitContentLength, 
Integer.valueOf(contentLength),
+                            headSource, expectedContentLength });
+                }
+            }
+        }
+        return parameterSets;
+    }
+
+    @Parameter(0)
+    public boolean explicitContentLength;
+    @Parameter(1)
+    public int contentLength;
+    @Parameter(2)
+    public HeadSource headSource;
+    @Parameter(3)
+    public Integer expectedContentLength;
+
+    @Test
+    public void testHeadWithtDoHeadMethod() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // No file system docBase required
+        Context ctx = getProgrammaticRootContext();
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "DoHeadServlet",
+                new DoHeadServlet(explicitContentLength, contentLength, 
headSource));
+
+        ctx.addServletMappingDecoded("/head", "DoHeadServlet");
+
+        tomcat.start();
+
+        ByteChunk responseBody = new ByteChunk();
+        Map<String, List<String>> responseHeaders = new HashMap<>();
+        int rc = headUrl("http://localhost:"; + getPort() + "/head", 
responseBody, responseHeaders);
+
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+
+        List<String> values = responseHeaders.get("Content-Length");
+        if (expectedContentLength == null) {
+            Assert.assertNull(values);
+        } else {
+            Assert.assertNotNull(values);
+            Assert.assertEquals(1, values.size());
+            Assert.assertEquals(expectedContentLength.toString(), 
values.get(0));
+        }
+    }
+
+
+    private static class DoHeadServlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        private boolean explicitContentLength;
+
+        private int contentLength;
+
+        private HeadSource headSource;
+
+        private DoHeadServlet(boolean explicitContentLength, int 
contentLength, HeadSource headSource) {
+            this.explicitContentLength = explicitContentLength;
+            this.contentLength = contentLength;
+            this.headSource = headSource;
+        }
+
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp) 
throws ServletException, IOException {
+            writeResponseHeaders(resp);
+            PrintWriter pw = resp.getWriter();
+            // Efficiency is not a concern here
+            for (int i = 0; i < contentLength; i++) {
+                pw.print('X');
+            }
+        }
+
+
+        @Override
+        protected void doHead(HttpServletRequest req, HttpServletResponse 
resp) throws ServletException, IOException {
+            switch (headSource) {
+                case SERVLET:
+                    writeResponseHeaders(resp);
+                    break;
+                case CONTAINER_LEGACY:
+                    super.doHead(req, resp);
+                    break;
+            }
+        }
+
+
+        private void writeResponseHeaders(HttpServletResponse resp) {
+            resp.setContentType("text/plain");
+            resp.setCharacterEncoding(StandardCharsets.UTF_8.name());
+            if (explicitContentLength) {
+                resp.setContentLength(contentLength);
+            }
+        }
+    }
+
+
+    private enum HeadSource {
+        SERVLET,
+        CONTAINER_LEGACY
+    }
+}
diff --git a/test/org/apache/coyote/http2/Http2TestBase.java 
b/test/org/apache/coyote/http2/Http2TestBase.java
index e5d4a4ee0d..ceec98e03b 100644
--- a/test/org/apache/coyote/http2/Http2TestBase.java
+++ b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -446,6 +446,34 @@ public abstract class Http2TestBase extends TomcatBaseTest 
{
     }
 
 
+    protected void buildHeadRequest(byte[] headersFrameHeader, ByteBuffer 
headersPayload, int streamId, String path) {
+        MimeHeaders headers = new MimeHeaders();
+        headers.addValue(":method").setString("HEAD");
+        headers.addValue(":scheme").setString("http");
+        headers.addValue(":path").setString(path);
+        headers.addValue(":authority").setString("localhost:" + getPort());
+        hpackEncoder.encode(headers, headersPayload);
+
+        headersPayload.flip();
+
+        ByteUtil.setThreeBytes(headersFrameHeader, 0, headersPayload.limit());
+        headersFrameHeader[3] = FrameType.HEADERS.getIdByte();
+        // Flags. end of headers (0x04)
+        headersFrameHeader[4] = 0x04;
+        // Stream id
+        ByteUtil.set31Bits(headersFrameHeader, 5, streamId);
+    }
+
+
+    protected void sendHeadRequest(int streamId, String path) throws 
IOException {
+        byte[] frameHeader = new byte[9];
+        ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+        buildHeadRequest(frameHeader, headersPayload, streamId, path);
+        writeFrame(frameHeader, headersPayload);
+    }
+
+
     protected void writeFrame(byte[] header, ByteBuffer payload) throws 
IOException {
         writeFrame(header, payload, 0, payload.limit());
     }
@@ -1031,7 +1059,7 @@ public abstract class Http2TestBase extends 
TomcatBaseTest {
     }
 
 
-    class TestOutput implements Output, HeaderEmitter {
+    public class TestOutput implements Output, HeaderEmitter {
 
         private StringBuffer trace = new StringBuffer();
         private String lastStreamId = "0";
@@ -1299,7 +1327,7 @@ public abstract class Http2TestBase extends 
TomcatBaseTest {
     }
 
 
-    protected static class SimpleServlet extends HttpServlet {
+    public static class SimpleServlet extends HttpServlet {
 
         private static final long serialVersionUID = 1L;
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 958c4c34a3..5143d645aa 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -161,6 +161,13 @@
         from external repositories. Prior to this fix, these classes could be
         incorrectly marked as not found. (markt)
       </fix>
+      <fix>
+        <bug>69466</bug>: Rework handling of HEAD requests. Headers explicitly
+        set by users will not be removed and any header present in a HEAD
+        request will also be present in the equivalent GET request. There may 
be
+        some headers, as per RFC 9110, section 9.3.2, that are present in a GET
+        request that are not present in the equivalent HEAD request. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org


Reply via email to