From 075a9eee0277d52b7e4a4a48006a49f6a628e9c8 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sat, 21 Feb 2026 14:06:05 +0100 Subject: [PATCH] HTTP/2: tighten SETTINGS validation Enforce RFC 9113 constraints for SETTINGS_ENABLE_PUSH and reject invalid values. Accept zero values for peer settings that may legitimately be 0. Add unit tests for client and server multiplexers. --- .../hc/core5/http2/config/H2Config.java | 11 +++---- .../impl/nio/ClientH2StreamMultiplexer.java | 9 ++++-- .../impl/nio/ServerH2StreamMultiplexer.java | 5 +++ .../nio/TestClientH2StreamMultiplexer.java | 28 ++++++++++++++-- .../nio/TestServerH2StreamMultiplexer.java | 32 ++++++++++++++++++- 5 files changed, 73 insertions(+), 12 deletions(-) diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/config/H2Config.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/config/H2Config.java index a801cc51fe..89b538860d 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/config/H2Config.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/config/H2Config.java @@ -179,14 +179,12 @@ public Builder setPushEnabled(final boolean pushEnabled) { } public Builder setMaxConcurrentStreams(final int maxConcurrentStreams) { - Args.positive(maxConcurrentStreams, "Max concurrent streams"); - this.maxConcurrentStreams = maxConcurrentStreams; + this.maxConcurrentStreams = Args.checkRange(maxConcurrentStreams, 0, Integer.MAX_VALUE, "Max concurrent streams"); return this; } public Builder setInitialWindowSize(final int initialWindowSize) { - Args.positive(initialWindowSize, "Initial window size"); - this.initialWindowSize = initialWindowSize; + this.initialWindowSize = Args.checkRange(initialWindowSize, 0, Integer.MAX_VALUE, "Initial window size"); return this; } @@ -197,8 +195,7 @@ public Builder setMaxFrameSize(final int maxFrameSize) { } public Builder setMaxHeaderListSize(final int maxHeaderListSize) { - Args.positive(maxHeaderListSize, "Max header list size"); - this.maxHeaderListSize = maxHeaderListSize; + this.maxHeaderListSize = Args.checkRange(maxHeaderListSize, 0, Integer.MAX_VALUE, "Max header list size"); return this; } @@ -214,7 +211,7 @@ public Builder setCompressionEnabled(final boolean compressionEnabled) { * @since 5,4 */ public Builder setMaxContinuations(final int maxContinuations) { - Args.positive(maxContinuations, "Max continuations"); + Args.notNegative(maxContinuations, "Max continuations"); this.maxContinuations = maxContinuations; return this; } diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamMultiplexer.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamMultiplexer.java index 7b313f8ee4..4d23d5144d 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamMultiplexer.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/ClientH2StreamMultiplexer.java @@ -104,8 +104,13 @@ public ClientH2StreamMultiplexer( @Override void validateSetting(final H2Param param, final int value) throws H2ConnectionException { - if (param == H2Param.ENABLE_PUSH && value == 1) { - throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Illegal ENABLE_PUSH setting"); + if (param == H2Param.ENABLE_PUSH) { + if (value != 0 && value != 1) { + throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Illegal ENABLE_PUSH setting: " + value); + } + if (value == 1) { + throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Illegal ENABLE_PUSH setting"); + } } } 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 8fca022d2f..9a70b3c959 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 @@ -90,6 +90,11 @@ public ServerH2StreamMultiplexer( @Override void validateSetting(final H2Param param, final int value) throws H2ConnectionException { + if (param == H2Param.ENABLE_PUSH) { + if (value != 0 && value != 1) { + throw new H2ConnectionException(H2Error.PROTOCOL_ERROR, "Illegal ENABLE_PUSH setting: " + value); + } + } } @Override diff --git a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientH2StreamMultiplexer.java b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientH2StreamMultiplexer.java index b38ffc8640..d667037730 100644 --- a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientH2StreamMultiplexer.java +++ b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestClientH2StreamMultiplexer.java @@ -60,13 +60,35 @@ private ClientH2StreamMultiplexer newMultiplexer() { } @Test - void validateSettingRejectsEnablePush() { + void validateSettingRejectsEnablePush1OnClient() { final ClientH2StreamMultiplexer multiplexer = newMultiplexer(); final H2ConnectionException ex = Assertions.assertThrows(H2ConnectionException.class, () -> multiplexer.validateSetting(H2Param.ENABLE_PUSH, 1)); Assertions.assertEquals(H2Error.PROTOCOL_ERROR.getCode(), ex.getCode()); } + @Test + void validateSettingAcceptsEnablePush0OnClient() throws Exception { + final ClientH2StreamMultiplexer multiplexer = newMultiplexer(); + multiplexer.validateSetting(H2Param.ENABLE_PUSH, 0); + } + + @Test + void validateSettingRejectsEnablePush2OnClient() { + final ClientH2StreamMultiplexer multiplexer = newMultiplexer(); + final H2ConnectionException ex = Assertions.assertThrows(H2ConnectionException.class, () -> + multiplexer.validateSetting(H2Param.ENABLE_PUSH, 2)); + Assertions.assertEquals(H2Error.PROTOCOL_ERROR.getCode(), ex.getCode()); + } + + @Test + void validateSettingRejectsEnablePushNegativeOnClient() { + final ClientH2StreamMultiplexer multiplexer = newMultiplexer(); + final H2ConnectionException ex = Assertions.assertThrows(H2ConnectionException.class, () -> + multiplexer.validateSetting(H2Param.ENABLE_PUSH, -1)); + Assertions.assertEquals(H2Error.PROTOCOL_ERROR.getCode(), ex.getCode()); + } + @Test void acceptHeaderFrameRejected() { final ClientH2StreamMultiplexer multiplexer = newMultiplexer(); @@ -95,7 +117,9 @@ void incomingRequestRejected() { void outgoingPushPromiseRejected() { final ClientH2StreamMultiplexer multiplexer = newMultiplexer(); Assertions.assertThrows(H2ConnectionException.class, () -> - multiplexer.outgoingPushPromise(Mockito.mock(H2StreamChannel.class), Mockito.mock(AsyncPushProducer.class))); + multiplexer.outgoingPushPromise( + Mockito.mock(H2StreamChannel.class), + Mockito.mock(AsyncPushProducer.class))); } @Test diff --git a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestServerH2StreamMultiplexer.java b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestServerH2StreamMultiplexer.java index 54c5319d3a..4f5bac3c33 100644 --- a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestServerH2StreamMultiplexer.java +++ b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/impl/nio/TestServerH2StreamMultiplexer.java @@ -35,6 +35,7 @@ import org.apache.hc.core5.http2.H2ConnectionException; import org.apache.hc.core5.http2.H2Error; import org.apache.hc.core5.http2.config.H2Config; +import org.apache.hc.core5.http2.config.H2Param; import org.apache.hc.core5.http2.frame.DefaultFrameFactory; import org.apache.hc.core5.reactor.ProtocolIOSession; import org.junit.jupiter.api.Assertions; @@ -59,6 +60,34 @@ private ServerH2StreamMultiplexer newMultiplexer() { null); } + @Test + void validateSettingAcceptsEnablePush0OnServer() throws Exception { + final ServerH2StreamMultiplexer multiplexer = newMultiplexer(); + multiplexer.validateSetting(H2Param.ENABLE_PUSH, 0); + } + + @Test + void validateSettingAcceptsEnablePush1OnServer() throws Exception { + final ServerH2StreamMultiplexer multiplexer = newMultiplexer(); + multiplexer.validateSetting(H2Param.ENABLE_PUSH, 1); + } + + @Test + void validateSettingRejectsEnablePush2OnServer() { + final ServerH2StreamMultiplexer multiplexer = newMultiplexer(); + final H2ConnectionException ex = Assertions.assertThrows(H2ConnectionException.class, () -> + multiplexer.validateSetting(H2Param.ENABLE_PUSH, 2)); + Assertions.assertEquals(H2Error.PROTOCOL_ERROR.getCode(), ex.getCode()); + } + + @Test + void validateSettingRejectsEnablePushNegativeOnServer() { + final ServerH2StreamMultiplexer multiplexer = newMultiplexer(); + final H2ConnectionException ex = Assertions.assertThrows(H2ConnectionException.class, () -> + multiplexer.validateSetting(H2Param.ENABLE_PUSH, -1)); + Assertions.assertEquals(H2Error.PROTOCOL_ERROR.getCode(), ex.getCode()); + } + @Test void acceptPushFrameRejected() { final ServerH2StreamMultiplexer multiplexer = newMultiplexer(); @@ -73,7 +102,8 @@ void outgoingRequestRejected() { final HandlerFactory pushHandlerFactory = (HandlerFactory) Mockito.mock(HandlerFactory.class); Assertions.assertThrows(H2ConnectionException.class, () -> - multiplexer.outgoingRequest(Mockito.mock(H2StreamChannel.class), + multiplexer.outgoingRequest( + Mockito.mock(H2StreamChannel.class), Mockito.mock(AsyncClientExchangeHandler.class), pushHandlerFactory, null));