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:20:53 UTC

[tomcat] branch 9.0.x updated: 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 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 205289b10e Align host header processing with RFC 9113
205289b10e is described below

commit 205289b10e998df0e0fd21c3ad8fb6fbcbc8ec2e
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 1ac42135f7..b9a2f77f12 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 da694e4018..ef57e98cef 100644
--- a/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
+++ b/test/org/apache/coyote/http2/TestHttp2Section_8_1.java
@@ -244,6 +244,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 cb72bd83ea..a91b637275 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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org