This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.0.x by this push:
new 177e25e67a Align host header processing with RFC 9113
177e25e67a is described below
commit 177e25e67af4f070add1c72e9cf42a6c9de1cbac
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 44dc56d105..13cc3a36f1 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -92,6 +92,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;
@@ -380,20 +381,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" ));
@@ -411,6 +399,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)) {
@@ -436,6 +440,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 4be7c73084..53035a335c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -135,6 +135,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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
