You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2022/08/08 13:19:48 UTC
[tomcat] 01/02: Align host header processing with RFC 9113
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 1a93cf89321e7a86aedcd39206b52cf605fe0806
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Aug 8 13:05:53 2022 +0100
Align host header processing with RFC 9113
---
.../apache/coyote/http2/LocalStrings.properties | 1 +
java/org/apache/coyote/http2/Stream.java | 71 +++++++++++++---
.../apache/coyote/http2/TestHttp2Section_8_1.java | 99 ++++++++++++++++++++++
webapps/docs/changelog.xml | 13 +++
4 files changed, 170 insertions(+), 14 deletions(-)
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties b/java/org/apache/coyote/http2/LocalStrings.properties
index 823ea18a7c..62f3f189bb 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -97,6 +97,7 @@ stream.header.required=Connection [{0}], Stream [{1}], One or more required head
stream.header.te=Connection [{0}], Stream [{1}], HTTP header [te] is not permitted to have the value [{2}] in an HTTP/2 request
stream.header.unexpectedPseudoHeader=Connection [{0}], Stream [{1}], Pseudo header [{2}] received after a regular header
stream.header.unknownPseudoHeader=Connection [{0}], Stream [{1}], Unknown pseudo header [{2}] received
+stream.host.inconsistent=Connection [{0}], Stream [{1}], The header host header [{2}] is inconsistent with previously provided values for host [{3}] and/or port [{4}]
stream.inputBuffer.copy=Copying [{0}] bytes from inBuffer to outBuffer
stream.inputBuffer.dispatch=Data added to inBuffer when read interest is registered. Triggering a read dispatch
stream.inputBuffer.empty=The Stream input buffer is empty. Waiting for more data
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
index cd15cb0f49..24f68c7e24 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -93,6 +93,7 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
private StreamException headerException = null;
private volatile StringBuilder cookieHeader = null;
+ private volatile boolean hostHeaderSeen = false;
private Object pendingWindowUpdateForStreamLock = new Object();
private int pendingWindowUpdateForStream = 0;
@@ -385,20 +386,7 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
}
case ":authority": {
if (coyoteRequest.serverName().isNull()) {
- int i;
- try {
- i = Host.parse(value);
- } catch (IllegalArgumentException iae) {
- // Host value invalid
- throw new HpackException(sm.getString("stream.header.invalid",
- getConnectionId(), getIdAsString(), ":authority", value));
- }
- if (i > -1) {
- coyoteRequest.serverName().setString(value.substring(0, i));
- coyoteRequest.setServerPort(Integer.parseInt(value.substring(i + 1)));
- } else {
- coyoteRequest.serverName().setString(value);
- }
+ parseAuthority(value, false);
} else {
throw new HpackException(sm.getString("stream.header.duplicate",
getConnectionId(), getIdAsString(), ":authority" ));
@@ -416,6 +404,22 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
cookieHeader.append(value);
break;
}
+ case "host": {
+ if (coyoteRequest.serverName().isNull()) {
+ // No :authority header. This is first host header. Use it.
+ hostHeaderSeen = true;
+ parseAuthority(value, true);
+ } else if (!hostHeaderSeen) {
+ // First host header - must be consistent with :authority
+ hostHeaderSeen = true;
+ compareAuthority(value);
+ } else {
+ // Multiple hosts headers - illegal
+ throw new HpackException(sm.getString("stream.header.duplicate",
+ getConnectionId(), getIdAsString(), "host" ));
+ }
+ break;
+ }
default: {
if (headerState == HEADER_STATE_TRAILER &&
!handler.getProtocol().isTrailerHeaderAllowed(name)) {
@@ -441,6 +445,45 @@ class Stream extends AbstractNonZeroStream implements HeaderEmitter {
}
+ private void parseAuthority(String value, boolean host) throws HpackException {
+ int i;
+ try {
+ i = Host.parse(value);
+ } catch (IllegalArgumentException iae) {
+ // Host value invalid
+ throw new HpackException(sm.getString("stream.header.invalid",
+ getConnectionId(), getIdAsString(), host ? "host" : ":authority", value));
+ }
+ if (i > -1) {
+ coyoteRequest.serverName().setString(value.substring(0, i));
+ coyoteRequest.setServerPort(Integer.parseInt(value.substring(i + 1)));
+ } else {
+ coyoteRequest.serverName().setString(value);
+ }
+ }
+
+
+ private void compareAuthority(String value) throws HpackException {
+ int i;
+ try {
+ i = Host.parse(value);
+ } catch (IllegalArgumentException iae) {
+ // Host value invalid
+ throw new HpackException(sm.getString("stream.header.invalid",
+ getConnectionId(), getIdAsString(), "host", value));
+ }
+ if (i == -1 && value.equals(coyoteRequest.serverName().getString()) ||
+ i > -1 && ((!value.substring(0, i).equals(coyoteRequest.serverName().getString()) ||
+ Integer.parseInt(value.substring(i + 1)) != coyoteRequest.getServerPort()))) {
+ // Host value inconsistent
+ throw new HpackException(sm.getString("stream.host.inconsistent",
+ getConnectionId(), getIdAsString(), value, coyoteRequest.serverName().getString(),
+ Integer.toString(coyoteRequest.getServerPort())));
+ }
+
+ }
+
+
@Override
public void setHeaderException(StreamException streamException) {
if (headerException == null) {
diff --git a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
index 1fa2b5d15b..3b47dc13ae 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
@@ -245,6 +245,105 @@ public class TestHttp2Section_8_1 extends Http2TestBase {
}
+ @Test
+ public void testHostHeaderValid() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "http"));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ buildGetRequest(headersFrameHeader, headersPayload, null, headers , 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+
+ parser.readFrame(true);
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.contains("3-Header-[:status]-[200]"));
+ }
+
+
+ @Test
+ public void testHostHeaderDuplicate() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "http"));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ buildGetRequest(headersFrameHeader, headersPayload, null, headers , 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+
+ parser.readFrame(true);
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.contains("0-Goaway-[1]-[9]"));
+ }
+
+
+ @Test
+ public void testHostHeaderConsistent() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "http"));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "localhost:" + getPort()));
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ buildGetRequest(headersFrameHeader, headersPayload, null, headers , 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+
+ parser.readFrame(true);
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.contains("3-Header-[:status]-[200]"));
+ }
+
+
+ @Test
+ public void testHostHeaderInconsistent() throws Exception {
+ http2Connect();
+
+ List<Header> headers = new ArrayList<>(4);
+ headers.add(new Header(":method", "GET"));
+ headers.add(new Header(":scheme", "http"));
+ headers.add(new Header(":authority", "localhost:" + getPort()));
+ headers.add(new Header(":path", "/simple"));
+ headers.add(new Header("host", "otherhost:" + getPort()));
+
+ byte[] headersFrameHeader = new byte[9];
+ ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+ buildGetRequest(headersFrameHeader, headersPayload, null, headers , 3);
+
+ writeFrame(headersFrameHeader, headersPayload);
+
+ parser.readFrame(true);
+
+ String trace = output.getTrace();
+ Assert.assertTrue(trace, trace.contains("0-Goaway-[1]-[9]"));
+ }
+
+
private void doInvalidPseudoHeaderTest(List<Header> headers) throws Exception {
http2Connect();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 40e9cbe546..caea8f83bb 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -144,6 +144,19 @@
Ensure HTTP/2 requests that include connection specific headers are
rejected. (markt)
</fix>
+ <fix>
+ When processing HTTP/2 requests, allow a <code>host</code> header to be
+ used in place of an <code>:authority</code> header. (markt)
+ </fix>
+ <fix>
+ When processing HTTP/2 requests, allow a <code>host</code> header and an
+ <code>:authority</code> header to be present providing the are
+ consistent. (markt)
+ </fix>
+ <fix>
+ When processing HTTP/2 requests, reject requests containing multiple
+ <code>host</code> headers. (markt)
+ </fix>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org