Skip to content

HTTP/2: ignore reserved MSB in 31-bit fields #633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arturobernalg merged 1 commit into from
Feb 22, 2026
Merged

HTTP/2: ignore reserved MSB in 31-bit fields #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions httpcore5-h2/src/main/java/org/apache/hc/core5/http2/frame/FramePrinter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public void printPayload(final RawFrame frame, final Appendable appendable) thro
break;
case GOAWAY:
if (buf.remaining() >= 8) {
final int lastStream = buf.getInt();
final int lastStream = buf.getInt() & 0x7fffffff;
appendable.append("Last stream ").append(Integer.toString(lastStream)).append("\r\n");
appendable.append("Code ");
final int code2 = buf.getInt();
Expand All @@ -155,7 +155,7 @@ public void printPayload(final RawFrame frame, final Appendable appendable) thro
break;
case WINDOW_UPDATE:
if (buf.remaining() == 4) {
final int increment = buf.getInt();
final int increment = buf.getInt() & 0x7fffffff;
appendable.append("Increment ").append(Integer.toString(increment)).append("\r\n");
} else {
appendable.append("Invalid\r\n");
Expand Down
22 changes: 13 additions & 9 deletions ...ore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ public final void onInput(final ByteBuffer src) throws HttpException, IOExceptio
final RawFrame frame = inputBuffer.read(src, ioSession);
if (frame != null) {
if (streamListener != null) {
streamListener.onFrameInput(this, frame.getStreamId(), frame);
streamListener.onFrameInput(this, frame.getStreamId() & 0x7fffffff, frame);
}
consumeFrame(frame);
} else {
Expand Down Expand Up @@ -770,7 +770,8 @@ public final void onException(final Exception cause) {
private void consumeFrame(final RawFrame frame) throws HttpException, IOException {
updateLastActivity();
final FrameType frameType = FrameType.valueOf(frame.getType());
final int streamId = frame.getStreamId();
final int streamId = frame.getStreamId() & 0x7fffffff;

if (continuation != null && frameType != FrameType.CONTINUATION) {
throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "CONTINUATION frame expected");
}
Expand Down Expand Up @@ -875,8 +876,8 @@ private void consumeFrame(final RawFrame frame) throws HttpException, IOExceptio
if (payload == null || payload.remaining() != 4) {
throw new H2ConnectionException(H2Error.FRAME_SIZE_ERROR, "Invalid WINDOW_UPDATE frame payload");
}
final int delta = payload.getInt();
if (delta <= 0) {
final int delta = payload.getInt() & 0x7fffffff;
if (delta == 0) {
Comment thread
ok2c marked this conversation as resolved.
throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Invalid WINDOW_UPDATE delta");
}
if (streamId == 0) {
Expand Down Expand Up @@ -946,7 +947,6 @@ private void consumeFrame(final RawFrame frame) throws HttpException, IOExceptio
throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Illegal stream id");
}
if (frame.isFlagSet(FrameFlag.ACK)) {
// RFC 9113, Section 6.5: SETTINGS with ACK set MUST have an empty payload.
final ByteBuffer payload = frame.getPayload();
if (payload != null && payload.hasRemaining()) {
throw new H2ConnectionException(H2Error.FRAME_SIZE_ERROR, "Invalid SETTINGS ACK payload");
Expand Down Expand Up @@ -997,7 +997,7 @@ private void consumeFrame(final RawFrame frame) throws HttpException, IOExceptio
if (payload == null || payload.remaining() < 4) {
throw new H2ConnectionException(H2Error.FRAME_SIZE_ERROR, "Invalid PUSH_PROMISE payload");
}
final int promisedStreamId = payload.getInt();
final int promisedStreamId = payload.getInt() & 0x7fffffff;
if (promisedStreamId == 0 || streams.isSameSide(promisedStreamId)) {
throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Illegal promised stream id: " + promisedStreamId);
}
Expand Down Expand Up @@ -1030,7 +1030,7 @@ private void consumeFrame(final RawFrame frame) throws HttpException, IOExceptio
if (payload == null || payload.remaining() < 8) {
throw new H2ConnectionException(H2Error.FRAME_SIZE_ERROR, "Invalid GOAWAY payload");
}
final int processedLocalStreamId = payload.getInt();
final int processedLocalStreamId = payload.getInt() & 0x7fffffff;
final int errorCode = payload.getInt();
goAwayReceived = true;
if (errorCode == H2Error.NO_ERROR.getCode()) {
Expand Down Expand Up @@ -1182,7 +1182,7 @@ private void consumeContinuationFrame(final RawFrame frame, final H2Stream strea
if (stream.isRemoteClosed()) {
throw new H2StreamResetException(H2Error.STREAM_CLOSED, "Stream already closed");
}
final int streamId = frame.getStreamId();
final int streamId = frame.getStreamId() & 0x7fffffff;
final ByteBuffer payload = frame.getPayload();
continuation.copyPayload(payload);
if (frame.isFlagSet(FrameFlag.END_HEADERS)) {
Expand Down Expand Up @@ -1227,10 +1227,14 @@ private void consumeSettingsFrame(final ByteBuffer payload) throws IOException {
configBuilder.setPushEnabled(value == 1);
break;
case INITIAL_WINDOW_SIZE:
if (value < 0) {
throw new H2ConnectionException(H2Error.FLOW_CONTROL_ERROR,
"Invalid initial window size: " + (value & 0xffffffffL));
}
try {
configBuilder.setInitialWindowSize(value);
} catch (final IllegalArgumentException ex) {
throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, ex.getMessage());
throw new H2ConnectionException(H2Error.FLOW_CONTROL_ERROR, ex.getMessage());
}
break;
case MAX_FRAME_SIZE:
Expand Down
206 changes: 205 additions & 1 deletion ...-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestAbstractH2StreamMultiplexer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,57 @@ void testZeroIncrement() throws Exception {
Assertions.assertEquals(H2Error.PROTOCOL_ERROR, H2Error.getByCode(exception.getCode()));
}

@Test
void testZeroIncrementConnectionWindowUpdate() throws Exception {
final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl(
protocolIOSession,
FRAME_FACTORY,
StreamIdGenerator.ODD,
httpProcessor,
CharCodingConfig.DEFAULT,
H2Config.custom().build(),
h2StreamListener,
() -> streamHandler);
try {
final ByteBuffer payload = ByteBuffer.allocate(4);
payload.putInt(0);
payload.flip();
final RawFrame incrementFrame = new RawFrame(FrameType.WINDOW_UPDATE.getValue(), 0, 0, payload);

final H2ConnectionException exception = Assertions.assertThrows(H2ConnectionException.class,
() -> mux.onInput(ByteBuffer.wrap(encodeFrame(incrementFrame))));
Assertions.assertEquals(H2Error.PROTOCOL_ERROR, H2Error.getByCode(exception.getCode()));
} finally {
mux.close();
}
}

@Test
void testInvalidInitialWindowSizeSettingIsFlowControlError() throws Exception {
final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl(
protocolIOSession,
FRAME_FACTORY,
StreamIdGenerator.ODD,
httpProcessor,
CharCodingConfig.DEFAULT,
H2Config.custom().build(),
h2StreamListener,
() -> streamHandler);
try {
final ByteBuffer payload = ByteBuffer.allocate(6);
payload.putShort((short) H2Param.INITIAL_WINDOW_SIZE.getCode());
payload.putInt(-1);
payload.flip();
final RawFrame settingsFrame = new RawFrame(FrameType.SETTINGS.getValue(), 0, 0, payload);

final H2ConnectionException exception = Assertions.assertThrows(H2ConnectionException.class,
() -> mux.onInput(ByteBuffer.wrap(encodeFrame(settingsFrame))));
Assertions.assertEquals(H2Error.FLOW_CONTROL_ERROR, H2Error.getByCode(exception.getCode()));
} finally {
mux.close();
}
}

@Test
void testIncrementOverflow() throws Exception {
final H2Config h2Config = H2Config.custom()
Expand Down Expand Up @@ -1605,5 +1656,158 @@ void testInputSettingsAckWithNonEmptyPayloadRejected() throws Exception {
Assertions.assertEquals(H2Error.FRAME_SIZE_ERROR, H2Error.getByCode(ex.getCode()));
}

}
@Test
void testWindowUpdateReservedBitIgnored() throws Exception {
final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl(
protocolIOSession,
FRAME_FACTORY,
StreamIdGenerator.ODD,
httpProcessor,
CharCodingConfig.DEFAULT,
H2Config.custom().build(),
h2StreamListener,
() -> streamHandler);

final ByteBuffer payload = ByteBuffer.allocate(4);
payload.putInt(0x80000001);
payload.flip();

final RawFrame windowUpdate = new RawFrame(FrameType.WINDOW_UPDATE.getValue(), 0, 0, payload);

Assertions.assertDoesNotThrow(() -> mux.onInput(ByteBuffer.wrap(encodeFrame(windowUpdate))));
}

@Test
void testPingReservedBitInStreamIdIgnored() throws Exception {
final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl(
protocolIOSession,
FRAME_FACTORY,
StreamIdGenerator.ODD,
httpProcessor,
CharCodingConfig.DEFAULT,
H2Config.custom().build(),
h2StreamListener,
() -> streamHandler);

final ByteBuffer payload = ByteBuffer.wrap(new byte[8]);
final RawFrame ping = new RawFrame(FrameType.PING.getValue(), 0, 0x80000000, payload);

Assertions.assertDoesNotThrow(() -> mux.onInput(ByteBuffer.wrap(encodeFrame(ping))));
}

@Test
void testPushPromiseReservedBitInPromisedStreamIdIgnored() throws Exception {
final H2Config h2Config = H2Config.custom()
.setPushEnabled(true)
.build();

final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl(
protocolIOSession,
FRAME_FACTORY,
StreamIdGenerator.ODD,
httpProcessor,
CharCodingConfig.DEFAULT,
h2Config,
h2StreamListener,
() -> streamHandler);

final H2StreamChannel s1 = mux.createChannel(1);
mux.createStream(s1, streamHandler);

final ByteArrayBuffer hbuf = new ByteArrayBuffer(256);
final HPackEncoder encoder = new HPackEncoder(
H2Config.INIT.getHeaderTableSize(),
CharCodingSupport.createEncoder(CharCodingConfig.DEFAULT));

final List<Header> reqHeaders = Arrays.asList(
new BasicHeader(":method", "GET"),
new BasicHeader(":scheme", "https"),
new BasicHeader(":authority", "example.test"),
new BasicHeader(":path", "/pushed"));

encoder.encodeHeaders(hbuf, reqHeaders, h2Config.isCompressionEnabled());

final ByteBuffer payload = ByteBuffer.allocate(4 + hbuf.length());
payload.putInt(0x80000002);
payload.put(hbuf.array(), 0, hbuf.length());
payload.flip();

final RawFrame pp = new RawFrame(
FrameType.PUSH_PROMISE.getValue(),
FrameFlag.END_HEADERS.getValue(),
1,
payload);

Assertions.assertDoesNotThrow(() -> mux.onInput(ByteBuffer.wrap(encodeFrame(pp))));

Mockito.verify(h2StreamListener).onHeaderInput(
ArgumentMatchers.same(mux),
ArgumentMatchers.eq(2),
ArgumentMatchers.anyList());

Mockito.verify(streamHandler).consumePromise(ArgumentMatchers.anyList());
}

@Test
void testGoAwayReservedBitInLastStreamIdAffectsStreamCulling() throws Exception {
final H2Config h2Config = H2Config.custom().build();

final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl(
protocolIOSession,
FRAME_FACTORY,
StreamIdGenerator.ODD, // local=odd, remote=even
httpProcessor,
CharCodingConfig.DEFAULT,
h2Config,
h2StreamListener,
() -> streamHandler);

// Create 3 remote (even) streams by feeding inbound HEADERS on 2,4,6.
final ByteArrayBuffer headerBuf = new ByteArrayBuffer(256);
final HPackEncoder encoder = new HPackEncoder(
H2Config.INIT.getHeaderTableSize(),
CharCodingSupport.createEncoder(CharCodingConfig.DEFAULT));

final List<Header> headers = Arrays.asList(
new BasicHeader(":method", "GET"),
new BasicHeader(":scheme", "https"),
new BasicHeader(":path", "/"),
new BasicHeader(":authority", "example.test"));
encoder.encodeHeaders(headerBuf, headers, h2Config.isCompressionEnabled());

final RawFrame h2 = FRAME_FACTORY.createHeaders(2,
ByteBuffer.wrap(headerBuf.array(), 0, headerBuf.length()),
true, // END_HEADERS
false // END_STREAM
);
final RawFrame h4 = FRAME_FACTORY.createHeaders(4,
ByteBuffer.wrap(headerBuf.array(), 0, headerBuf.length()),
true,
false
);
final RawFrame h6 = FRAME_FACTORY.createHeaders(6,
ByteBuffer.wrap(headerBuf.array(), 0, headerBuf.length()),
true,
false
);

feedFrame(mux, h2);
feedFrame(mux, h4);
feedFrame(mux, h6);

// GOAWAY last-stream-id = 4, but with reserved MSB set.
// Correct masking keeps streams <= 4 (2 and 4) and drops only stream 6.
final ByteBuffer goAwayPayload = ByteBuffer.allocate(8);
goAwayPayload.putInt(0x80000004); // reserved bit set, last-stream-id = 4
goAwayPayload.putInt(H2Error.NO_ERROR.getCode());
goAwayPayload.flip();

final RawFrame goAway = new RawFrame(FrameType.GOAWAY.getValue(), 0, 0, goAwayPayload);

Assertions.assertDoesNotThrow(() -> mux.onInput(ByteBuffer.wrap(encodeFrame(goAway))));

Mockito.verify(streamHandler, Mockito.times(1)).failed(exceptionCaptor.capture());
Assertions.assertInstanceOf(org.apache.hc.core5.http.RequestNotExecutedException.class, exceptionCaptor.getValue());
}

}
Loading