From cf825f16a889c3f9c128f5c1b46bda978c6f979d Mon Sep 17 00:00:00 2001 From: Shang Xiang Date: Tue, 26 Nov 2024 16:30:35 +0100 Subject: [PATCH 1/3] Update the pointer of Continuation's headerBuffer after copying the payload. --- .../core5/http2/impl/nio/AbstractH2StreamMultiplexer.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 f7792f8aad..c9d67c9e39 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 @@ -1342,8 +1342,11 @@ void copyPayload(final ByteBuffer payload) { if (payload == null) { return; } - headerBuffer.ensureCapacity(payload.remaining()); - payload.get(headerBuffer.array(), headerBuffer.length(), payload.remaining()); + int originalLength = headerBuffer.length(); + int toCopy = payload.remaining(); + headerBuffer.ensureCapacity(toCopy); + payload.get(headerBuffer.array(), originalLength, toCopy); + headerBuffer.setLength(originalLength + toCopy); } ByteBuffer getContent() { From 84cd3072087facfca7a657cbe85a33fbc26635d5 Mon Sep 17 00:00:00 2001 From: CoolTomatos <24667806+CoolTomatos@users.noreply.github.com> Date: Tue, 26 Nov 2024 19:46:26 +0100 Subject: [PATCH 2/3] Fix the build. --- .../hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 c9d67c9e39..14e03cab81 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 @@ -1342,8 +1342,8 @@ void copyPayload(final ByteBuffer payload) { if (payload == null) { return; } - int originalLength = headerBuffer.length(); - int toCopy = payload.remaining(); + final int originalLength = headerBuffer.length(); + final int toCopy = payload.remaining(); headerBuffer.ensureCapacity(toCopy); payload.get(headerBuffer.array(), originalLength, toCopy); headerBuffer.setLength(originalLength + toCopy); From 39feaee75f1e415d809ee49bd9037825e99bfd5f Mon Sep 17 00:00:00 2001 From: Shang Xiang Date: Wed, 27 Nov 2024 11:17:43 +0100 Subject: [PATCH 3/3] Add test coverage. --- .../nio/TestAbstractH2StreamMultiplexer.java | 55 ++++++++++++++++++- 1 file changed, 52 insertions(+), 3 deletions(-) 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 976d1913bb..0704604965 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 @@ -29,9 +29,15 @@ import java.io.IOException; import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.config.CharCodingConfig; import org.apache.hc.core5.http.impl.BasicHttpConnectionMetrics; +import org.apache.hc.core5.http.impl.CharCodingSupport; +import org.apache.hc.core5.http.message.BasicHeader; import org.apache.hc.core5.http.nio.AsyncPushConsumer; import org.apache.hc.core5.http.nio.HandlerFactory; import org.apache.hc.core5.http.nio.command.ExecutableCommand; @@ -45,16 +51,22 @@ import org.apache.hc.core5.http2.frame.FrameType; import org.apache.hc.core5.http2.frame.RawFrame; import org.apache.hc.core5.http2.frame.StreamIdGenerator; +import org.apache.hc.core5.http2.hpack.HPackEncoder; import org.apache.hc.core5.reactor.ProtocolIOSession; +import org.apache.hc.core5.util.ByteArrayBuffer; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatchers; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; class TestAbstractH2StreamMultiplexer { + private static final FrameFactory FRAME_FACTORY = DefaultFrameFactory.INSTANCE; + private static final H2StreamHandler STREAM_HANDLER = Mockito.mock(H2StreamHandler.class); @Mock ProtocolIOSession protocolIOSession; @@ -62,6 +74,8 @@ class TestAbstractH2StreamMultiplexer { HttpProcessor httpProcessor; @Mock H2StreamListener h2StreamListener; + @Captor + ArgumentCaptor> headersCaptor; @BeforeEach void prepareMocks() { @@ -99,7 +113,7 @@ H2StreamHandler createRemotelyInitiatedStream( final HttpProcessor httpProcessor, final BasicHttpConnectionMetrics connMetrics, final HandlerFactory pushHandlerFactory) throws IOException { - return null; + return STREAM_HANDLER; } @Override @@ -128,7 +142,7 @@ void testInputOneFrame() throws Exception { final AbstractH2StreamMultiplexer streamMultiplexer = new H2StreamMultiplexerImpl( protocolIOSession, - DefaultFrameFactory.INSTANCE, + FRAME_FACTORY, StreamIdGenerator.ODD, httpProcessor, CharCodingConfig.DEFAULT, @@ -179,7 +193,7 @@ void testInputMultipleFrames() throws Exception { final AbstractH2StreamMultiplexer streamMultiplexer = new H2StreamMultiplexerImpl( protocolIOSession, - DefaultFrameFactory.INSTANCE, + FRAME_FACTORY, StreamIdGenerator.ODD, httpProcessor, CharCodingConfig.DEFAULT, @@ -212,5 +226,40 @@ void testInputMultipleFrames() throws Exception { }); } + @Test + void testInputHeaderContinuationFrame() throws IOException, HttpException { + final H2Config h2Config = H2Config.custom().setMaxFrameSize(FrameConsts.MIN_FRAME_SIZE) + .build(); + + final ByteArrayBuffer buf = new ByteArrayBuffer(19); + final HPackEncoder encoder = new HPackEncoder(H2Config.INIT.getHeaderTableSize(), CharCodingSupport.createEncoder(CharCodingConfig.DEFAULT)); + final List
headers = new ArrayList<>(); + headers.add(new BasicHeader("test-header-key", "value")); + headers.add(new BasicHeader(":status", "200")); + encoder.encodeHeaders(buf, headers, h2Config.isCompressionEnabled()); + + final WritableByteChannelMock writableChannel = new WritableByteChannelMock(1024); + final FrameOutputBuffer outBuffer = new FrameOutputBuffer(16 * 1024); + + final RawFrame headerFrame = FRAME_FACTORY.createHeaders(2, ByteBuffer.wrap(buf.array(), 0, 10), false, false); + outBuffer.write(headerFrame, writableChannel); + final RawFrame continuationFrame = FRAME_FACTORY.createContinuation(2, ByteBuffer.wrap(buf.array(), 10, 9), true); + outBuffer.write(continuationFrame, writableChannel); + final byte[] bytes = writableChannel.toByteArray(); + + final AbstractH2StreamMultiplexer streamMultiplexer = new H2StreamMultiplexerImpl( + protocolIOSession, + FRAME_FACTORY, + StreamIdGenerator.ODD, + httpProcessor, + CharCodingConfig.DEFAULT, + h2Config, + h2StreamListener + ); + + streamMultiplexer.onInput(ByteBuffer.wrap(bytes)); + Mockito.verify(STREAM_HANDLER).consumeHeader(headersCaptor.capture(), ArgumentMatchers.eq(false)); + Assertions.assertFalse(headersCaptor.getValue().isEmpty()); + } }