Skip to content

Commit 6aaaa15

Browse files
sbordetsbordet
authored
Fixes jetty#12488 - HTTP/2 headers may not be split in CONTINUATION frames. (jetty#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 <[email protected]>
1 parent 5efb2fd commit 6aaaa15

File tree

13 files changed

+109
-42
lines changed

13 files changed

+109
-42
lines changed

‎jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/HttpClientTransportOverHTTP2.java

+1
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ static void configure(HttpClient httpClient, HTTP2Client http2Client)
103103
http2Client.setUseOutputDirectByteBuffers(httpClient.isUseOutputDirectByteBuffers());
104104
http2Client.setConnectBlocking(httpClient.isConnectBlocking());
105105
http2Client.setBindAddress(httpClient.getBindAddress());
106+
http2Client.setMaxRequestHeadersSize(httpClient.getRequestBufferSize());
106107
http2Client.setMaxResponseHeadersSize(httpClient.getMaxResponseHeadersSize());
107108
}
108109

‎jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2Client.java

+12
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ public class HTTP2Client extends ContainerLifeCycle implements AutoCloseable
112112
private int maxDecoderTableCapacity = HpackContext.DEFAULT_MAX_TABLE_CAPACITY;
113113
private int maxEncoderTableCapacity = HpackContext.DEFAULT_MAX_TABLE_CAPACITY;
114114
private int maxHeaderBlockFragment = 0;
115+
private int maxRequestHeadersSize = 8 * 1024;
115116
private int maxResponseHeadersSize = 8 * 1024;
116117
private FlowControlStrategy.Factory flowControlStrategyFactory = () -> new BufferingFlowControlStrategy(0.5F);
117118
private long streamIdleTimeout;
@@ -356,6 +357,17 @@ public void setMaxHeaderBlockFragment(int maxHeaderBlockFragment)
356357
this.maxHeaderBlockFragment = maxHeaderBlockFragment;
357358
}
358359

360+
@ManagedAttribute("The max size of request headers")
361+
public int getMaxRequestHeadersSize()
362+
{
363+
return maxRequestHeadersSize;
364+
}
365+
366+
public void setMaxRequestHeadersSize(int maxRequestHeadersSize)
367+
{
368+
this.maxRequestHeadersSize = maxRequestHeadersSize;
369+
}
370+
359371
@ManagedAttribute("The max size of response headers")
360372
public int getMaxResponseHeadersSize()
361373
{

‎jetty-core/jetty-http2/jetty-http2-client/src/main/java/org/eclipse/jetty/http2/client/HTTP2ClientConnectionFactory.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,11 @@ public Connection newConnection(EndPoint endPoint, Map<String, Object> context)
5454
Promise<Session> sessionPromise = (Promise<Session>)context.get(SESSION_PROMISE_CONTEXT_KEY);
5555

5656
Generator generator = new Generator(bufferPool, client.isUseOutputDirectByteBuffers(), client.getMaxHeaderBlockFragment());
57+
generator.getHpackEncoder().setMaxHeaderListSize(client.getMaxRequestHeadersSize());
58+
5759
FlowControlStrategy flowControl = client.getFlowControlStrategyFactory().newFlowControlStrategy();
5860

5961
Parser parser = new Parser(bufferPool, client.getMaxResponseHeadersSize());
60-
parser.setMaxFrameSize(client.getMaxFrameSize());
6162
parser.setMaxSettingsKeys(client.getMaxSettingsKeys());
6263

6364
HTTP2ClientSession session = new HTTP2ClientSession(client.getScheduler(), endPoint, parser, generator, listener, flowControl);

‎jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/generator/FrameGenerator.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.eclipse.jetty.http.MetaData;
1919
import org.eclipse.jetty.http2.frames.Frame;
2020
import org.eclipse.jetty.http2.frames.FrameType;
21+
import org.eclipse.jetty.http2.hpack.HpackContext;
2122
import org.eclipse.jetty.http2.hpack.HpackEncoder;
2223
import org.eclipse.jetty.http2.hpack.HpackException;
2324
import org.eclipse.jetty.io.ByteBufferPool;
@@ -50,9 +51,12 @@ public boolean isUseDirectByteBuffers()
5051
return headerGenerator.isUseDirectByteBuffers();
5152
}
5253

