HTTP/2: handle zero WINDOW_UPDATE increment (#634)

Treat WINDOW_UPDATE with a window size increment of 0 as PROTOCOL_ERROR.
Apply stream error semantics for stream windows and connection error semantics
for stream 0, as required by RFC 9113.

Author: arturobernalg

httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java

@@ -878,7 +878,15 @@ private void consumeFrame(final RawFrame frame) throws HttpException, IOExceptio
                 }
                 final int delta = payload.getInt() & 0x7fffffff;
                 if (delta == 0) {
-                    throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Invalid WINDOW_UPDATE delta");
+                    if (streamId == 0) {
+                        throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Invalid WINDOW_UPDATE delta");
+                    }
+                    final H2Stream stream = streams.lookup(streamId);
+                    if (stream != null) {
+                        stream.localReset(new H2StreamResetException(H2Error.PROTOCOL_ERROR, "Invalid WINDOW_UPDATE delta"));
+                        requestSessionOutput();
+                    }
+                    break;
                 }
                 if (streamId == 0) {
                     try {
@@ -947,6 +955,7 @@ 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");

httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestAbstractH2StreamMultiplexer.java

@@ -380,9 +380,8 @@ void testZeroIncrement() throws Exception {
         final RawFrame incrementFrame = new RawFrame(FrameType.WINDOW_UPDATE.getValue(), 0, 1, payload);
         outBuffer.write(incrementFrame, writableChannel);
 
-        final H2ConnectionException exception = Assertions.assertThrows(H2ConnectionException.class, () ->
+        Assertions.assertDoesNotThrow(() ->
                 streamMultiplexer.onInput(ByteBuffer.wrap(writableChannel.toByteArray())));
-        Assertions.assertEquals(H2Error.PROTOCOL_ERROR, H2Error.getByCode(exception.getCode()));
     }
 
     @Test
@@ -1810,4 +1809,58 @@ void testGoAwayReservedBitInLastStreamIdAffectsStreamCulling() throws Exception
         Assertions.assertInstanceOf(org.apache.hc.core5.http.RequestNotExecutedException.class, exceptionCaptor.getValue());
     }
 
+    @Test
+    void testWindowUpdateZeroIncrementOnConnectionIsConnectionError() 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(0);
+        payload.flip();
+
+        final RawFrame windowUpdate = new RawFrame(FrameType.WINDOW_UPDATE.getValue(), 0, 0, payload);
+
+        final H2ConnectionException ex = Assertions.assertThrows(
+                H2ConnectionException.class,
+                () -> mux.onInput(ByteBuffer.wrap(encodeFrame(windowUpdate))));
+
+        Assertions.assertEquals(H2Error.PROTOCOL_ERROR, H2Error.getByCode(ex.getCode()));
+    }
+
+    @Test
+    void testWindowUpdateZeroIncrementOnStreamIsStreamError() throws Exception {
+        final AbstractH2StreamMultiplexer mux = new H2StreamMultiplexerImpl(
+                protocolIOSession,
+                FRAME_FACTORY,
+                StreamIdGenerator.ODD,
+                httpProcessor,
+                CharCodingConfig.DEFAULT,
+                H2Config.custom().build(),
+                h2StreamListener,
+                () -> streamHandler);
+
+        final H2StreamChannel channel = mux.createChannel(1);
+        mux.createStream(channel, streamHandler);
+
+        final ByteBuffer payload = ByteBuffer.allocate(4);
+        payload.putInt(0);
+        payload.flip();
+
+        final RawFrame windowUpdate = new RawFrame(FrameType.WINDOW_UPDATE.getValue(), 0, 1, payload);
+
+        Assertions.assertDoesNotThrow(() -> mux.onInput(ByteBuffer.wrap(encodeFrame(windowUpdate))));
+
+        Mockito.verify(streamHandler).failed(exceptionCaptor.capture());
+        final Exception cause = exceptionCaptor.getValue();
+        Assertions.assertInstanceOf(H2StreamResetException.class, cause);
+        Assertions.assertEquals(H2Error.PROTOCOL_ERROR, H2Error.getByCode(((H2StreamResetException) cause).getCode()));
+    }
+
 }