Fix - Http3FrameCodec decode fail during unknown settings (#15998)

sanjomo · normanmaurer · web-flow · commit 640b6b72c73f · 2025-12-09T10:32:50.000-08:00
**Motivation:** The change in [https://github.com/netty/netty/pull/15909](https://github.com/netty/netty/pull/15909) is merged but not yet released. While working on HTTP/3 connections I noticed a logical issue: when an unknown settings key is received, the current implementation returns a value, which causes Http3FrameCodec to treat it as an error (because settings.put(key, value) returns the previous value for known keys and null for new ones). For unknown keys we should ignore them, so we now always return null to achieve the expected behavior. **Modification:** * [x] settings.put(key, value) now returns null for unknown keys (non null triggers an error in Http3FrameCodec decoding). * [x] Added unit tests to verify this. > Note: Http3CodecTest didn’t catch the issue previously because it used frames produced by the same put logic, so both encoding and decoding behaved consistently and the issue was hidden. --------- Co-authored-by: Norman Maurer <norman_maurer@apple.com>
diff --git a/codec-http3/src/main/java/io/netty/handler/codec/http3/Http3SettingIdentifier.java b/codec-http3/src/main/java/io/netty/handler/codec/http3/Http3SettingIdentifier.java
@@ -91,11 +91,7 @@ public enum Http3SettingIdentifier {
     private final long id;
 
     private static final Map<Long, Http3SettingIdentifier> LOOKUP = Collections.unmodifiableMap(
-        Arrays.stream(values())
-                    .collect(Collectors.toMap(
-                            Http3SettingIdentifier::id,
-                            Function.identity()
-                    ))
+        Arrays.stream(values()).collect(Collectors.toMap(Http3SettingIdentifier::id, Function.identity()))
     );
 
     Http3SettingIdentifier(long id) {
diff --git a/codec-http3/src/main/java/io/netty/handler/codec/http3/Http3Settings.java b/codec-http3/src/main/java/io/netty/handler/codec/http3/Http3Settings.java
@@ -54,7 +54,7 @@ public final class Http3Settings implements Iterable<Map.Entry<Long, Long>> {
      * Creates a new instance
      */
     public Http3Settings() {
-        this.settings = new LongObjectHashMap<>(4);
+        this.settings = new LongObjectHashMap<>(Http3SettingIdentifier.values().length);
     }
 
     /**
@@ -99,7 +99,7 @@ public Long put(long key, Long value) {
 
         // When Non-Standard/Unknown settings identifier identifier present - Ignore
         if (identifier == null) {
-            return value;
+            return null;
         }
 
         //Validation
diff --git a/codec-http3/src/test/java/io/netty/handler/codec/http3/Http3SettingsTest.java b/codec-http3/src/test/java/io/netty/handler/codec/http3/Http3SettingsTest.java
@@ -264,4 +264,46 @@ void testCopyFromNullThrows() {
         Http3Settings settings = new Http3Settings();
         assertThrows(NullPointerException.class, () -> settings.putAll(null));
     }
+
+    @Test
+    void duplicateSettingsValuesInsideFrameTest() {
+        Http3SettingsFrame settingsFrame = new DefaultHttp3SettingsFrame();
+        assertNull(settingsFrame.put(Http3SettingIdentifier.HTTP3_SETTINGS_QPACK_MAX_TABLE_CAPACITY.id(), 100L));
+        assertNull(settingsFrame.put(Http3SettingIdentifier.HTTP3_SETTINGS_QPACK_BLOCKED_STREAMS.id(), 1L));
+        assertNull(settingsFrame.put(Http3SettingIdentifier.HTTP3_SETTINGS_MAX_FIELD_SECTION_SIZE.id(), 128L));
+        assertNull(settingsFrame.put(Http3SettingIdentifier.HTTP3_SETTINGS_ENABLE_CONNECT_PROTOCOL.id(), 0L));
+        assertNull(settingsFrame.put(Http3SettingIdentifier.HTTP3_SETTINGS_H3_DATAGRAM.id(), 1L));
+        //known headers should not contain duplicate so give non-null
+        // which is used in http3framecodec to throw error
+        assertNotNull(settingsFrame.put(Http3SettingIdentifier.HTTP3_SETTINGS_H3_DATAGRAM.id(), 1L));
+        // Ensure we can encode and decode all sizes correctly.
+        // unknown settings id/key will be ignored
+        assertNull(settingsFrame.put(63, 63L));
+        assertNull(settingsFrame.put(16383, 16383L));
+        assertNull(settingsFrame.put(1073741823, 1073741823L));
+        assertNull(settingsFrame.put(4611686018427387903L, 4611686018427387903L));
+        //even duplicates of unknown ignored as we ignore unknown
+        assertNull(settingsFrame.put(4611686018427387903L, 4611686018427387903L));
+    }
+
+    @Test
+    void duplicateSettingsValuesInsideHttp3SettingsTest() {
+        Http3Settings http3Settings = new Http3Settings();
+        assertNull(http3Settings.put(Http3SettingIdentifier.HTTP3_SETTINGS_QPACK_MAX_TABLE_CAPACITY.id(), 100L));
+        assertNull(http3Settings.put(Http3SettingIdentifier.HTTP3_SETTINGS_QPACK_BLOCKED_STREAMS.id(), 1L));
+        assertNull(http3Settings.put(Http3SettingIdentifier.HTTP3_SETTINGS_MAX_FIELD_SECTION_SIZE.id(), 128L));
+        assertNull(http3Settings.put(Http3SettingIdentifier.HTTP3_SETTINGS_ENABLE_CONNECT_PROTOCOL.id(), 0L));
+        assertNull(http3Settings.put(Http3SettingIdentifier.HTTP3_SETTINGS_H3_DATAGRAM.id(), 1L));
+        //known headers should not contain duplicate so give non-null
+        // which is used in http3framecodec to throw error
+        assertNotNull(http3Settings.put(Http3SettingIdentifier.HTTP3_SETTINGS_H3_DATAGRAM.id(), 1L));
+        // Ensure we can encode and decode all sizes correctly.
+        // unknown settings id/key will be ignored
+        assertNull(http3Settings.put(63, 63L));
+        assertNull(http3Settings.put(16383, 16383L));
+        assertNull(http3Settings.put(1073741823, 1073741823L));
+        assertNull(http3Settings.put(4611686018427387903L, 4611686018427387903L));
+        //even duplicates of unknown ignored as we ignore unknown
+        assertNull(http3Settings.put(4611686018427387903L, 4611686018427387903L));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ public final class Http3Settings implements Iterable<Map.Entry<Long, Long>> {`
`54`	`54`	`* Creates a new instance`
`55`	`55`	`*/`
`56`	`56`	`public Http3Settings() {`
`57`		`- this.settings = new LongObjectHashMap<>(4);`
	`57`	`+ this.settings = new LongObjectHashMap<>(Http3SettingIdentifier.values().length);`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`/**`
`@@ -99,7 +99,7 @@ public Long put(long key, Long value) {`
`99`	`99`
`100`	`100`	`// When Non-Standard/Unknown settings identifier identifier present - Ignore`
`101`	`101`	`if (identifier == null) {`
`102`		`- return value;`
	`102`	`+ return null;`
`103`	`103`	`}`
`104`	`104`
`105`	`105`	`//Validation`