Skip to content

Commit 0f246d1

Browse files
sbordetsbordet
committed
Fixes #10679 - Review HTTP/2 rate control.
* Bumped the rate control rate from 50 events/s to 128. * Added rate control for all CONTINUATION frames. * Added rate control for invalid PUSH_PROMISE frames. * Added rate control for RST_STREAM frames. * Added rate control for all SETTINGS frames. * Fixed growth of header block accumulation buffer. Signed-off-by: Simone Bordet <[email protected]>
1 parent 2691ad0 commit 0f246d1

File tree

12 files changed

+211
-65
lines changed

12 files changed

+211
-65
lines changed

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

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,28 @@ public boolean parse(ByteBuffer buffer)
8080
int remaining = buffer.remaining();
8181
if (remaining < length)
8282
{
83-
headerBlockFragments.storeFragment(buffer, remaining, false);
83+
ContinuationFrame frame = new ContinuationFrame(getStreamId(), false);
84+
if (!rateControlOnEvent(frame))
85+
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_continuation_frame_rate");
86+
87+
if (!headerBlockFragments.storeFragment(buffer, remaining, false))
88+
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_continuation_stream");
89+
8490
length -= remaining;
8591
break;
8692
}
8793
else
8894
{
89-
boolean last = hasFlag(Flags.END_HEADERS);
90-
headerBlockFragments.storeFragment(buffer, length, last);
95+
boolean endHeaders = hasFlag(Flags.END_HEADERS);
96+
ContinuationFrame frame = new ContinuationFrame(getStreamId(), endHeaders);
97+
if (!rateControlOnEvent(frame))
98+
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_continuation_frame_rate");
99+
100+
if (!headerBlockFragments.storeFragment(buffer, length, endHeaders))
101+
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_continuation_stream");
102+
91103
reset();
92-
if (last)
104+
if (endHeaders)
93105
return onHeaders(buffer);
94106
return true;
95107
}
@@ -107,17 +119,21 @@ private boolean onHeaders(ByteBuffer buffer)
107119
{
108120
ByteBuffer headerBlock = headerBlockFragments.complete();
109121
MetaData metaData = headerBlockParser.parse(headerBlock, headerBlock.remaining());
110-
if (metaData == null)
111-
return true;
122+
HeadersFrame frame = new HeadersFrame(getStreamId(), metaData, headerBlockFragments.getPriorityFrame(), headerBlockFragments.isEndStream());
123+
headerBlockFragments.reset();
124+
112125
if (metaData == HeaderBlockParser.SESSION_FAILURE)
113126
return false;
114-
HeadersFrame frame = new HeadersFrame(getStreamId(), metaData, headerBlockFragments.getPriorityFrame(), headerBlockFragments.isEndStream());
115-
if (metaData == HeaderBlockParser.STREAM_FAILURE)
127+
128+
if (metaData != HeaderBlockParser.STREAM_FAILURE)
129+
{
130+
notifyHeaders(frame);
131+
}
132+
else
116133
{
117134
if (!rateControlOnEvent(frame))
118-
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_continuation_frame_rate");
135+
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_headers_frame_rate");
119136
}
120-
notifyHeaders(frame);
121137
return true;
122138
}
123139

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

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,40 @@
2424

2525
public class HeaderBlockFragments
2626
{
27+
private final int maxCapacity;
2728
private PriorityFrame priorityFrame;
28-
private boolean endStream;
2929
private int streamId;
30+
private boolean endStream;
3031
private ByteBuffer storage;
3132

32-
public void storeFragment(ByteBuffer fragment, int length, boolean last)
33+
public HeaderBlockFragments(int maxCapacity)
34+
{
35+
this.maxCapacity = maxCapacity;
36+
}
37+
38+
void reset()
39+
{
40+
priorityFrame = null;
41+
streamId = 0;
42+
endStream = false;
43+
storage = null;
44+
}
45+
46+
public boolean storeFragment(ByteBuffer fragment, int length, boolean last)
3347
{
3448
if (storage == null)
3549
{
36-
int space = last ? length : length * 2;
37-
storage = ByteBuffer.allocate(space);
50+
if (length > maxCapacity)
51+
return false;
52+
int capacity = last ? length : length * 2;
53+
storage = ByteBuffer.allocate(capacity);
3854
}
3955

4056
// Grow the storage if necessary.
4157
if (storage.remaining() < length)
4258
{
59+
if (storage.position() + length > maxCapacity)
60+
return false;
4361
int space = last ? length : length * 2;
4462
int capacity = storage.position() + space;
4563
ByteBuffer newStorage = ByteBuffer.allocate(capacity);
@@ -53,6 +71,7 @@ public void storeFragment(ByteBuffer fragment, int length, boolean last)
5371
fragment.limit(fragment.position() + length);
5472
storage.put(fragment);
5573
fragment.limit(limit);
74+
return true;
5675
}
5776

5877
public PriorityFrame getPriorityFrame()
@@ -77,10 +96,8 @@ public void setEndStream(boolean endStream)
7796

7897
public ByteBuffer complete()
7998
{
80-
ByteBuffer result = storage;
81-
storage = null;
82-
result.flip();
83-
return result;
99+
storage.flip();
100+
return storage;
84101
}
85102

86103
public int getStreamId()

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

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,15 @@ else if (hasFlag(Flags.END_HEADERS))
7676
}
7777
else
7878
{
79-
headerBlockFragments.setStreamId(getStreamId());
80-
headerBlockFragments.setEndStream(isEndStream());
79+
if (headerBlockFragments.getStreamId() != 0)
80+
{
81+
connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_headers_frame");
82+
}
83+
else
84+
{
85+
headerBlockFragments.setStreamId(getStreamId());
86+
headerBlockFragments.setEndStream(isEndStream());
87+
}
8188
}
8289
}
8390

