Skip to content

Commit

Permalink
Fixes #12488 - HTTP/2 headers may not be split in CONTINUATION frames. (
Browse files Browse the repository at this point in the history 
#12489)

* Fixed headers generation, taking into account maxHeaderListSize and maxFrameSize correctly.
* Introduced HTTP2Client.maxRequestHeadersSize.
* Initialized HpackEncoder.maxHeaderListSize.
* Introduced HpackContext.DEFAULT_MAX_HEADER_LIST_SIZE.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
@sbordet
sbordet authored Nov 12, 2024
1 parent 5efb2fd commit 6aaaa15
Show file tree
Hide file tree
Showing 13 changed files with 109 additions and 42 deletions.
1 change: 1 addition & 0 deletions .../src/main/java/org/eclipse/jetty/http2/client/transport/HttpClientTransportOverHTTP2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ static void configure(HttpClient httpClient, HTTP2Client http2Client)
http2Client.setUseOutputDirectByteBuffers(httpClient.isUseOutputDirectByteBuffers());
http2Client.setConnectBlocking(httpClient.isConnectBlocking());
http2Client.setBindAddress(httpClient.getBindAddress());
http2Client.setMaxRequestHeadersSize(httpClient.getRequestBufferSize());
http2Client.setMaxResponseHeadersSize(httpClient.getMaxResponseHeadersSize());
}

Expand Down
12 changes: 12 additions & 0 deletions ...ty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ public class HTTP2Client extends ContainerLifeCycle implements AutoCloseable
private int maxDecoderTableCapacity = HpackContext.DEFAULT_MAX_TABLE_CAPACITY;
private int maxEncoderTableCapacity = HpackContext.DEFAULT_MAX_TABLE_CAPACITY;
private int maxHeaderBlockFragment = 0;
private int maxRequestHeadersSize = 8 * 1024;
private int maxResponseHeadersSize = 8 * 1024;
private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F);
private long streamIdleTimeout;
Expand Down Expand Up @@ -356,6 +357,17 @@ public void setMaxHeaderBlockFragment(int maxHeaderBlockFragment)
this.maxHeaderBlockFragment = maxHeaderBlockFragment;
}

@ManagedAttribute("The max size of request headers")
public int getMaxRequestHeadersSize()
{
return maxRequestHeadersSize;
}

public void setMaxRequestHeadersSize(int maxRequestHeadersSize)
{
this.maxRequestHeadersSize = maxRequestHeadersSize;
}

@ManagedAttribute("The max size of response headers")
public int getMaxResponseHeadersSize()
{
Expand Down
3 changes: 2 additions & 1 deletion ...tp2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@ public Connection newConnection(EndPoint endPoint, Map<String, Object> context)
Promise<Session> sessionPromise = (Promise<Session>)context.get(SESSION_PROMISE_CONTEXT_KEY);

Generator generator = new Generator(bufferPool, client.isUseOutputDirectByteBuffers(), client.getMaxHeaderBlockFragment());
generator.getHpackEncoder().setMaxHeaderListSize(client.getMaxRequestHeadersSize());

FlowControlStrategy flowControl = client.getFlowControlStrategyFactory().newFlowControlStrategy();

Parser parser = new Parser(bufferPool, client.getMaxResponseHeadersSize());
parser.setMaxFrameSize(client.getMaxFrameSize());
parser.setMaxSettingsKeys(client.getMaxSettingsKeys());

HTTP2ClientSession session = new HTTP2ClientSession(client.getScheduler(), endPoint, parser, generator, listener, flowControl);
Expand Down
8 changes: 6 additions & 2 deletions ...p2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/generator/FrameGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.http2.frames.Frame;
import org.eclipse.jetty.http2.frames.FrameType;
import org.eclipse.jetty.http2.hpack.HpackContext;
import org.eclipse.jetty.http2.hpack.HpackEncoder;
import org.eclipse.jetty.http2.hpack.HpackException;
import org.eclipse.jetty.io.ByteBufferPool;
Expand Down Expand Up @@ -50,9 +51,12 @@ public boolean isUseDirectByteBuffers()
return headerGenerator.isUseDirectByteBuffers();
}

