Skip to content

okhttp: optimize HPACK to index :path and fix dynamic table eviction #12907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AgraVator wants to merge 2 commits into
base: master
Choose a base branch
from
+46 −23
Open

okhttp: optimize HPACK to index :path and fix dynamic table eviction #12907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a9a28c
okhttp: optimize HPACK to index :path and fix dynamic table eviction …
AgraVator Jul 13, 2026
d0272ef
okhttp: update HpackTest to verify unindexed pseudo header :method PU…
AgraVator Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Hpack.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,15 @@ private int evictToRecoverBytes(int bytesToRecover) {
int entriesToEvict = 0;
if (bytesToRecover > 0) {
// determine how many headers need to be evicted.
for (int j = dynamicTable.length - 1; j >= nextDynamicTableIndex && bytesToRecover > 0; j--) {
for (int j = dynamicTable.length - 1; j > nextDynamicTableIndex && bytesToRecover > 0; j--) {
bytesToRecover -= dynamicTable[j].hpackSize;
dynamicTableByteCount -= dynamicTable[j].hpackSize;
dynamicTableHeaderCount--;
entriesToEvict++;
}
System.arraycopy(dynamicTable, nextDynamicTableIndex + 1, dynamicTable,
nextDynamicTableIndex + 1 + entriesToEvict, dynamicTableHeaderCount);
Arrays.fill(dynamicTable, nextDynamicTableIndex + 1, nextDynamicTableIndex + 1 + entriesToEvict, null);
nextDynamicTableIndex += entriesToEvict;
}
return entriesToEvict;
Expand Down Expand Up @@ -489,17 +490,21 @@ void writeHeaders(List<io.grpc.okhttp.internal.framed.Header> headerBlock) throw
out.writeByte(0x40);
writeByteString(name);
writeByteString(value);
insertIntoDynamicTable(header);
} else if (name.startsWith(PSEUDO_PREFIX) && !io.grpc.okhttp.internal.framed.Header.TARGET_AUTHORITY.equals(name)) {
// Follow Chromes lead - only include the :authority pseudo header, but exclude all other
// pseudo headers. Literal Header Field without Indexing - Indexed Name.
insertIntoDynamicTable(new io.grpc.okhttp.internal.framed.Header(name, value));
} else if (name.startsWith(PSEUDO_PREFIX)
&& !io.grpc.okhttp.internal.framed.Header.TARGET_AUTHORITY.equals(name)
&& !io.grpc.okhttp.internal.framed.Header.TARGET_PATH.equals(name)) {
// Allow :authority and :path pseudo headers to be indexed. Other pseudo headers are not
// indexed.
// This is a departure from original Chrome behavior, as gRPC paths (ServiceName/MethodName)
// are stable and benefit from indexing.
writeInt(headerNameIndex, PREFIX_4_BITS, 0);
writeByteString(value);
} else {
// Literal Header Field with Incremental Indexing - Indexed Name.
writeInt(headerNameIndex, PREFIX_6_BITS, 0x40);
writeByteString(value);
insertIntoDynamicTable(header);
insertIntoDynamicTable(new io.grpc.okhttp.internal.framed.Header(name, value));
}
}
}
Expand Down Expand Up @@ -557,14 +562,15 @@ private int evictToRecoverBytes(int bytesToRecover) {
int entriesToEvict = 0;
if (bytesToRecover > 0) {
// determine how many headers need to be evicted.
for (int j = dynamicTable.length - 1; j >= nextDynamicTableIndex && bytesToRecover > 0; j--) {
for (int j = dynamicTable.length - 1; j > nextDynamicTableIndex && bytesToRecover > 0; j--) {
bytesToRecover -= dynamicTable[j].hpackSize;
dynamicTableByteCount -= dynamicTable[j].hpackSize;
dynamicTableHeaderCount--;
entriesToEvict++;
}
System.arraycopy(dynamicTable, nextDynamicTableIndex + 1, dynamicTable,
nextDynamicTableIndex + 1 + entriesToEvict, dynamicTableHeaderCount);
Arrays.fill(dynamicTable, nextDynamicTableIndex + 1, nextDynamicTableIndex + 1 + entriesToEvict, null);
nextDynamicTableIndex += entriesToEvict;
}
return entriesToEvict;
Expand Down
49 changes: 33 additions & 16 deletions okhttp/third_party/okhttp/test/java/io/grpc/okhttp/internal/framed/HpackTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,12 @@ public void readerEviction() throws IOException {
* http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-12#appendix-C.2.2
*/
@Test public void literalHeaderFieldWithoutIndexingIndexedName() throws IOException {
List<Header> headerBlock = headerEntries(":path", "/sample/path");
List<Header> headerBlock = headerEntries(":method", "PUT");

bytesIn.writeByte(0x04); // == Literal not indexed ==
// Indexed name (idx = 4) -> :path
bytesIn.writeByte(0x0c); // Literal value (len = 12)
bytesIn.writeUtf8("/sample/path");
bytesIn.writeByte(0x02); // == Literal not indexed ==
// Indexed name (idx = 2) -> :method
bytesIn.writeByte(0x03); // Literal value (len = 3)
bytesIn.writeUtf8("PUT");

hpackWriter.writeHeaders(headerBlock);
assertEquals(bytesIn, bytesOut);
Expand Down Expand Up @@ -1108,30 +1108,47 @@ public void doNotIndexPseudoHeaders() throws IOException {
hpackWriter.writeHeaders(headerEntries(":method", "PUT"));
assertBytes(0x02, 3, 'P', 'U', 'T');
assertEquals(0, hpackWriter.dynamicTableHeaderCount);

hpackWriter.writeHeaders(headerEntries(":path", "/okhttp"));
assertBytes(0x04, 7, '/', 'o', 'k', 'h', 't', 't', 'p');
assertEquals(0, hpackWriter.dynamicTableHeaderCount);
}

@Test
public void incrementalIndexingWithAuthorityPseudoHeader() throws IOException {
hpackWriter.writeHeaders(headerEntries(":authority", "foo.com"));
assertBytes(0x41, 7, 'f', 'o', 'o', '.', 'c', 'o', 'm');
public void pseudoHeaderIndexingForPathAndAuthority() throws IOException {
// :path should now be indexed
hpackWriter.writeHeaders(headerEntries(":path", "/okhttp"));
assertBytes(0x44, 7, '/', 'o', 'k', 'h', 't', 't', 'p');
assertEquals(1, hpackWriter.dynamicTableHeaderCount);
// Second call to same :path should be an index reference byte (0xbe)
hpackWriter.writeHeaders(headerEntries(":path", "/okhttp"));
assertBytes(0xbe);
assertEquals(1, hpackWriter.dynamicTableHeaderCount);

// :authority should be indexed
hpackWriter.writeHeaders(headerEntries(":authority", "foo.com"));
assertBytes(0x41, 7, 'f', 'o', 'o', '.', 'c', 'o', 'm');
assertEquals(2, hpackWriter.dynamicTableHeaderCount);
// Second call to same :authority should be an index reference byte (0xbe)
hpackWriter.writeHeaders(headerEntries(":authority", "foo.com"));
assertBytes(0xbe);
assertEquals(1, hpackWriter.dynamicTableHeaderCount);
assertEquals(2, hpackWriter.dynamicTableHeaderCount);

// If the :authority header somehow changes, it should be re-added to the dynamic table.
// If the :authority header value changes, it should be added as a new dynamic table entry.
hpackWriter.writeHeaders(headerEntries(":authority", "bar.com"));
assertBytes(0x41, 7, 'b', 'a', 'r', '.', 'c', 'o', 'm');
assertEquals(2, hpackWriter.dynamicTableHeaderCount);
assertEquals(3, hpackWriter.dynamicTableHeaderCount);

hpackWriter.writeHeaders(headerEntries(":authority", "bar.com"));
assertBytes(0xbe);
assertEquals(2, hpackWriter.dynamicTableHeaderCount);
assertEquals(3, hpackWriter.dynamicTableHeaderCount);
}

@Test
public void evictToRecoverBytesDoesNotNpeWhenBytesToRecoverExceedsTable() throws IOException {
hpackWriter.writeHeaders(headerEntries("custom-key", "custom-value"));
assertEquals(1, hpackWriter.dynamicTableHeaderCount);

// Force resize to table size 0, requiring total eviction exceeding table capacity
hpackWriter.resizeHeaderTable(0);

assertEquals(0, hpackWriter.dynamicTableHeaderCount);
}

@Test
Expand Down
Loading