@@ -172,6 +179,18 @@ else if (hasFlag(Flags.PRIORITY))
172179
break;
173180
}
174181
case HEADERS:
182+
{
183+
if (!hasFlag(Flags.END_HEADERS))
184+
{
185+
headerBlockFragments.setStreamId(getStreamId());
186+
headerBlockFragments.setEndStream(isEndStream());
187+
if (hasFlag(Flags.PRIORITY))
188+
headerBlockFragments.setPriorityFrame(new PriorityFrame(getStreamId(), parentStreamId, weight, exclusive));
189+
}
190+
state = State.HEADER_BLOCK;
191+
break;
192+
}
193+
case HEADER_BLOCK:
175194
{
176195
if (hasFlag(Flags.END_HEADERS))
177196
{
@@ -196,7 +215,7 @@ else if (hasFlag(Flags.PRIORITY))
196215
{
197216
HeadersFrame frame = new HeadersFrame(getStreamId(), metaData, null, isEndStream());
198217
if (!rateControlOnEvent(frame))
199-
connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_headers_frame_rate");
218+
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_headers_frame_rate");
200219
}
201220
}
202221
}
@@ -205,16 +224,14 @@ else if (hasFlag(Flags.PRIORITY))
205224
int remaining = buffer.remaining();
206225
if (remaining < length)
207226
{
208-
headerBlockFragments.storeFragment(buffer, remaining, false);
227+
if (!headerBlockFragments.storeFragment(buffer, remaining, false))
228+
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_headers_frame");
209229
length -= remaining;
210230
}
211231
else
212232
{
213-
headerBlockFragments.setStreamId(getStreamId());
214-
headerBlockFragments.setEndStream(isEndStream());
215-
if (hasFlag(Flags.PRIORITY))
216-
headerBlockFragments.setPriorityFrame(new PriorityFrame(getStreamId(), parentStreamId, weight, exclusive));
217-
headerBlockFragments.storeFragment(buffer, length, false);
233+
if (!headerBlockFragments.storeFragment(buffer, length, false))
234+
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_headers_frame");
218235
state = State.PADDING;
219236
loop = paddingLength == 0;
220237
}
@@ -258,6 +275,6 @@ private void onHeaders(HeadersFrame frame)
258275

259276
private enum State
260277
{
261-
PREPARE, PADDING_LENGTH, EXCLUSIVE, PARENT_STREAM_ID, PARENT_STREAM_ID_BYTES, WEIGHT, HEADERS, PADDING
278+
PREPARE, PADDING_LENGTH, EXCLUSIVE, PARENT_STREAM_ID, PARENT_STREAM_ID_BYTES, WEIGHT, HEADERS, HEADER_BLOCK, PADDING
262279
}
263280
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public void init(Listener listener)
9292
this.listener = listener;
9393
unknownBodyParser = new UnknownBodyParser(headerParser, listener);
9494
HeaderBlockParser headerBlockParser = new HeaderBlockParser(headerParser, byteBufferPool, hpackDecoder, unknownBodyParser);
95-
HeaderBlockFragments headerBlockFragments = new HeaderBlockFragments();
95+
HeaderBlockFragments headerBlockFragments = new HeaderBlockFragments(hpackDecoder.getMaxHeaderListSize());
9696
bodyParsers[FrameType.DATA.getType()] = new DataBodyParser(headerParser, listener);
9797
bodyParsers[FrameType.HEADERS.getType()] = new HeadersBodyParser(headerParser, listener, headerBlockParser, headerBlockFragments);
9898
bodyParsers[FrameType.PRIORITY.getType()] = new PriorityBodyParser(headerParser, listener);

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.eclipse.jetty.http.MetaData;
2424
import org.eclipse.jetty.http2.ErrorCode;
2525
import org.eclipse.jetty.http2.Flags;
26+
import org.eclipse.jetty.http2.frames.HeadersFrame;
2627
import org.eclipse.jetty.http2.frames.PushPromiseFrame;
2728

