Skip to content

Commit

Permalink
Properly alert receivers when using non-default dynamic table sizes
Browse files Browse the repository at this point in the history 
Implements https://datatracker.ietf.org/doc/html/rfc7541#section-6.3
  • Loading branch information
@ok2c
Brendan Thomas authored and ok2c committed Oct 12, 2024
1 parent 549e239 commit 3509d45
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 38 deletions.
20 changes: 12 additions & 8 deletions httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -62,23 +63,23 @@ 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;
}

HPackDecoder(final InboundDynamicTable dynamicTable, final Charset charset) {
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 {
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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() {
Expand Down
17 changes: 11 additions & 6 deletions httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/HPackEncoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}

}
10 changes: 7 additions & 3 deletions httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/InboundDynamicTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
10 changes: 7 additions & 3 deletions httpcore5-h2/src/main/java/org/apache/hc/core5/http2/hpack/OutboundDynamicTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
6 changes: 2 additions & 4 deletions ...ore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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;
Expand Down
97 changes: 83 additions & 14 deletions httpcore5-h2/src/test/java/org/apache/hc/core5/http2/hpack/TestHPackCoding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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++) {

Expand All @@ -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++) {

Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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)));
}

}

0 comments on commit 3509d45

Please sign in to comment.