Skip to content

Fail unprocessed local streams on GOAWAY #711

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
ok2c merged 1 commit into from
Sep 30, 2026
+52 −35
Merged

Fail unprocessed local streams on GOAWAY #711

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
2 changes: 1 addition & 1 deletion ...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 @@ -1083,7 +1083,7 @@ private void consumeFrame(final RawFrame frame) throws HttpException, IOExceptio
for (final Iterator<H2Stream> it = streams.iterator(); it.hasNext(); ) {
final H2Stream stream = it.next();
final int activeStreamId = stream.getId();
if (!streams.isSameSide(activeStreamId) && activeStreamId > processedLocalStreamId) {
if (streams.isSameSide(activeStreamId) && activeStreamId > processedLocalStreamId) {
stream.fail(new RequestNotExecutedException());
it.remove();
}
Expand Down
85 changes: 51 additions & 34 deletions ...-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 @@ -42,6 +42,7 @@
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.RequestNotExecutedException;
import org.apache.hc.core5.http.config.CharCodingConfig;
import org.apache.hc.core5.http.impl.CharCodingSupport;
import org.apache.hc.core5.http.message.BasicHeader;
Expand Down Expand Up @@ -2026,52 +2027,68 @@ void testGoAwayReservedBitInLastStreamIdAffectsStreamCulling() throws Exception
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));
// Create 3 local (odd) streams: 1, 3, 5.
final H2StreamHandler streamHandler1 = Mockito.mock(H2StreamHandler.class);
final H2StreamHandler streamHandler3 = Mockito.mock(H2StreamHandler.class);
final H2StreamHandler streamHandler5 = Mockito.mock(H2StreamHandler.class);
mux.createStream(mux.createChannel(1), streamHandler1);
mux.createStream(mux.createChannel(3), streamHandler3);
mux.createStream(mux.createChannel(5), streamHandler5);

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());
// GOAWAY last-stream-id = 4, but with reserved MSB set.
// Correct masking keeps streams <= 4 (1 and 3) and drops only stream 5.
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 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
);
final RawFrame goAway = new RawFrame(FrameType.GOAWAY.getValue(), 0, 0, goAwayPayload);

feedFrame(mux, h2);
feedFrame(mux, h4);
feedFrame(mux, h6);
Assertions.assertDoesNotThrow(() -> mux.onInput(ByteBuffer.wrap(encodeFrame(goAway))));

// GOAWAY last-stream-id = 4, but with reserved MSB set.
// Correct masking keeps streams <= 4 (2 and 4) and drops only stream 6.
Mockito.verify(streamHandler5, Mockito.times(1)).failed(exceptionCaptor.capture());
Assertions.assertInstanceOf(RequestNotExecutedException.class, exceptionCaptor.getValue());
Mockito.verify(streamHandler1, Mockito.never()).failed(ArgumentMatchers.any());
Mockito.verify(streamHandler3, Mockito.never()).failed(ArgumentMatchers.any());
}

@Test
void testGoAwayNoErrorFailsUnprocessedLocalStreamsOnly() 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 2 local (odd) streams: 1, 3 and 1 remote (even) stream: 2.
final H2StreamHandler streamHandler1 = Mockito.mock(H2StreamHandler.class);
final H2StreamHandler streamHandler2 = Mockito.mock(H2StreamHandler.class);
final H2StreamHandler streamHandler3 = Mockito.mock(H2StreamHandler.class);
mux.createStream(mux.createChannel(1), streamHandler1);
mux.createStream(mux.createChannel(2), streamHandler2);
mux.createStream(mux.createChannel(3), streamHandler3);

// GOAWAY last-stream-id = 1: local stream 3 was not processed and must fail.
// Last-stream-id does not apply to remote streams, so stream 2 must not fail.
final ByteBuffer goAwayPayload = ByteBuffer.allocate(8);
goAwayPayload.putInt(0x80000004); // reserved bit set, last-stream-id = 4
goAwayPayload.putInt(1); // last-stream-id = 1
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());
Mockito.verify(streamHandler3, Mockito.times(1)).failed(exceptionCaptor.capture());
Assertions.assertInstanceOf(RequestNotExecutedException.class, exceptionCaptor.getValue());
Mockito.verify(streamHandler1, Mockito.never()).failed(ArgumentMatchers.any());
Mockito.verify(streamHandler2, Mockito.never()).failed(ArgumentMatchers.any());
}

@Test
Expand Down
Loading