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