53-
protected RetainableByteBuffer encode(HpackEncoder encoder, MetaData metaData, int maxFrameSize) throws HpackException
54+
protected RetainableByteBuffer encode(HpackEncoder encoder, MetaData metaData) throws HpackException
5455
{
55-
RetainableByteBuffer hpacked = headerGenerator.getByteBufferPool().acquire(maxFrameSize, isUseDirectByteBuffers());
56+
int bufferSize = encoder.getMaxHeaderListSize();
57+
if (bufferSize <= 0)
58+
bufferSize = HpackContext.DEFAULT_MAX_HEADER_LIST_SIZE;
59+
RetainableByteBuffer hpacked = headerGenerator.getByteBufferPool().acquire(bufferSize, isUseDirectByteBuffers());
5660
try
5761
{
5862
ByteBuffer byteBuffer = hpacked.getByteBuffer();

‎jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/generator/HeadersGenerator.java

+20-26
Original file line numberDiff line numberDiff line change
@@ -59,22 +59,24 @@ public int generateHeaders(ByteBufferPool.Accumulator accumulator, int streamId,
5959
throw new IllegalArgumentException("Invalid stream id: " + streamId);
6060

6161
int flags = Flags.NONE;
62-
6362
if (priority != null)
6463
flags = Flags.PRIORITY;
64+
if (endStream)
65+
flags |= Flags.END_STREAM;
6566

66-
RetainableByteBuffer hpack = encode(encoder, metaData, getMaxFrameSize());
67+
RetainableByteBuffer hpack = encode(encoder, metaData);
6768
ByteBuffer hpackByteBuffer = hpack.getByteBuffer();
68-
int hpackLength = hpackByteBuffer.position();
6969
BufferUtil.flipToFlush(hpackByteBuffer, 0);
70+
int hpackLength = hpackByteBuffer.remaining();
71+
72+
int maxHeaderBlock = getMaxFrameSize();
73+
if (maxHeaderBlockFragment > 0)
74+
maxHeaderBlock = Math.min(maxHeaderBlock, maxHeaderBlockFragment);
7075

7176
// Split into CONTINUATION frames if necessary.
72-
if (maxHeaderBlockFragment > 0 && hpackLength > maxHeaderBlockFragment)
77+
if (hpackLength > maxHeaderBlock)
7378
{
74-
if (endStream)
75-
flags |= Flags.END_STREAM;
76-
77-
int length = maxHeaderBlockFragment;
79+
int length = maxHeaderBlock;
7880
if (priority != null)
7981
length += PriorityFrame.PRIORITY_LENGTH;
8082

@@ -83,30 +85,24 @@ public int generateHeaders(ByteBufferPool.Accumulator accumulator, int streamId,
8385
generatePriority(headerByteBuffer, priority);
8486
BufferUtil.flipToFlush(headerByteBuffer, 0);
8587
accumulator.append(header);
86-
hpackByteBuffer.limit(maxHeaderBlockFragment);
87-
accumulator.append(RetainableByteBuffer.wrap(hpackByteBuffer.slice()));
88+
accumulator.append(RetainableByteBuffer.wrap(hpackByteBuffer.slice(0, maxHeaderBlock)));
8889

8990
int totalLength = Frame.HEADER_LENGTH + length;
9091

91-
int position = maxHeaderBlockFragment;
92-
int limit = position + maxHeaderBlockFragment;
93-
while (limit < hpackLength)
92+
int position = maxHeaderBlock;
93+
while (position + maxHeaderBlock < hpackLength)
9494
{
95-
hpackByteBuffer.position(position).limit(limit);
96-
header = generateHeader(FrameType.CONTINUATION, maxHeaderBlockFragment, Flags.NONE, streamId);
97-
headerByteBuffer = header.getByteBuffer();
98-
BufferUtil.flipToFlush(headerByteBuffer, 0);
95+
header = generateHeader(FrameType.CONTINUATION, maxHeaderBlock, Flags.NONE, streamId);
96+
BufferUtil.flipToFlush(header.getByteBuffer(), 0);
9997
accumulator.append(header);
100-
accumulator.append(RetainableByteBuffer.wrap(hpackByteBuffer.slice()));
101-
position += maxHeaderBlockFragment;
102-
limit += maxHeaderBlockFragment;
103-
totalLength += Frame.HEADER_LENGTH + maxHeaderBlockFragment;
98+
accumulator.append(RetainableByteBuffer.wrap(hpackByteBuffer.slice(position, maxHeaderBlock)));
99+
position += maxHeaderBlock;
100+
totalLength += Frame.HEADER_LENGTH + maxHeaderBlock;
104101
}
102+
hpackByteBuffer.position(position);
105103

106-
hpackByteBuffer.position(position).limit(hpackLength);
107104
header = generateHeader(FrameType.CONTINUATION, hpack.remaining(), Flags.END_HEADERS, streamId);
108-
headerByteBuffer = header.getByteBuffer();
109-
BufferUtil.flipToFlush(headerByteBuffer, 0);
105+
BufferUtil.flipToFlush(header.getByteBuffer(), 0);
110106
accumulator.append(header);
111107
accumulator.append(hpack);
112108
totalLength += Frame.HEADER_LENGTH + hpack.remaining();
@@ -116,8 +112,6 @@ public int generateHeaders(ByteBufferPool.Accumulator accumulator, int streamId,
116112
else
117113
{
118114
flags |= Flags.END_HEADERS;
119-
if (endStream)
120-
flags |= Flags.END_STREAM;
121115

122116
int length = hpackLength;
123117
if (priority != null)

‎jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/generator/PushPromiseGenerator.java

+8-8
Original file line numberDiff line numberDiff line change
@@ -50,17 +50,17 @@ public int generatePushPromise(ByteBufferPool.Accumulator accumulator, int strea
5050
if (promisedStreamId < 0)
5151
throw new IllegalArgumentException("Invalid promised stream id: " + promisedStreamId);
5252

53-
int maxFrameSize = getMaxFrameSize();
54-
// The promised streamId space.
55-
int extraSpace = 4;
56-
maxFrameSize -= extraSpace;
57-
58-
RetainableByteBuffer hpack = encode(encoder, metaData, maxFrameSize);
53+
RetainableByteBuffer hpack = encode(encoder, metaData);
5954
ByteBuffer hpackByteBuffer = hpack.getByteBuffer();
60-
int hpackLength = hpackByteBuffer.position();
6155
BufferUtil.flipToFlush(hpackByteBuffer, 0);
56+
int hpackLength = hpackByteBuffer.remaining();
57+
58+
// No support for splitting in CONTINUATION frames,
59+
// also PushPromiseBodyParser does not support it.
6260

63-
int length = hpackLength + extraSpace;
61+
// The promised streamId length.
62+
int promisedStreamIdLength = 4;
63+
int length = hpackLength + promisedStreamIdLength;
6464
int flags = Flags.END_HEADERS;
6565

6666
RetainableByteBuffer header = generateHeader(FrameType.PUSH_PROMISE, length, flags, streamId);

‎jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/parser/SettingsBodyParser.java

+4
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,10 @@ protected boolean onSettings(ByteBuffer buffer, Map<Integer, Integer> settings)
209209
if (maxFrameSize != null && (maxFrameSize < Frame.DEFAULT_MAX_SIZE || maxFrameSize > Frame.MAX_MAX_SIZE))
210210
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_max_frame_size");
211211

212+
Integer maxHeaderListSize = settings.get(SettingsFrame.MAX_HEADER_LIST_SIZE);
213+
if (maxHeaderListSize != null && maxHeaderListSize <= 0)
214+
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_max_header_list_size");
215+
212216
SettingsFrame frame = new SettingsFrame(settings, hasFlag(Flags.ACK));
213217
return onSettings(buffer, frame);
214218
}

‎jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackContext.java

+1
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ public class HpackContext
116116
private static final StaticEntry[] __staticTable = new StaticEntry[STATIC_TABLE.length];
117117
public static final int STATIC_SIZE = STATIC_TABLE.length - 1;
118118
public static final int DEFAULT_MAX_TABLE_CAPACITY = 4096;
119+
public static final int DEFAULT_MAX_HEADER_LIST_SIZE = 4096;
119120

120121
static
121122
{

‎jetty-core/jetty-http2/jetty-http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java

+1
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ public HpackEncoder()
107107
_debug = LOG.isDebugEnabled();
108108
setMaxTableCapacity(HpackContext.DEFAULT_MAX_TABLE_CAPACITY);
109109
setTableCapacity(HpackContext.DEFAULT_MAX_TABLE_CAPACITY);
110+
setMaxHeaderListSize(HpackContext.DEFAULT_MAX_HEADER_LIST_SIZE);
110111
}
111112

112113
public int getMaxTableCapacity()

‎jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/AbstractHTTP2ServerConnectionFactory.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -286,10 +286,13 @@ protected Map<Integer, Integer> newSettings()
286286
int maxTableSize = getMaxDecoderTableCapacity();
287287
if (maxTableSize != HpackContext.DEFAULT_MAX_TABLE_CAPACITY)
288288
settings.put(SettingsFrame.HEADER_TABLE_SIZE, maxTableSize);
289+
settings.put(SettingsFrame.MAX_CONCURRENT_STREAMS, getMaxConcurrentStreams());
289290
int initialStreamRecvWindow = getInitialStreamRecvWindow();
290291
if (initialStreamRecvWindow != FlowControlStrategy.DEFAULT_WINDOW_SIZE)
291292
settings.put(SettingsFrame.INITIAL_WINDOW_SIZE, initialStreamRecvWindow);
292-
settings.put(SettingsFrame.MAX_CONCURRENT_STREAMS, getMaxConcurrentStreams());
293+
int maxFrameSize = getMaxFrameSize();
294+
if (maxFrameSize > Frame.DEFAULT_MAX_SIZE)
295+
settings.put(SettingsFrame.MAX_FRAME_SIZE, maxFrameSize);
293296
int maxHeadersSize = getHttpConfiguration().getRequestHeaderSize();
294297
if (maxHeadersSize > 0)
295298
settings.put(SettingsFrame.MAX_HEADER_LIST_SIZE, maxHeadersSize);
@@ -304,10 +307,10 @@ public Connection newConnection(Connector connector, EndPoint endPoint)
304307

305308
Generator generator = new Generator(connector.getByteBufferPool(), isUseOutputDirectByteBuffers(), getMaxHeaderBlockFragment());
306309
generator.getHpackEncoder().setMaxHeaderListSize(getHttpConfiguration().getResponseHeaderSize());
310+
307311
FlowControlStrategy flowControl = getFlowControlStrategyFactory().newFlowControlStrategy();
308312

309313
ServerParser parser = newServerParser(connector, getRateControlFactory().newRateControl(endPoint));
310-
parser.setMaxFrameSize(getMaxFrameSize());
311314
parser.setMaxSettingsKeys(getMaxSettingsKeys());
312315

313316
HTTP2ServerSession session = new HTTP2ServerSession(connector.getScheduler(), endPoint, parser, generator, listener, flowControl);

‎jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2Test.java

+46-2
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@
4545
import org.eclipse.jetty.http2.frames.ResetFrame;
4646
import org.eclipse.jetty.http2.frames.SettingsFrame;
4747
import org.eclipse.jetty.http2.hpack.HpackException;
48+
import org.eclipse.jetty.http2.server.AbstractHTTP2ServerConnectionFactory;
4849
import org.eclipse.jetty.io.Content;
4950
import org.eclipse.jetty.server.Handler;
51+
import org.eclipse.jetty.server.HttpConfiguration;
5052
import org.eclipse.jetty.server.Request;
5153
import org.eclipse.jetty.server.Response;
5254
import org.eclipse.jetty.util.BufferUtil;
@@ -1051,7 +1053,7 @@ public void onClose(Session session, GoAwayFrame frame, Callback callback)
10511053
.put("custom", value);
10521054
MetaData.Request metaData = newRequest("GET", requestFields);
10531055
HeadersFrame request = new HeadersFrame(metaData, null, true);
1054-
session.newStream(request, new FuturePromise<>(), new Stream.Listener(){});
1056+
session.newStream(request, new FuturePromise<>(), new Stream.Listener() {});
10551057

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

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

1299+
@Test
1300+
public void testLargeRequestHeaders() throws Exception
1301+
{
1302+
int maxHeadersSize = 20 * 1024;
1303+
HttpConfiguration httpConfig = new HttpConfiguration();
1304+
httpConfig.setRequestHeaderSize(2 * maxHeadersSize);
1305+
start(new Handler.Abstract()
1306+
{
1307+
@Override
1308+
public boolean handle(Request request, Response response, Callback callback)
1309+
{
1310+
callback.succeeded();
1311+
return true;
1312+
}
1313+
}, httpConfig);
1314+
connector.getBean(AbstractHTTP2ServerConnectionFactory.class).setMaxFrameSize(17 * 1024);
1315+
http2Client.setMaxFrameSize(18 * 1024);
1316+
1317+
Session session = newClientSession(new Session.Listener() {});
1318+
1319+
CountDownLatch responseLatch = new CountDownLatch(1);
1320+
HttpFields.Mutable headers = HttpFields.build()
1321+
// Symbol "<" needs 15 bits to be Huffman encoded,
1322+
// while letters/numbers take typically less than
1323+
// 8 bits, and here we want to exceed maxHeadersSize.
1324+
.put("X-Large", "<".repeat(maxHeadersSize));
1325+
MetaData.Request request = newRequest("GET", headers);
1326+
session.newStream(new HeadersFrame(request, null, true), new Stream.Listener()
1327+
{
1328+
@Override
1329+
public void onHeaders(Stream stream, HeadersFrame frame)
1330+
{
1331+
assertTrue(frame.isEndStream());
1332+
MetaData.Response response = (MetaData.Response)frame.getMetaData();
1333+
assertEquals(HttpStatus.OK_200, response.getStatus());
1334+
responseLatch.countDown();
1335+
}
1336+
}).get(5, TimeUnit.SECONDS);
1337+
1338+
assertTrue(responseLatch.await(5, TimeUnit.SECONDS));
1339+
}
1340+
12971341
private static void sleep(long time)
12981342
{
12991343
try

‎jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HttpClientTransportOverHTTP2Test.java

+1
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ private void testPropertiesAreForwarded(HTTP2Client http2Client, HttpClientTrans
144144
assertEquals(httpClient.getIdleTimeout(), http2Client.getIdleTimeout());
145145
assertEquals(httpClient.isUseInputDirectByteBuffers(), http2Client.isUseInputDirectByteBuffers());
146146
assertEquals(httpClient.isUseOutputDirectByteBuffers(), http2Client.isUseOutputDirectByteBuffers());
147+
assertEquals(httpClient.getRequestBufferSize(), http2Client.getMaxRequestHeadersSize());
147148
assertEquals(httpClient.getMaxResponseHeadersSize(), http2Client.getMaxResponseHeadersSize());
148149
}
149150
assertTrue(http2Client.isStopped());

‎jetty-core/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ReverseProxyTest.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@ public void testServerResponseHeadersTooLargeForServerConfiguration(HttpVersion
152152
@Override
153153
public boolean handle(Request request, Response response, Callback callback)
154154
{
155-
response.getHeaders().put("X-Large", "A".repeat(maxResponseHeadersSize));
155+
// Use "+" because in HTTP/2 is Huffman encoded in more than 8 bits.
156+
response.getHeaders().put("X-Large", "+".repeat(maxResponseHeadersSize));
156157

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

0 commit comments

Comments
 (0)