diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java index 53d200c29..70d619a2b 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java @@ -1083,7 +1083,7 @@ private void consumeFrame(final RawFrame frame) throws HttpException, IOExceptio for (final Iterator 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(); } diff --git a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestAbstractH2StreamMultiplexer.java b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestAbstractH2StreamMultiplexer.java index 4b72cfec3..3c390acff 100644 --- a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestAbstractH2StreamMultiplexer.java +++ b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestAbstractH2StreamMultiplexer.java @@ -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; @@ -2026,43 +2027,57 @@ 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
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(); @@ -2070,8 +2085,10 @@ void testGoAwayReservedBitInLastStreamIdAffectsStreamCulling() throws Exception 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