Skip to content

Commit

Permalink
Update the default HEAD response to exclude payload headers
Browse files Browse the repository at this point in the history
First explicitly allowed in RFC 7231 and also in the current RFC 9110
  • Loading branch information
markt-asf committed Jan 19, 2023
1 parent 47508ce commit 8e786a8
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 9 deletions.
10 changes: 9 additions & 1 deletion java/org/apache/coyote/http11/Http11Processor.java
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,8 @@ protected final void prepareResponse() throws IOException {
}

MessageBytes methodMB = request.method();
if (methodMB.equals("HEAD")) {
boolean head = methodMB.equals("HEAD");
if (head) {
// No entity body
outputBuffer.addActiveFilter
(outputFilters[Constants.VOID_FILTER]);
Expand Down Expand Up @@ -1056,6 +1057,13 @@ protected final void prepareResponse() throws IOException {
headers.setValue("Server").setString(server);
}

if (head) {
headers.removeHeader("content-length");
headers.removeHeader("content-range");
headers.removeHeader("trailer");
headers.removeHeader("transfer-encoding");
}

// Build the response header
try {
outputBuffer.sendStatus();
Expand Down
6 changes: 6 additions & 0 deletions java/org/apache/coyote/http2/StreamProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,12 @@ static void prepareHeaders(Request coyoteRequest, Response coyoteResponse, boole
if (statusCode >= 200 && headers.getValue("date") == null) {
headers.addValue("date").setString(FastHttpDateFormat.getCurrentDate());
}

// Remove payload headers for HEAD requests
if (coyoteRequest != null && "HEAD".equals(coyoteRequest.method().toString())) {
headers.removeHeader("content-length");
headers.removeHeader("content-range");
}
}


Expand Down
27 changes: 21 additions & 6 deletions test/jakarta/servlet/http/HttpServletDoHeadBaseTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,15 @@ public void testDoHead() throws Exception {
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();
Expand Down Expand Up @@ -158,15 +166,22 @@ public void testDoHeadHttp2() throws Exception {
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;
Expand Down
20 changes: 18 additions & 2 deletions test/jakarta/servlet/http/TestHttpServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@

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();
Expand All @@ -64,8 +68,7 @@ public void testBug53454() throws Exception {
resHeaders);

Assert.assertEquals(HttpServletResponse.SC_OK, rc);
Assert.assertEquals(LargeBodyServlet.RESPONSE_LENGTH,
resHeaders.get("Content-Length").get(0));
Assert.assertNull(resHeaders.get("Content-Length"));
}


Expand Down Expand Up @@ -178,6 +181,7 @@ private void doTestHead(Servlet servlet) throws Exception {

int rc = getUrl(path, out, getHeaders);
Assert.assertEquals(HttpServletResponse.SC_OK, rc);
removePayloadHeaders(getHeaders);
out.recycle();

Map<String,List<String>> headHeaders = new HashMap<>();
Expand All @@ -204,6 +208,18 @@ private void doTestHead(Servlet servlet) throws Exception {
}


/*
* 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");
Expand Down
4 changes: 4 additions & 0 deletions webapps/docs/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@
Implement the new Servlet API methods for setting character encodings
that accept {@code Charset} objects. (markt)
</add>
<update>
The default HEAD response no longer includes the payload HTTP header
fields as per section 9.3.2 of RFC 9110. (markt)
</update>
</changelog>
</subsection>
<subsection name="Coyote">
Expand Down

0 comments on commit 8e786a8

Please sign in to comment.