2829
public class PushPromiseBodyParser extends BodyParser
@@ -70,13 +71,9 @@ public boolean parse(ByteBuffer buffer)
7071
length = getBodyLength();
7172

7273
if (isPadding())
73-
{
7474
state = State.PADDING_LENGTH;
75-
}
7675
else
77-
{
7876
state = State.STREAM_ID;
79-
}
8077
break;
8178
}
8279
case PADDING_LENGTH:
@@ -136,7 +133,15 @@ public boolean parse(ByteBuffer buffer)
136133
state = State.PADDING;
137134
loop = paddingLength == 0;
138135
if (metaData != HeaderBlockParser.STREAM_FAILURE)
136+
{
139137
onPushPromise(streamId, metaData);
138+
}
139+
else
140+
{
141+
HeadersFrame frame = new HeadersFrame(getStreamId(), metaData, null, isEndStream());
142+
if (!rateControlOnEvent(frame))
143+
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_headers_frame_rate");
144+
}
140145
}
141146
break;
142147
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public boolean parse(ByteBuffer buffer)
6363
{
6464
if (buffer.remaining() >= 4)
6565
{
66-
return onReset(buffer.getInt());
66+
return onReset(buffer, buffer.getInt());
6767
}
6868
else
6969
{
@@ -78,7 +78,7 @@ public boolean parse(ByteBuffer buffer)
7878
--cursor;
7979
error += currByte << (8 * cursor);
8080
if (cursor == 0)
81-
return onReset(error);
81+
return onReset(buffer, error);
8282
break;
8383
}
8484
default:
@@ -90,9 +90,11 @@ public boolean parse(ByteBuffer buffer)
9090
return false;
9191
}
9292

93-
private boolean onReset(int error)
93+
private boolean onReset(ByteBuffer buffer, int error)
9494
{
9595
ResetFrame frame = new ResetFrame(getStreamId(), error);
96+
if (!rateControlOnEvent(frame))
97+
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_rst_stream_frame_rate");
9698
reset();
9799
notifyReset(frame);
98100
return true;

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

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,22 @@ public int getMaxKeys()
7373
@Override
7474
protected void emptyBody(ByteBuffer buffer)
7575
{
76+
if (!validateFrame(buffer, getStreamId(), 0))
77+
return;
7678
boolean isReply = hasFlag(Flags.ACK);
7779
SettingsFrame frame = new SettingsFrame(Collections.emptyMap(), isReply);
78-
if (!isReply && !rateControlOnEvent(frame))
79-
connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_settings_frame_rate");
80-
else
81-
onSettings(frame);
80+
onSettings(buffer, frame);
81+
}
82+
83+
private boolean validateFrame(ByteBuffer buffer, int streamId, int bodyLength)
84+
{
85+
// SPEC: wrong streamId is treated as connection error.
86+
if (streamId != 0)
87+
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_frame");
88+
// SPEC: reply with body is treated as connection error.
89+
if (hasFlag(Flags.ACK) && bodyLength > 0)
90+
return connectionFailure(buffer, ErrorCode.FRAME_SIZE_ERROR.code, "invalid_settings_frame");
91+
return true;
8292
}
8393

8494
@Override
@@ -95,9 +105,8 @@ private boolean parse(ByteBuffer buffer, int streamId, int bodyLength)
95105
{
96106
case PREPARE:
97107
{
98-
// SPEC: wrong streamId is treated as connection error.
99-
if (streamId != 0)
100-
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_frame");
108+
if (!validateFrame(buffer, streamId, bodyLength))
109+
return false;
101110
length = bodyLength;
102111
settings = new HashMap<>();
103112
state = State.SETTING_ID;
@@ -211,11 +220,13 @@ protected boolean onSettings(ByteBuffer buffer, Map<Integer, Integer> settings)
211220
return connectionFailure(buffer, ErrorCode.PROTOCOL_ERROR.code, "invalid_settings_max_frame_size");
212221

213222
SettingsFrame frame = new SettingsFrame(settings, hasFlag(Flags.ACK));
214-
return onSettings(frame);
223+
return onSettings(buffer, frame);
215224
}
216225

217-
private boolean onSettings(SettingsFrame frame)
226+
private boolean onSettings(ByteBuffer buffer, SettingsFrame frame)
218227
{
228+
if (!rateControlOnEvent(frame))
229+
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_settings_frame_rate");
219230
reset();
220231
notifySettings(frame);
221232
return true;

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ public boolean parse(ByteBuffer buffer)
4040
boolean parsed = cursor == 0;
4141
if (parsed && !rateControlOnEvent(new UnknownFrame(getFrameType())))
4242
return connectionFailure(buffer, ErrorCode.ENHANCE_YOUR_CALM_ERROR.code, "invalid_unknown_frame_rate");
43-
4443
return parsed;
4544
}
4645

0 commit comments

Comments
 (0)