jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.8k stars 1.9k forks source link

HTTP requests with large headers are not fully processes and timeout #11837

Open kylef opened 2 months ago

kylef commented 2 months ago

Jetty version(s) Tested on 12.0.9 and 12.0.5

Jetty Environment core

Java version/vendor (use: java -version)

$ java --version
openjdk 21.0.2 2024-01-16

OS type/version

macOS (darwin kernel 23.3.0)

Description

When I create a Jetty server using Jetty 12 when adding support for larger request header sizes. When a request reaches around this size the server doesn't process the entire request and keeps the connection open. After 30 seconds it times out.

Either the request headers are within the limit and should be accepted, or they are not and should return a 431 error.

How to reproduce?

We can make a request to one of the example jetty projects to reproduce.

Setup:

$ git clone https://github.com/jetty/jetty-examples.git
$ cd jetty-examples
$ git checkout 429396f  # latest commit at time of writing

# edit embedded/connectors/src/main/java/examples/ServerConnectorHttps.java
# add "httpsConf.setRequestHeaderSize(128 * 1024);" on line 42

# run ServerConnectorHttps (embedded/connectors/src/main/java/examples/ServerConnectorHttps.java

Perform request against the server:

$ curl https://localhost:8443/ -k -H "Test: $(python3 -c 'print("." * 330972)')" -v

curl: (52) Empty reply from server

After 30 seconds request will timeout.

kylef commented 2 months ago

I've spent a little bit of time debugging this issue, and from what I can tell the HttpParser isn't the cause and I suspect somewhere before the HttpParser this is occurring.

I have written a test case for HttpParser which rules out the parser by matching the input and buffering. However one difference is that the ByteBuffer used in tests differs from the NIO buffers used when running the full server.

For posterity here is a test that shows HttpParser appears to process the request in this form which makes me suspect elsewhere.

diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
index 57db19b528..88bce2d734 100644
--- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
+++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
@@ -306,6 +307,79 @@ public class HttpParserTest
         assertEquals(-1, _headers);
     }

+    void parseBuffer(ByteBuffer buffer, HttpParser parser) {
+        int remaining = buffer.remaining();
+        while (!parser.isState(State.END) && remaining > 0)
+        {
+            int wasRemaining = remaining;
+            parser.parseNext(buffer);
+            remaining = buffer.remaining();
+            if (remaining == wasRemaining)
+                break;
+        }
+    }
+
+    @Test
+    public void test()
+    {
+        // 16384 for each buffer
+        ByteBuffer buffer1 = BufferUtil.toBuffer(
+                "GET / HTTP/1.1\r\n" +
+                        "Host: localhost:8443\r\n" +
+                        "User-Agent: curl/8.4.0\r\n" +
+                        "Accept: */*\r\n" +
+                        "Foo: " + ".".repeat(16304));
+        ByteBuffer buffer2 = BufferUtil.toBuffer(".".repeat(16384));
+        ByteBuffer buffer3 = BufferUtil.toBuffer(".".repeat(16384));
+        ByteBuffer buffer4 = BufferUtil.toBuffer(".".repeat(16384));
+        ByteBuffer buffer5 = BufferUtil.toBuffer(".".repeat(16384));
+        ByteBuffer buffer6 = BufferUtil.toBuffer(".".repeat(16384));
+        ByteBuffer buffer7 = BufferUtil.toBuffer(".".repeat(16384));
+        ByteBuffer buffer8 = BufferUtil.toBuffer(".".repeat(16363) +
+                "\r\n" + "F: a\r\n" + "Connection: ");
+        //130972
+        ByteBuffer buffer10 = BufferUtil.toBuffer(
+                        "close\r\n\r\n");
+
+        HttpParser.RequestHandler handler = new Handler();
+        HttpParser parser = new HttpParser(handler, 128 * 1024);
+
+        if (parser.isState(State.END))
+            parser.reset();
+        if (!parser.isState(State.START))
+            throw new IllegalStateException("!START");
+
+        parseBuffer(buffer1, parser);
+        parseBuffer(buffer2, parser);
+        parseBuffer(buffer3, parser);
+        parseBuffer(buffer4, parser);
+        parseBuffer(buffer5, parser);
+        parseBuffer(buffer6, parser);
+        parseBuffer(buffer7, parser);
+        parseBuffer(buffer8, parser);
+        parseBuffer(buffer10, parser);
+
+        assertTrue(_headerCompleted);
+        assertTrue(_messageCompleted);
+        assertEquals("GET", _methodOrVersion);
+        assertEquals("/", _uriOrStatus);
+        assertEquals("HTTP/1.1", _versionOrReason);
+        assertEquals("Host", _hdr[0]);
+        assertEquals("localhost:8443", _val[0]);
+        assertEquals("User-Agent", _hdr[1]);
+        assertEquals("curl/8.4.0", _val[1]);
+        assertEquals("Accept", _hdr[2]);
+        assertEquals("*/*", _val[2]);
+        assertEquals("Foo", _hdr[3]);
+        assertEquals(130971, _val[3].length());
+        assertEquals("F", _hdr[4]);
+        assertEquals("a", _val[4]);
+        assertEquals("Connection", _hdr[5]);
+        assertEquals("close", _val[5]);
+        assertEquals(5, _headers);
+    }
+
     @ParameterizedTest
     @ValueSource(strings = {"\r\n", "\n"})
     public void testSimple(String eoln)