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 964ae61e4..914584e0f 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 @@ -85,6 +85,8 @@ import org.apache.hc.core5.http2.frame.StreamIdGenerator; import org.apache.hc.core5.http2.hpack.HPackDecoder; import org.apache.hc.core5.http2.hpack.HPackEncoder; +import org.apache.hc.core5.http2.hpack.HPackException; +import org.apache.hc.core5.http2.hpack.HeaderListConstraintException; import org.apache.hc.core5.http2.impl.BasicH2TransportMetrics; import org.apache.hc.core5.http2.nio.AsyncPingHandler; import org.apache.hc.core5.http2.nio.command.PingCommand; @@ -1114,7 +1116,7 @@ private void consumePushPromiseFrame(final RawFrame frame, final ByteBuffer payl localConfig.getMaxContinuations()); } if (continuation == null) { - final List
headers = hPackDecoder.decodeHeaders(payload); + final List
headers = decodeHeaders(payload); if (streamListener != null) { streamListener.onHeaderInput(this, promisedStreamId, headers); } @@ -1124,8 +1126,23 @@ private void consumePushPromiseFrame(final RawFrame frame, final ByteBuffer payl } } - List
decodeHeaders(final ByteBuffer payload) throws HttpException { - return hPackDecoder.decodeHeaders(payload); + List
decodeHeaders(final ByteBuffer payload) throws HttpException, IOException { + try { + return hPackDecoder.decodeHeaders(payload); + } catch (final HeaderListConstraintException ex) { + // Not a decoding failure; allow upper layers to map (server maps to 431). + throw ex; + } catch (final HPackException ex) { + final H2ConnectionException connEx = + new H2ConnectionException(H2Error.COMPRESSION_ERROR, ex.getMessage()); + connEx.initCause(ex); + throw connEx; + } catch (final IllegalArgumentException ex) { + // Decoder invariant violation during HPACK parsing -> treat as decoding failure. + final H2ConnectionException connEx = new H2ConnectionException(H2Error.COMPRESSION_ERROR, ex.getMessage()); + connEx.initCause(ex); + throw connEx; + } } private void consumeHeaderFrame(final RawFrame frame, final H2Stream stream) throws HttpException, IOException { diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerH2StreamMultiplexer.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerH2StreamMultiplexer.java index 046047398..8fca022d2 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerH2StreamMultiplexer.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ServerH2StreamMultiplexer.java @@ -154,7 +154,7 @@ boolean allowGracefulAbort(final H2Stream stream) { } @Override - List
decodeHeaders(final ByteBuffer payload) throws HttpException { + List
decodeHeaders(final ByteBuffer payload) throws HttpException, IOException { try { return super.decodeHeaders(payload); } catch (final HeaderListConstraintException ex) { 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 65d5493ec..e1464bf7e 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 @@ -67,6 +67,7 @@ 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.http2.hpack.HPackException; import org.apache.hc.core5.reactor.ProtocolIOSession; import org.apache.hc.core5.util.ByteArrayBuffer; import org.apache.hc.core5.util.Timeout; @@ -1435,5 +1436,108 @@ private static void makeMuxIdle(final AbstractH2StreamMultiplexer mux, final Tim setLastActivityTime(mux, System.currentTimeMillis() - effectiveMillis - 10); } + @Test + void testHpackDecodeFailureInHeadersCausesConnectionCompressionError() throws Exception { + final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl( + protocolIOSession, + FRAME_FACTORY, + StreamIdGenerator.ODD, // peer-initiated streams are EVEN (e.g. 2) + httpProcessor, + CharCodingConfig.DEFAULT, + H2Config.custom().build(), + h2StreamListener, + () -> streamHandler); + + final WritableByteChannelMock writableChannel = new WritableByteChannelMock(256); + final FrameOutputBuffer outBuffer = new FrameOutputBuffer(16 * 1024); + + // Invalid HPACK: indexed header field representation with index = 0 (illegal). + final byte[] invalidHeaderBlock = new byte[] { (byte) 0x80 }; + + final RawFrame headersFrame = FRAME_FACTORY.createHeaders( + 2, + ByteBuffer.wrap(invalidHeaderBlock), + true, // END_HEADERS + false); + + outBuffer.write(headersFrame, writableChannel); + + final H2ConnectionException exception = Assertions.assertThrows(H2ConnectionException.class, () -> + mux.onInput(ByteBuffer.wrap(writableChannel.toByteArray()))); + + Assertions.assertEquals(H2Error.COMPRESSION_ERROR, H2Error.getByCode(exception.getCode())); + + // Must not be delegated to stream-level handlers. + Mockito.verify(streamHandler, Mockito.never()) + .consumeHeader(ArgumentMatchers.anyList(), ArgumentMatchers.anyBoolean()); + Mockito.verify(streamHandler, Mockito.never()) + .handle(ArgumentMatchers.any(HttpException.class), ArgumentMatchers.anyBoolean()); + Mockito.verify(streamHandler, Mockito.never()) + .failed(ArgumentMatchers.any(Exception.class)); + } + + @Test + void testHpackDecodeFailureInPushPromiseCausesConnectionCompressionError() throws Exception { + final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl( + protocolIOSession, + FRAME_FACTORY, + StreamIdGenerator.ODD, + httpProcessor, + CharCodingConfig.DEFAULT, + H2Config.custom().setPushEnabled(true).build(), + h2StreamListener, + () -> streamHandler); + + // Create an existing local stream (client-initiated, odd) that can receive PUSH_PROMISE. + final H2StreamChannel channel = mux.createChannel(1); + mux.createStream(channel, streamHandler); + + final ByteBuffer payload = ByteBuffer.allocate(5); + payload.putInt(2); // promised stream id (peer/server uses even) + } + + @Test + void testHpackExceptionInHeadersMappedToConnectionCompressionError() throws Exception { + final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl( + protocolIOSession, + FRAME_FACTORY, + StreamIdGenerator.ODD, + httpProcessor, + CharCodingConfig.DEFAULT, + H2Config.custom().build(), + h2StreamListener, + () -> streamHandler); + + final WritableByteChannelMock writableChannel = new WritableByteChannelMock(256); + final FrameOutputBuffer outBuffer = new FrameOutputBuffer(16 * 1024); + + // HPACK: literal header field with incremental indexing (0x40), "new name". + // Next byte is the name string length (here: 10), but we provide no bytes -> truncated. + // This should throw HPackException. + final byte[] invalidHeaderBlock = new byte[] { (byte) 0x40, (byte) 0x0a }; + + final RawFrame headersFrame = FRAME_FACTORY.createHeaders( + 2, + ByteBuffer.wrap(invalidHeaderBlock), + true, // END_HEADERS + false); + + outBuffer.write(headersFrame, writableChannel); + + final H2ConnectionException ex = Assertions.assertThrows(H2ConnectionException.class, () -> + mux.onInput(ByteBuffer.wrap(writableChannel.toByteArray()))); + + Assertions.assertEquals(H2Error.COMPRESSION_ERROR, H2Error.getByCode(ex.getCode())); + Assertions.assertInstanceOf(HPackException.class, ex.getCause()); + + // Must not be handled at stream level + Mockito.verify(streamHandler, Mockito.never()) + .consumeHeader(ArgumentMatchers.anyList(), ArgumentMatchers.anyBoolean()); + Mockito.verify(streamHandler, Mockito.never()) + .handle(ArgumentMatchers.any(HttpException.class), ArgumentMatchers.anyBoolean()); + Mockito.verify(streamHandler, Mockito.never()) + .failed(ArgumentMatchers.any(Exception.class)); + } + }