Clarify that requestHeaderSize is a cumulative limit (#7417)
* 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ó <mszabo@wikia-inc.com>
This commit is contained in:
parent
8cc9802dbd
commit
fd2407c72f
|
@ -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()
|
||||
|
|
|
@ -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());
|
||||
}
|
||||
}
|
|
@ -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
|
|||
}
|
||||
|
||||
/**
|
||||
* <p>Sets the maximum allowed size in bytes for the HTTP request line and HTTP request headers.</p>
|
||||
*
|
||||
* <p>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.</p>
|
||||
*
|
||||
* @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)
|
||||
{
|
||||
|
|
Loading…
Reference in New Issue