diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackDecoder.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackDecoder.java index 880d0ccbd..06009233c 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackDecoder.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackDecoder.java @@ -36,6 +36,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import org.apache.hc.core5.annotation.Internal; import org.apache.hc.core5.http.Header; @@ -62,10 +63,10 @@ public final class HPackDecoder { private int maxListSize; HPackDecoder(final InboundDynamicTable dynamicTable, final CharsetDecoder charsetDecoder) { - this.dynamicTable = dynamicTable != null ? dynamicTable : new InboundDynamicTable(); + this.dynamicTable = Objects.requireNonNull(dynamicTable); this.contentBuf = new ByteArrayBuffer(256); this.charsetDecoder = charsetDecoder; - this.maxTableSize = dynamicTable != null ? dynamicTable.getMaxSize() : Integer.MAX_VALUE; + this.maxTableSize = this.dynamicTable.getMaxSize(); this.maxListSize = Integer.MAX_VALUE; } @@ -73,12 +74,12 @@ public final class HPackDecoder { this(dynamicTable, charset != null && !StandardCharsets.US_ASCII.equals(charset) ? charset.newDecoder() : null); } - public HPackDecoder(final Charset charset) { - this(new InboundDynamicTable(), charset); + public HPackDecoder(final int maxTableSize, final Charset charset) { + this(new InboundDynamicTable(maxTableSize), charset); } - public HPackDecoder(final CharsetDecoder charsetDecoder) { - this(new InboundDynamicTable(), charsetDecoder); + public HPackDecoder(final int maxTableSize, final CharsetDecoder charsetDecoder) { + this(new InboundDynamicTable(maxTableSize), charsetDecoder); } static int readByte(final ByteBuffer src) throws HPackException { @@ -284,7 +285,10 @@ HPackHeader decodeHPackHeader(final ByteBuffer src) throws HPackException { return decodeLiteralHeader(src, HPackRepresentation.NEVER_INDEXED); } else if ((b & 0xe0) == 0x20) { final int maxSize = decodeInt(src, 5); - this.dynamicTable.setMaxSize(Math.min(this.maxTableSize, maxSize)); + if (maxSize > this.maxTableSize) { + throw new HPackException("Requested dynamic header table size exceeds maximum size: " + maxSize); + } + this.dynamicTable.setMaxSize(maxSize); } else { throw new HPackException("Unexpected header first byte: 0x" + Integer.toHexString(b)); } @@ -323,7 +327,7 @@ public int getMaxTableSize() { public void setMaxTableSize(final int maxTableSize) { Args.notNegative(maxTableSize, "Max table size"); this.maxTableSize = maxTableSize; - this.dynamicTable.setMaxSize(maxTableSize); + this.dynamicTable.setMaxSize(Math.min(this.dynamicTable.getMaxSize(), maxTableSize)); } public int getMaxListSize() { diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackEncoder.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackEncoder.java index b64b27a51..a06c5c9d0 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackEncoder.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackEncoder.java @@ -57,21 +57,22 @@ public final class HPackEncoder { private int maxTableSize; HPackEncoder(final OutboundDynamicTable dynamicTable, final CharsetEncoder charsetEncoder) { - this.dynamicTable = dynamicTable != null ? dynamicTable : new OutboundDynamicTable(); + this.dynamicTable = Objects.requireNonNull(dynamicTable); this.huffmanBuf = new ByteArrayBuffer(128); this.charsetEncoder = charsetEncoder; + this.maxTableSize = this.dynamicTable.getMaxSize(); } HPackEncoder(final OutboundDynamicTable dynamicTable, final Charset charset) { this(dynamicTable, charset != null && !StandardCharsets.US_ASCII.equals(charset) ? charset.newEncoder() : null); } - public HPackEncoder(final Charset charset) { - this(new OutboundDynamicTable(), charset); + public HPackEncoder(final int maxTableSize, final Charset charset) { + this(new OutboundDynamicTable(maxTableSize), charset); } - public HPackEncoder(final CharsetEncoder charsetEncoder) { - this(new OutboundDynamicTable(), charsetEncoder); + public HPackEncoder(final int maxTableSize, final CharsetEncoder charsetEncoder) { + this(new OutboundDynamicTable(maxTableSize), charsetEncoder); } static void encodeInt(final ByteArrayBuffer dst, final int n, final int i, final int mask) { @@ -261,6 +262,11 @@ void encodeHeader( void encodeHeader( final ByteArrayBuffer dst, final String name, final String value, final boolean sensitive, final boolean noIndexing, final boolean useHuffman) throws CharacterCodingException { + //send receiver the updated dynamic table size + if (maxTableSize != this.dynamicTable.getMaxSize()) { + encodeInt(dst, 5, maxTableSize, 0x20); + this.dynamicTable.setMaxSize(maxTableSize); + } final HPackRepresentation representation; if (sensitive) { @@ -336,7 +342,6 @@ public int getMaxTableSize() { public void setMaxTableSize(final int maxTableSize) { Args.notNegative(maxTableSize, "Max table size"); this.maxTableSize = maxTableSize; - this.dynamicTable.setMaxSize(maxTableSize); } } diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/InboundDynamicTable.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/InboundDynamicTable.java index 28c3f0b72..88e10f085 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/InboundDynamicTable.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/InboundDynamicTable.java @@ -39,15 +39,19 @@ final class InboundDynamicTable { private int maxSize; private int currentSize; - InboundDynamicTable(final StaticTable staticTable) { + InboundDynamicTable(final int maxSize, final StaticTable staticTable) { this.staticTable = staticTable; this.headers = new FifoBuffer(256); - this.maxSize = Integer.MAX_VALUE; + this.maxSize = maxSize; this.currentSize = 0; } + InboundDynamicTable(final int maxSize) { + this(maxSize, StaticTable.INSTANCE); + } + InboundDynamicTable() { - this(StaticTable.INSTANCE); + this(Integer.MAX_VALUE); } public int getMaxSize() { diff --git a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/OutboundDynamicTable.java b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/OutboundDynamicTable.java index 3791b48af..92aed4659 100644 --- a/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/OutboundDynamicTable.java +++ b/httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/OutboundDynamicTable.java @@ -45,16 +45,20 @@ final class OutboundDynamicTable { private int maxSize; private int currentSize; - OutboundDynamicTable(final StaticTable staticTable) { + OutboundDynamicTable(final int maxSize, final StaticTable staticTable) { this.staticTable = staticTable; this.headers = new FifoLinkedList(); this.mapByName = new HashMap<>(); - this.maxSize = Integer.MAX_VALUE; + this.maxSize = maxSize; this.currentSize = 0; } + OutboundDynamicTable(final int maxSize) { + this(maxSize, StaticTable.INSTANCE); + } + OutboundDynamicTable() { - this(StaticTable.INSTANCE); + this(Integer.MAX_VALUE); } public int getMaxSize() { 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 431ee5b0b..12b063219 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 @@ -159,8 +159,8 @@ enum SettingsHandshake { READY, TRANSMITTED, ACKED } this.pingHandlers = new ConcurrentLinkedQueue<>(); this.outputRequests = new AtomicInteger(0); this.lastStreamId = new AtomicInteger(0); - this.hPackEncoder = new HPackEncoder(CharCodingSupport.createEncoder(charCodingConfig)); - this.hPackDecoder = new HPackDecoder(CharCodingSupport.createDecoder(charCodingConfig)); + this.hPackEncoder = new HPackEncoder(H2Config.INIT.getHeaderTableSize(), CharCodingSupport.createEncoder(charCodingConfig)); + this.hPackDecoder = new HPackDecoder(H2Config.INIT.getHeaderTableSize(), CharCodingSupport.createDecoder(charCodingConfig)); this.streamMap = new ConcurrentHashMap<>(); this.remoteConfig = H2Config.INIT; this.connInputWindow = new AtomicInteger(H2Config.INIT.getInitialWindowSize()); @@ -169,8 +169,6 @@ enum SettingsHandshake { READY, TRANSMITTED, ACKED } this.initInputWinSize = H2Config.INIT.getInitialWindowSize(); this.initOutputWinSize = H2Config.INIT.getInitialWindowSize(); - this.hPackDecoder.setMaxTableSize(H2Config.INIT.getHeaderTableSize()); - this.hPackEncoder.setMaxTableSize(H2Config.INIT.getHeaderTableSize()); this.hPackDecoder.setMaxListSize(H2Config.INIT.getMaxHeaderListSize()); this.lowMark = H2Config.INIT.getInitialWindowSize() / 2; diff --git a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/hpack/TestHPackCoding.java b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/hpack/TestHPackCoding.java index dba042bd2..b93ed6084 100644 --- a/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/hpack/TestHPackCoding.java +++ b/httpcore5-h2/src/test/java/org/apache/hc/core5/http2/hpack/TestHPackCoding.java @@ -210,8 +210,8 @@ void testHuffmanEncoding() { @Test void testBasicStringCoding() throws Exception { - final HPackEncoder encoder = new HPackEncoder(StandardCharsets.US_ASCII); - final HPackDecoder decoder = new HPackDecoder(StandardCharsets.US_ASCII); + final HPackEncoder encoder = new HPackEncoder(Integer.MAX_VALUE, StandardCharsets.US_ASCII); + final HPackDecoder decoder = new HPackDecoder(Integer.MAX_VALUE, StandardCharsets.US_ASCII); final ByteArrayBuffer buffer = new ByteArrayBuffer(16); encoder.encodeString(buffer, "this and that", false); @@ -230,8 +230,8 @@ void testBasicStringCoding() throws Exception { @Test void testEnsureCapacity() throws Exception { - final HPackEncoder encoder = new HPackEncoder(StandardCharsets.US_ASCII); - final HPackDecoder decoder = new HPackDecoder(StandardCharsets.UTF_8); + final HPackEncoder encoder = new HPackEncoder(Integer.MAX_VALUE, StandardCharsets.US_ASCII); + final HPackDecoder decoder = new HPackDecoder(Integer.MAX_VALUE, StandardCharsets.UTF_8); final ByteArrayBuffer buffer = new ByteArrayBuffer(16); encoder.encodeString(buffer, "this and that", false); @@ -274,8 +274,8 @@ void testComplexStringCoding1() throws Exception { final ByteArrayBuffer buffer = new ByteArrayBuffer(16); final StringBuilder strBuf = new StringBuilder(); - final HPackEncoder encoder = new HPackEncoder(charset); - final HPackDecoder decoder = new HPackDecoder(charset); + final HPackEncoder encoder = new HPackEncoder(Integer.MAX_VALUE, charset); + final HPackDecoder decoder = new HPackDecoder(Integer.MAX_VALUE, charset); for (int n = 0; n < 10; n++) { @@ -302,8 +302,8 @@ void testComplexStringCoding2() throws Exception { final ByteArrayBuffer buffer = new ByteArrayBuffer(16); final StringBuilder strBuf = new StringBuilder(); - final HPackEncoder encoder = new HPackEncoder(charset); - final HPackDecoder decoder = new HPackDecoder(charset); + final HPackEncoder encoder = new HPackEncoder(Integer.MAX_VALUE, charset); + final HPackDecoder decoder = new HPackDecoder(Integer.MAX_VALUE, charset); for (int n = 0; n < 10; n++) { @@ -1061,8 +1061,8 @@ void testHeaderEntrySizeNonAscii() throws Exception { @Test void testHeaderSizeLimit() throws Exception { - final HPackEncoder encoder = new HPackEncoder(StandardCharsets.US_ASCII); - final HPackDecoder decoder = new HPackDecoder(StandardCharsets.US_ASCII); + final HPackEncoder encoder = new HPackEncoder(Integer.MAX_VALUE, StandardCharsets.US_ASCII); + final HPackDecoder decoder = new HPackDecoder(Integer.MAX_VALUE, StandardCharsets.US_ASCII); final ByteArrayBuffer buf = new ByteArrayBuffer(128); @@ -1086,8 +1086,8 @@ void testHeaderSizeLimit() throws Exception { @Test void testHeaderEmptyASCII() throws Exception { - final HPackEncoder encoder = new HPackEncoder(StandardCharsets.US_ASCII); - final HPackDecoder decoder = new HPackDecoder(StandardCharsets.US_ASCII); + final HPackEncoder encoder = new HPackEncoder(Integer.MAX_VALUE, StandardCharsets.US_ASCII); + final HPackDecoder decoder = new HPackDecoder(Integer.MAX_VALUE, StandardCharsets.US_ASCII); final ByteArrayBuffer buf = new ByteArrayBuffer(128); @@ -1100,8 +1100,8 @@ void testHeaderEmptyASCII() throws Exception { @Test void testHeaderEmptyUTF8() throws Exception { - final HPackEncoder encoder = new HPackEncoder(StandardCharsets.UTF_8); - final HPackDecoder decoder = new HPackDecoder(StandardCharsets.UTF_8); + final HPackEncoder encoder = new HPackEncoder(Integer.MAX_VALUE, StandardCharsets.UTF_8); + final HPackDecoder decoder = new HPackDecoder(Integer.MAX_VALUE, StandardCharsets.UTF_8); final ByteArrayBuffer buf = new ByteArrayBuffer(128); @@ -1111,5 +1111,74 @@ void testHeaderEmptyUTF8() throws Exception { assertHeaderEquals(header, decoder.decodeHeader(wrap(buf))); } + @Test + void encoderDynamicHeaderTableMaxSizeNotIncreasedBySettingsFrame() throws Exception { + final OutboundDynamicTable dynamicTable = new OutboundDynamicTable(4096); + final HPackEncoder encoder = new HPackEncoder(dynamicTable, StandardCharsets.UTF_8); + //emulate receiving a settings frame from the receiver + encoder.setMaxTableSize(Integer.MAX_VALUE); + //actual table size should not change until we are able to send an update to the receiver + Assertions.assertEquals(4096, dynamicTable.getMaxSize()); + } + + @Test + void encoderDynamicHeaderTableMaxSizeChangeCausesUpdateHeader() throws Exception { + final OutboundDynamicTable dynamicTable = new OutboundDynamicTable(4096); + final HPackEncoder encoder = new HPackEncoder(dynamicTable, StandardCharsets.UTF_8); + //emulate receiving a settings frame from the receiver + encoder.setMaxTableSize(8192); + + final ByteArrayBuffer buf = new ByteArrayBuffer(128); + + final Header header = new BasicHeader("empty-header", ""); + encoder.encodeHeader(buf, header); + + final int firstByte = buf.byteAt(0); + final int masked = firstByte & 0xe0; + + //first header field is table size update + Assertions.assertEquals(0x20, masked); + + //update has new table size as value + Assertions.assertEquals(8192, HPackDecoder.decodeInt(wrap(buf), 5)); + } + + @Test + void decoderDynamicHeaderTableMaxSizeNotIncreasedBySettingsFrame() throws Exception { + final InboundDynamicTable dynamicTable = new InboundDynamicTable(4096); + final HPackDecoder decoder = new HPackDecoder(dynamicTable, StandardCharsets.UTF_8); + //emulate something on our side changing the max dynamic table size + //this would cause us to send a new settings frame to the sender + decoder.setMaxTableSize(Integer.MAX_VALUE); + //actual table size should not change until sender sends us a table update + Assertions.assertEquals(4096, dynamicTable.getMaxSize()); + } + + @Test + void decoderDynamicHeaderTableMaxSizeUpdatesAfter() throws Exception { + final InboundDynamicTable dynamicTable = new InboundDynamicTable(4096); + final HPackDecoder decoder = new HPackDecoder(dynamicTable, StandardCharsets.UTF_8); + decoder.setMaxTableSize(Integer.MAX_VALUE); + + final ByteArrayBuffer buf = new ByteArrayBuffer(128); + HPackEncoder.encodeInt(buf, 5, 8192, 0x20); + + decoder.decodeHeaders(wrap(buf)); + + Assertions.assertEquals(8192, dynamicTable.getMaxSize()); + } + + @Test + void decoderDynamicHeaderTableMaxSizeLimitedByConfig() throws Exception { + final InboundDynamicTable dynamicTable = new InboundDynamicTable(4096); + final HPackDecoder decoder = new HPackDecoder(dynamicTable, StandardCharsets.UTF_8); + //do not increase max size, this should limit requests from the receiver to increase max size + + //emulate receiving header that illegally increases the table size above our max + final ByteArrayBuffer buf = new ByteArrayBuffer(128); + HPackEncoder.encodeInt(buf, 5, 8192, 0x20); + Assertions.assertThrows(HPackException.class, () -> decoder.decodeHeaders(wrap(buf))); + } + }