From fd2407c72fad2e058ac9799d160589eac22d3bf9 Mon Sep 17 00:00:00 2001 From: mszabo-wikia Date: Wed, 19 Jan 2022 18:56:27 +0100 Subject: [PATCH] Clarify that requestHeaderSize is a cumulative limit (#7417) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Clarify that requestHeaderSize is a cumulative limit HttpConfiguration documents the requestHeaderSize configuration option as being a limit on the size of a single request header, but it is in fact a limit on the cumulative size of all request headers as well as the request URI. This patch updates the documentation accordingly, and adds test cases for the HTTP/1.x and HTTP/2 parsers to verify the behavior. NB.: the HTTP/3 parser and configuration seem to correctly document this option as being a global limit on header size. * Improve requestHeaderSize tests and documentation per review Signed-off-by: Máté Szabó --- .../eclipse/jetty/http/HttpParserTest.java | 35 +++++++ .../frames/HeadersTooLargeParseTest.java | 96 +++++++++++++++++++ .../jetty/server/HttpConfiguration.java | 6 +- 3 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/HeadersTooLargeParseTest.java diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java index addce5e9938..7be1a93cc96 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java @@ -2104,6 +2104,41 @@ public class HttpParserTest assertNull(_bad); } + @Test + public void testRequestMaxHeaderBytesURITooLong() + { + ByteBuffer buffer = BufferUtil.toBuffer( + "GET /long/nested/path/uri HTTP/1.1\r\n" + + "Host: example.com\r\n" + + "Connection: close\r\n" + + "\r\n"); + + int maxHeaderBytes = 5; + HttpParser.RequestHandler handler = new Handler(); + HttpParser parser = new HttpParser(handler, maxHeaderBytes); + + parseAll(parser, buffer); + assertEquals("414", _bad); + } + + @Test + public void testRequestMaxHeaderBytesCumulative() + { + ByteBuffer buffer = BufferUtil.toBuffer( + "GET /nested/path/uri HTTP/1.1\r\n" + + "Host: example.com\r\n" + + "X-Large-Header: lorem-ipsum-dolor-sit\r\n" + + "Connection: close\r\n" + + "\r\n"); + + int maxHeaderBytes = 64; + HttpParser.RequestHandler handler = new Handler(); + HttpParser parser = new HttpParser(handler, maxHeaderBytes); + + parseAll(parser, buffer); + assertEquals("431", _bad); + } + @Test @SuppressWarnings("ReferenceEquality") public void testCachedField() diff --git a/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/HeadersTooLargeParseTest.java b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/HeadersTooLargeParseTest.java new file mode 100644 index 00000000000..94e9ecaa94a --- /dev/null +++ b/jetty-http2/http2-common/src/test/java/org/eclipse/jetty/http2/frames/HeadersTooLargeParseTest.java @@ -0,0 +1,96 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.http2.frames; + +import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.UnaryOperator; + +import org.eclipse.jetty.http.HostPortHttpField; +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpScheme; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.ErrorCode; +import org.eclipse.jetty.http2.generator.HeaderGenerator; +import org.eclipse.jetty.http2.generator.HeadersGenerator; +import org.eclipse.jetty.http2.hpack.HpackEncoder; +import org.eclipse.jetty.http2.hpack.HpackException; +import org.eclipse.jetty.http2.parser.Parser; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.MappedByteBufferPool; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class HeadersTooLargeParseTest +{ + private final ByteBufferPool byteBufferPool = new MappedByteBufferPool(); + + @Test + public void testProtocolErrorURITooLong() throws HpackException + { + HttpFields fields = HttpFields.build() + .put("B", "test"); + MetaData.Request metaData = new MetaData.Request("GET", HttpScheme.HTTP.asString(), new HostPortHttpField("localhost:8080"), "/nested/uri/path/too/long", HttpVersion.HTTP_2, fields, -1); + int maxHeaderSize = 48; + + assertProtocolError(maxHeaderSize, metaData); + } + + @Test + public void testProtocolErrorCumulativeHeaderSize() throws HpackException + { + HttpFields fields = HttpFields.build() + .put("X-Large-Header", "lorem-ipsum-dolor-sit") + .put("X-Other-Header", "test"); + MetaData.Request metaData = new MetaData.Request("GET", HttpScheme.HTTP.asString(), new HostPortHttpField("localhost:8080"), "/", HttpVersion.HTTP_2, fields, -1); + int maxHeaderSize = 64; + + assertProtocolError(maxHeaderSize, metaData); + } + + private void assertProtocolError(int maxHeaderSize, MetaData.Request metaData) throws HpackException + { + HeadersGenerator generator = new HeadersGenerator(new HeaderGenerator(), new HpackEncoder()); + + AtomicInteger failure = new AtomicInteger(); + Parser parser = new Parser(byteBufferPool, new Parser.Listener.Adapter() + { + @Override + public void onConnectionFailure(int error, String reason) + { + failure.set(error); + } + }, 4096, maxHeaderSize); + parser.init(UnaryOperator.identity()); + + int streamId = 48; + ByteBufferPool.Lease lease = new ByteBufferPool.Lease(byteBufferPool); + PriorityFrame priorityFrame = new PriorityFrame(streamId, 3 * streamId, 200, true); + int len = generator.generateHeaders(lease, streamId, metaData, priorityFrame, true); + + for (ByteBuffer buffer : lease.getByteBuffers()) + { + while (buffer.hasRemaining() && failure.get() == 0) + { + parser.parse(buffer); + } + } + + assertTrue(len > maxHeaderSize); + assertEquals(ErrorCode.PROTOCOL_ERROR.code, failure.get()); + } +} diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java index 9adc05c2018..833f37409ef 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java @@ -193,7 +193,7 @@ public class HttpConfiguration implements Dumpable return _outputAggregationSize; } - @ManagedAttribute("The maximum allowed size in bytes for an HTTP request header") + @ManagedAttribute("The maximum allowed size in bytes for the HTTP request line and HTTP request headers") public int getRequestHeaderSize() { return _requestHeaderSize; @@ -406,11 +406,13 @@ public class HttpConfiguration implements Dumpable } /** + *

Sets the maximum allowed size in bytes for the HTTP request line and HTTP request headers.

+ * *

Larger headers will allow for more and/or larger cookies plus larger form content encoded * in a URL. However, larger headers consume more memory and can make a server more vulnerable to denial of service * attacks.

* - * @param requestHeaderSize the maximum size in bytes of the request header + * @param requestHeaderSize the maximum allowed size in bytes for the HTTP request line and HTTP request headers */ public void setRequestHeaderSize(int requestHeaderSize) {