From ef729e70213adbfca7b26c2c337eb5d12839fbad Mon Sep 17 00:00:00 2001 From: nmittler Date: Wed, 1 Apr 2015 07:51:11 -0700 Subject: [PATCH] Allow non-standard HTTP/2 settings Motivation: The Http2Settings class currently disallows setting non-standard settings, which violates the spec. Modifications: Updated Http2Settings to permit arbitrary settings. Also adjusting the default initial capacity to allow setting all of the standard settings without reallocation. Result: Fixes #3560 --- .../handler/codec/http2/Http2CodecUtil.java | 1 + .../handler/codec/http2/Http2Settings.java | 23 +++++++++++++++---- .../codec/http2/Http2SettingsTest.java | 6 +++++ .../util/collection/IntObjectHashMap.java | 4 ++-- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java index 308ac81ea6..88bdd859ba 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java @@ -69,6 +69,7 @@ public final class Http2CodecUtil { public static final int SETTINGS_INITIAL_WINDOW_SIZE = 4; public static final int SETTINGS_MAX_FRAME_SIZE = 5; public static final int SETTINGS_MAX_HEADER_LIST_SIZE = 6; + public static final int NUM_STANDARD_SETTINGS = 6; public static final int MAX_HEADER_TABLE_SIZE = Integer.MAX_VALUE; // Size limited by HPACK library public static final long MAX_CONCURRENT_STREAMS = MAX_UNSIGNED_INT; diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Settings.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Settings.java index 574145dc5e..c2fd28787d 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Settings.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Settings.java @@ -23,6 +23,7 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_HEADER_TABLE_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_CONCURRENT_STREAMS; import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_INITIAL_WINDOW_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_HEADER_LIST_SIZE; +import static io.netty.handler.codec.http2.Http2CodecUtil.NUM_STANDARD_SETTINGS; import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_ENABLE_PUSH; import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_HEADER_TABLE_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_INITIAL_WINDOW_SIZE; @@ -39,9 +40,14 @@ import io.netty.util.collection.IntObjectHashMap; * methods for standard settings. */ public final class Http2Settings extends IntObjectHashMap { + /** + * Default capacity based on the number of standard settings from the HTTP/2 spec, adjusted so that adding all of + * the standard settings will not cause the map capacity to change. + */ + private static final int DEFAULT_CAPACITY = (int) (NUM_STANDARD_SETTINGS / DEFAULT_LOAD_FACTOR) + 1; public Http2Settings() { - this(6 /* number of standard settings */); + this(DEFAULT_CAPACITY); } public Http2Settings(int initialCapacity, float loadFactor) { @@ -53,9 +59,10 @@ public final class Http2Settings extends IntObjectHashMap { } /** - * Overrides the superclass method to perform verification of standard HTTP/2 settings. + * Adds the given setting key/value pair. For standard settings defined by the HTTP/2 spec, performs + * validation on the values. * - * @throws IllegalArgumentException if verification of the setting fails. + * @throws IllegalArgumentException if verification for a standard HTTP/2 setting fails. */ @Override public Long put(int key, Long value) { @@ -176,7 +183,12 @@ public final class Http2Settings extends IntObjectHashMap { return this; } - Integer getIntValue(int key) { + /** + * A helper method that returns {@link Long#intValue()} on the return of {@link #get(int)}, if present. Note that + * if the range of the value exceeds {@link Integer#MAX_VALUE}, the {@link #get(int)} method should + * be used instead to avoid truncation of the value. + */ + public Integer getIntValue(int key) { Long value = get(key); if (value == null) { return null; @@ -220,7 +232,8 @@ public final class Http2Settings extends IntObjectHashMap { } break; default: - throw new IllegalArgumentException("key"); + // Non-standard HTTP/2 setting - don't do validation. + break; } } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2SettingsTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2SettingsTest.java index 38e8d2ce7f..06c25d7fa8 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2SettingsTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2SettingsTest.java @@ -61,4 +61,10 @@ public class Http2SettingsTest { assertEquals(MAX_FRAME_SIZE_UPPER_BOUND, (int) settings.maxFrameSize()); assertEquals(4L, (long) settings.maxHeaderListSize()); } + + @Test + public void nonStandardSettingsShouldBeSet() { + settings.put(0, 123L); + assertEquals(123L, (long) settings.get(0)); + } } diff --git a/common/src/main/java/io/netty/util/collection/IntObjectHashMap.java b/common/src/main/java/io/netty/util/collection/IntObjectHashMap.java index c60e9bdda8..4ed354dd3a 100644 --- a/common/src/main/java/io/netty/util/collection/IntObjectHashMap.java +++ b/common/src/main/java/io/netty/util/collection/IntObjectHashMap.java @@ -33,10 +33,10 @@ import java.util.NoSuchElementException; public class IntObjectHashMap implements IntObjectMap, Iterable> { /** Default initial capacity. Used if not specified in the constructor */ - private static final int DEFAULT_CAPACITY = 11; + public static final int DEFAULT_CAPACITY = 11; /** Default load factor. Used if not specified in the constructor */ - private static final float DEFAULT_LOAD_FACTOR = 0.5f; + public static final float DEFAULT_LOAD_FACTOR = 0.5f; /** * Placeholder for null values, so we can use the actual null to mean available.