protected RetainableByteBuffer encode(HpackEncoder encoder, MetaData metaData, int maxFrameSize) throws HpackException
protected RetainableByteBuffer encode(HpackEncoder encoder, MetaData metaData) throws HpackException
{
RetainableByteBuffer hpacked = headerGenerator.getByteBufferPool().acquire(maxFrameSize, isUseDirectByteBuffers());
int bufferSize = encoder.getMaxHeaderListSize();
if (bufferSize <= 0)
bufferSize = HpackContext.DEFAULT_MAX_HEADER_LIST_SIZE;
RetainableByteBuffer hpacked = headerGenerator.getByteBufferPool().acquire(bufferSize, isUseDirectByteBuffers());
try
{
ByteBuffer byteBuffer = hpacked.getByteBuffer();
Expand Down
46 changes: 20 additions & 26 deletions .../jetty-http2-common/src/main/java/org/eclipse/jetty/http2/generator/HeadersGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,22 +59,24 @@ public int generateHeaders(ByteBufferPool.Accumulator accumulator, int streamId,
throw new IllegalArgumentException("Invalid stream id: " + streamId);

int flags = Flags.NONE;

if (priority != null)
flags = Flags.PRIORITY;
if (endStream)
flags |= Flags.END_STREAM;

RetainableByteBuffer hpack = encode(encoder, metaData, getMaxFrameSize());
RetainableByteBuffer hpack = encode(encoder, metaData);
ByteBuffer hpackByteBuffer = hpack.getByteBuffer();
int hpackLength = hpackByteBuffer.position();
BufferUtil.flipToFlush(hpackByteBuffer, 0);
int hpackLength = hpackByteBuffer.remaining();

int maxHeaderBlock = getMaxFrameSize();
if (maxHeaderBlockFragment > 0)
maxHeaderBlock = Math.min(maxHeaderBlock, maxHeaderBlockFragment);

// Split into CONTINUATION frames if necessary.
if (maxHeaderBlockFragment > 0 && hpackLength > maxHeaderBlockFragment)
if (hpackLength > maxHeaderBlock)
{
if (endStream)
flags |= Flags.END_STREAM;

int length = maxHeaderBlockFragment;
int length = maxHeaderBlock;
if (priority != null)
length += PriorityFrame.PRIORITY_LENGTH;

Expand All @@ -83,30 +85,24 @@ public int generateHeaders(ByteBufferPool.Accumulator accumulator, int streamId,
generatePriority(headerByteBuffer, priority);
BufferUtil.flipToFlush(headerByteBuffer, 0);
accumulator.append(header);
hpackByteBuffer.limit(maxHeaderBlockFragment);
accumulator.append(RetainableByteBuffer.wrap(hpackByteBuffer.slice()));
accumulator.append(RetainableByteBuffer.wrap(hpackByteBuffer.slice(0, maxHeaderBlock)));

int totalLength = Frame.HEADER_LENGTH + length;

int position = maxHeaderBlockFragment;
int limit = position + maxHeaderBlockFragment;
while (limit < hpackLength)
int position = maxHeaderBlock;
while (position + maxHeaderBlock < hpackLength)
{
hpackByteBuffer.position(position).limit(limit);
header = generateHeader(FrameType.CONTINUATION, maxHeaderBlockFragment, Flags.NONE, streamId);
headerByteBuffer = header.getByteBuffer();
BufferUtil.flipToFlush(headerByteBuffer, 0);
header = generateHeader(FrameType.CONTINUATION, maxHeaderBlock, Flags.NONE, streamId);
BufferUtil.flipToFlush(header.getByteBuffer(), 0);
accumulator.append(header);
accumulator.append(RetainableByteBuffer.wrap(hpackByteBuffer.slice()));
position += maxHeaderBlockFragment;
limit += maxHeaderBlockFragment;
totalLength += Frame.HEADER_LENGTH + maxHeaderBlockFragment;
accumulator.append(RetainableByteBuffer.wrap(hpackByteBuffer.slice(position, maxHeaderBlock)));
position += maxHeaderBlock;
totalLength += Frame.HEADER_LENGTH + maxHeaderBlock;
}
hpackByteBuffer.position(position);

hpackByteBuffer.position(position).limit(hpackLength);
header = generateHeader(FrameType.CONTINUATION, hpack.remaining(), Flags.END_HEADERS, streamId);
headerByteBuffer = header.getByteBuffer();
BufferUtil.flipToFlush(headerByteBuffer, 0);
BufferUtil.flipToFlush(header.getByteBuffer(), 0);
accumulator.append(header);
accumulator.append(hpack);
totalLength += Frame.HEADER_LENGTH + hpack.remaining();
Expand All @@ -116,8 +112,6 @@ public int generateHeaders(ByteBufferPool.Accumulator accumulator, int streamId,
else
{
flags |= Flags.END_HEADERS;
if (endStream)
flags |= Flags.END_STREAM;

int length = hpackLength;
if (priority != null)
Expand Down
16 changes: 8 additions & 8 deletions ...ty-http2-common/src/main/java/org/eclipse/jetty/http2/generator/PushPromiseGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,17 @@ public int generatePushPromise(ByteBufferPool.Accumulator accumulator, int strea
if (promisedStreamId < 0)
throw new IllegalArgumentException("Invalid promised stream id: " + promisedStreamId);

int maxFrameSize = getMaxFrameSize();
// The promised streamId space.
int extraSpace = 4;
maxFrameSize -= extraSpace;

RetainableByteBuffer hpack = encode(encoder, metaData, maxFrameSize);
RetainableByteBuffer hpack = encode(encoder, metaData);
ByteBuffer hpackByteBuffer = hpack.getByteBuffer();
int hpackLength = hpackByteBuffer.position();
BufferUtil.flipToFlush(hpackByteBuffer, 0);
int hpackLength = hpackByteBuffer.remaining();

// No support for splitting in CONTINUATION frames,
// also PushPromiseBodyParser does not support it.

int length = hpackLength + extraSpace;
// The promised streamId length.
int promisedStreamIdLength = 4;
int length = hpackLength + promisedStreamIdLength;
int flags = Flags.END_HEADERS;

RetainableByteBuffer header = generateHeader(FrameType.PUSH_PROMISE, length, flags, streamId);
Expand Down
4 changes: 4 additions & 0 deletions ...2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/SettingsBodyParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@ protected boolean onSettings(ByteBuffer buffer, Map<Integer, Integer> settings)
if (maxFrameSize != null && (maxFrameSize < Frame.DEFAULT_MAX_SIZE || maxFrameSize > Frame.MAX_MAX_SIZE))
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_max_frame_size");

Integer maxHeaderListSize = settings.get(SettingsFrame.MAX_HEADER_LIST_SIZE);
if (maxHeaderListSize != null && maxHeaderListSize <= 0)
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_max_header_list_size");

SettingsFrame frame = new SettingsFrame(settings, hasFlag(Flags.ACK));
return onSettings(buffer, frame);
}
Expand Down
1 change: 1 addition & 0 deletions ...tty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ public class HpackContext
private static final StaticEntry[] __staticTable = new StaticEntry[STATIC_TABLE.length];
public static final int STATIC_SIZE = STATIC_TABLE.length - 1;
public static final int DEFAULT_MAX_TABLE_CAPACITY = 4096;
public static final int DEFAULT_MAX_HEADER_LIST_SIZE = 4096;

static
{
Expand Down
1 change: 1 addition & 0 deletions ...tty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ public HpackEncoder()
_debug = LOG.isDebugEnabled();
setMaxTableCapacity(HpackContext.DEFAULT_MAX_TABLE_CAPACITY);
setTableCapacity(HpackContext.DEFAULT_MAX_TABLE_CAPACITY);
setMaxHeaderListSize(HpackContext.DEFAULT_MAX_HEADER_LIST_SIZE);
}

public int getMaxTableCapacity()
Expand Down
7 changes: 5 additions & 2 deletions ...er/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,13 @@ protected Map<Integer, Integer> newSettings()
int maxTableSize = getMaxDecoderTableCapacity();
if (maxTableSize != HpackContext.DEFAULT_MAX_TABLE_CAPACITY)
settings.put(SettingsFrame.HEADER_TABLE_SIZE, maxTableSize);
settings.put(SettingsFrame.MAX_CONCURRENT_STREAMS, getMaxConcurrentStreams());
int initialStreamRecvWindow = getInitialStreamRecvWindow();
if (initialStreamRecvWindow != FlowControlStrategy.DEFAULT_WINDOW_SIZE)
settings.put(SettingsFrame.INITIAL_WINDOW_SIZE, initialStreamRecvWindow);
settings.put(SettingsFrame.MAX_CONCURRENT_STREAMS, getMaxConcurrentStreams());
int maxFrameSize = getMaxFrameSize();
if (maxFrameSize > Frame.DEFAULT_MAX_SIZE)
settings.put(SettingsFrame.MAX_FRAME_SIZE, maxFrameSize);
int maxHeadersSize = getHttpConfiguration().getRequestHeaderSize();
if (maxHeadersSize > 0)
settings.put(SettingsFrame.MAX_HEADER_LIST_SIZE, maxHeadersSize);
Expand All @@ -304,10 +307,10 @@ public Connection newConnection(Connector connector, EndPoint endPoint)

Generator generator = new Generator(connector.getByteBufferPool(), isUseOutputDirectByteBuffers(), getMaxHeaderBlockFragment());
generator.getHpackEncoder().setMaxHeaderListSize(getHttpConfiguration().getResponseHeaderSize());

FlowControlStrategy flowControl = getFlowControlStrategyFactory().newFlowControlStrategy();

ServerParser parser = newServerParser(connector, getRateControlFactory().newRateControl(endPoint));
parser.setMaxFrameSize(getMaxFrameSize());
parser.setMaxSettingsKeys(getMaxSettingsKeys());

HTTP2ServerSession session = new HTTP2ServerSession(connector.getScheduler(), endPoint, parser, generator, listener, flowControl);
Expand Down
48 changes: 46 additions & 2 deletions .../jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@
import org.eclipse.jetty.http2.frames.ResetFrame;
import org.eclipse.jetty.http2.frames.SettingsFrame;
import org.eclipse.jetty.http2.hpack.HpackException;
import org.eclipse.jetty.http2.server.AbstractHTTP2ServerConnectionFactory;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.util.BufferUtil;
Expand Down Expand Up @@ -1051,7 +1053,7 @@ public void onClose(Session session, GoAwayFrame frame, Callback callback)
.put("custom", value);
MetaData.Request metaData = newRequest("GET", requestFields);
HeadersFrame request = new HeadersFrame(metaData, null, true);
session.newStream(request, new FuturePromise<>(), new Stream.Listener(){});
session.newStream(request, new FuturePromise<>(), new Stream.Listener() {});

// Test failure and close reason on client.
String closeReason = clientCloseReasonFuture.get(5, TimeUnit.SECONDS);
Expand Down Expand Up @@ -1125,7 +1127,7 @@ public void onClose(Session session, GoAwayFrame frame, Callback callback)
Session session = newClientSession(listener);
MetaData.Request metaData = newRequest("GET", HttpFields.EMPTY);
HeadersFrame request = new HeadersFrame(metaData, null, true);
session.newStream(request, new FuturePromise<>(), new Stream.Listener(){});
session.newStream(request, new FuturePromise<>(), new Stream.Listener() {});

// Test failure and close reason on server.
String closeReason = serverCloseReasonFuture.get(5, TimeUnit.SECONDS);
Expand Down Expand Up @@ -1294,6 +1296,48 @@ public Stream.Listener onNewStream(Stream stream, HeadersFrame frame)
assertTrue(latch.await(5, TimeUnit.SECONDS));
}

@Test
public void testLargeRequestHeaders() throws Exception
{
int maxHeadersSize = 20 * 1024;
HttpConfiguration httpConfig = new HttpConfiguration();
httpConfig.setRequestHeaderSize(2 * maxHeadersSize);
start(new Handler.Abstract()
{
@Override
public boolean handle(Request request, Response response, Callback callback)
{
callback.succeeded();
return true;
}
}, httpConfig);
connector.getBean(AbstractHTTP2ServerConnectionFactory.class).setMaxFrameSize(17 * 1024);
http2Client.setMaxFrameSize(18 * 1024);

Session session = newClientSession(new Session.Listener() {});

CountDownLatch responseLatch = new CountDownLatch(1);
HttpFields.Mutable headers = HttpFields.build()
// Symbol "<" needs 15 bits to be Huffman encoded,
// while letters/numbers take typically less than
// 8 bits, and here we want to exceed maxHeadersSize.
.put("X-Large", "<".repeat(maxHeadersSize));
MetaData.Request request = newRequest("GET", headers);
session.newStream(new HeadersFrame(request, null, true), new Stream.Listener()
{
@Override
public void onHeaders(Stream stream, HeadersFrame frame)
{
assertTrue(frame.isEndStream());
MetaData.Response response = (MetaData.Response)frame.getMetaData();
assertEquals(HttpStatus.OK_200, response.getStatus());
responseLatch.countDown();
}
}).get(5, TimeUnit.SECONDS);

assertTrue(responseLatch.await(5, TimeUnit.SECONDS));
}

private static void sleep(long time)
{
try
Expand Down
1 change: 1 addition & 0 deletions ...2-tests/src/test/java/org/eclipse/jetty/http2/tests/HttpClientTransportOverHTTP2Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ private void testPropertiesAreForwarded(HTTP2Client http2Client, HttpClientTrans
assertEquals(httpClient.getIdleTimeout(), http2Client.getIdleTimeout());
assertEquals(httpClient.isUseInputDirectByteBuffers(), http2Client.isUseInputDirectByteBuffers());
assertEquals(httpClient.isUseOutputDirectByteBuffers(), http2Client.isUseOutputDirectByteBuffers());
assertEquals(httpClient.getRequestBufferSize(), http2Client.getMaxRequestHeadersSize());
assertEquals(httpClient.getMaxResponseHeadersSize(), http2Client.getMaxResponseHeadersSize());
}
assertTrue(http2Client.isStopped());
Expand Down
3 changes: 2 additions & 1 deletion jetty-core/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ReverseProxyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ public void testServerResponseHeadersTooLargeForServerConfiguration(HttpVersion
@Override
public boolean handle(Request request, Response response, Callback callback)
{
response.getHeaders().put("X-Large", "A".repeat(maxResponseHeadersSize));
// Use "+" because in HTTP/2 is Huffman encoded in more than 8 bits.
response.getHeaders().put("X-Large", "+".repeat(maxResponseHeadersSize));

// With HTTP/1.1, calling response.write() would fail the Handler callback
// which would trigger ErrorHandler and result in a 500 to the proxy.
Expand Down

0 comments on commit 6aaaa15

Please sign in to comment.