Skip to content

Commit

Permalink
Removed ability to change the HPACK decoder header table size setting.
Browse files Browse the repository at this point in the history
Previously, we exposed this method and called it when receiving the
header table size setting from a remote peer. We concluded this was not
the intent of the spec and removed that code.
  • Loading branch information
dave-r12 committed Jul 2, 2016
1 parent e49bf70 commit ffd9dbe
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 26 deletions.
60 changes: 49 additions & 11 deletions okhttp-tests/src/test/java/okhttp3/internal/framed/HpackTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ public class HpackTest {
* larger header content is not lost.
*/
@Test public void tooLargeToHPackIsStillEmitted() throws IOException {
bytesIn.writeByte(0x21); // Dynamic table size update (size = 1).
bytesIn.writeByte(0x00); // Literal indexed
bytesIn.writeByte(0x0a); // Literal name (len = 10)
bytesIn.writeUtf8("custom-key");

bytesIn.writeByte(0x0d); // Literal value (len = 13)
bytesIn.writeUtf8("custom-header");

hpackReader.headerTableSizeSetting(1);
hpackReader.readHeaders();

assertEquals(0, hpackReader.headerCount);
Expand All @@ -81,7 +81,7 @@ public class HpackTest {
}

/** Oldest entries are evicted to support newer ones. */
@Test public void testEviction() throws IOException {
@Test public void writerEviction() throws IOException {
List<Header> headerBlock =
headerEntries(
"custom-foo", "custom-header",
Expand Down Expand Up @@ -124,30 +124,69 @@ public class HpackTest {

entry = writer.dynamicTable[tableLength - 2];
checkEntry(entry, "custom-baz", "custom-header", 55);
}

@Test public void readerEviction() throws IOException {
List<Header> headerBlock =
headerEntries(
"custom-foo", "custom-header",
"custom-bar", "custom-header",
"custom-baz", "custom-header");

// Set to only support 110 bytes (enough for 2 headers).
hpackReader.headerTableSizeSetting(110);
bytesIn.writeByte(0x3F); // Dynamic table size update (size = 110).
bytesIn.writeByte(0x4F);

bytesIn.writeByte(0x40); // Literal indexed
bytesIn.writeByte(0x0a); // Literal name (len = 10)
bytesIn.writeUtf8("custom-foo");

bytesIn.writeByte(0x0d); // Literal value (len = 13)
bytesIn.writeUtf8("custom-header");

bytesIn.writeByte(0x40); // Literal indexed
bytesIn.writeByte(0x0a); // Literal name (len = 10)
bytesIn.writeUtf8("custom-bar");

bytesIn.writeByte(0x0d); // Literal value (len = 13)
bytesIn.writeUtf8("custom-header");

bytesIn.writeByte(0x40); // Literal indexed
bytesIn.writeByte(0x0a); // Literal name (len = 10)
bytesIn.writeUtf8("custom-baz");

bytesIn.writeByte(0x0d); // Literal value (len = 13)
bytesIn.writeUtf8("custom-header");

hpackReader.readHeaders();

assertEquals(2, hpackReader.headerCount);

entry = hpackReader.dynamicTable[readerHeaderTableLength() - 1];
checkEntry(entry, "custom-bar", "custom-header", 55);
Header entry1 = hpackReader.dynamicTable[readerHeaderTableLength() - 1];
checkEntry(entry1, "custom-bar", "custom-header", 55);

entry = hpackReader.dynamicTable[readerHeaderTableLength() - 2];
checkEntry(entry, "custom-baz", "custom-header", 55);
Header entry2 = hpackReader.dynamicTable[readerHeaderTableLength() - 2];
checkEntry(entry2, "custom-baz", "custom-header", 55);

// Once a header field is decoded and added to the reconstructed header
// list, it cannot be removed from it. Hence, foo is here.
assertEquals(headerBlock, hpackReader.getAndResetHeaderList());

// Simulate receiving a small settings frame, that implies eviction.
hpackReader.headerTableSizeSetting(55);
// Simulate receiving a small dynamic table size update, that implies eviction.
bytesIn.writeByte(0x3F); // Dynamic table size update (size = 55).
bytesIn.writeByte(0x18);
hpackReader.readHeaders();
assertEquals(1, hpackReader.headerCount);
}

/** Header table backing array is initially 8 long, let's ensure it grows. */
@Test public void dynamicallyGrowsBeyond64Entries() throws IOException {
// Lots of headers need more room!
hpackReader = new Hpack.Reader(16384, 4096, bytesIn);
bytesIn.writeByte(0x3F); // Dynamic table size update (size = 16384).
bytesIn.writeByte(0xE1);
bytesIn.writeByte(0x7F);

for (int i = 0; i < 256; i++) {
bytesIn.writeByte(0x40); // Literal indexed
bytesIn.writeByte(0x0a); // Literal name (len = 10)
Expand All @@ -157,7 +196,6 @@ public class HpackTest {
bytesIn.writeUtf8("custom-header");
}

hpackReader.headerTableSizeSetting(16384); // Lots of headers need more room!
hpackReader.readHeaders();

assertEquals(256, hpackReader.headerCount);
Expand Down Expand Up @@ -435,10 +473,10 @@ public class HpackTest {
* http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-12#appendix-C.2.4
*/
@Test public void readIndexedHeaderFieldFromStaticTableWithoutBuffering() throws IOException {
bytesIn.writeByte(0x20); // Dynamic table size update (size = 0).
bytesIn.writeByte(0x82); // == Indexed - Add ==
// idx = 2 -> :method: GET

hpackReader.headerTableSizeSetting(0); // SETTINGS_HEADER_TABLE_SIZE == 0
hpackReader.readHeaders();

// Not buffered in header table.
Expand Down
22 changes: 7 additions & 15 deletions okhttp/src/main/java/okhttp3/internal/framed/Hpack.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,9 @@ static final class Reader {
private final List<Header> headerList = new ArrayList<>();
private final BufferedSource source;

private int headerTableSizeSetting;
private final int headerTableSizeSetting;
private int maxDynamicTableByteCount;

// Visible for testing.
Header[] dynamicTable = new Header[8];
// Array is populated back to front, so new entries always have lowest index.
Expand All @@ -126,28 +127,19 @@ static final class Reader {
int dynamicTableByteCount = 0;

Reader(int headerTableSizeSetting, Source source) {
this(headerTableSizeSetting, headerTableSizeSetting, source);
}

Reader(int headerTableSizeSetting, int maxDynamicTableByteCount, Source source) {
this.headerTableSizeSetting = headerTableSizeSetting;
this.maxDynamicTableByteCount = headerTableSizeSetting;
this.maxDynamicTableByteCount = maxDynamicTableByteCount;
this.source = Okio.buffer(source);
}

int maxDynamicTableByteCount() {
return maxDynamicTableByteCount;
}

/**
* Called by the reader when the peer sent {@link Settings#HEADER_TABLE_SIZE}. While this
* establishes the maximum dynamic table size, the {@link #maxDynamicTableByteCount} set during
* processing may limit the table size to a smaller amount.
*
* <p>Evicts entries or clears the table as needed.
*/
void headerTableSizeSetting(int headerTableSizeSetting) {
this.headerTableSizeSetting = headerTableSizeSetting;
this.maxDynamicTableByteCount = headerTableSizeSetting;
adjustDynamicTableByteCount();
}

private void adjustDynamicTableByteCount() {
if (maxDynamicTableByteCount < dynamicTableByteCount) {
if (maxDynamicTableByteCount == 0) {
Expand Down

0 comments on commit ffd9dbe

Please sign in to comment.