From ed07722f62546cb5ff70b254ed1b7766bf595185 Mon Sep 17 00:00:00 2001 From: Mingyu Kang Date: Tue, 5 May 2026 17:48:56 -0700 Subject: [PATCH] okhttp: Optimize HPACK to index :path Allow the :path pseudo-header to be added to the HPACK dynamic table in the gRPC-Java OkHttp transport. In gRPC, the :path header typically represents the service and method name (e.g., /package.Service/Method), which is constant across multiple calls to the same method. The previous HPACK implementation, inherited from OkHttp, did not index :path, treating it as potentially highly variable. By allowing :path to be indexed, subsequent gRPC calls to the same method can benefit from HPACK's dynamic table, reducing header size. --- .../io/grpc/okhttp/internal/framed/Hpack.java | 11 ++++-- .../okhttp/internal/framed/HpackTest.java | 35 ++++++++++++++----- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Hpack.java b/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Hpack.java index 3155d6d533a..fa9458e2024 100644 --- a/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Hpack.java +++ b/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Hpack.java @@ -490,9 +490,14 @@ void writeHeaders(List headerBlock) throw 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. + } 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 the original Chrome-inspired behavior, as gRPC paths + // (ServiceName/MethodName) + // are stable and benefit from indexing. writeInt(headerNameIndex, PREFIX_4_BITS, 0); writeByteString(value); } else { diff --git a/okhttp/third_party/okhttp/test/java/io/grpc/okhttp/internal/framed/HpackTest.java b/okhttp/third_party/okhttp/test/java/io/grpc/okhttp/internal/framed/HpackTest.java index dc5e030810f..8635151c6b9 100644 --- a/okhttp/third_party/okhttp/test/java/io/grpc/okhttp/internal/framed/HpackTest.java +++ b/okhttp/third_party/okhttp/test/java/io/grpc/okhttp/internal/framed/HpackTest.java @@ -272,14 +272,18 @@ public void readerEviction() throws IOException { /** * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-12#appendix-C.2.2 + * + *

This test mimics the draft example which uses ":path", but since gRPC-Java now indexes + * ":path" for performance, we use ":method" (with a non-static value like "PUT") to verify the + * "Literal Header Field without Indexing - Indexed Name" representation. */ @Test public void literalHeaderFieldWithoutIndexingIndexedName() throws IOException { - List

headerBlock = headerEntries(":path", "/sample/path"); + List
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); @@ -1104,14 +1108,29 @@ public void dynamicTableIndexedHeader() throws IOException { } @Test - public void doNotIndexPseudoHeaders() throws IOException { + public void pseudoHeaderIndexing() throws IOException { + // :method is not indexed (unless it's GET or POST, which are in the static table) hpackWriter.writeHeaders(headerEntries(":method", "PUT")); assertBytes(0x02, 3, 'P', 'U', 'T'); assertEquals(0, hpackWriter.dynamicTableHeaderCount); + // :path should now be indexed hpackWriter.writeHeaders(headerEntries(":path", "/okhttp")); - assertBytes(0x04, 7, '/', 'o', 'k', 'h', 't', 't', 'p'); - assertEquals(0, hpackWriter.dynamicTableHeaderCount); + assertBytes(0x44, 7, '/', 'o', 'k', 'h', 't', 't', 'p'); + assertEquals(1, hpackWriter.dynamicTableHeaderCount); + // Second time should be an index + hpackWriter.writeHeaders(headerEntries(":path", "/okhttp")); + assertBytes(0xbe); + assertEquals(1, hpackWriter.dynamicTableHeaderCount); + + // :authority should be indexed + hpackWriter.writeHeaders(headerEntries(":authority", "test.com")); + assertBytes(0x41, 8, 't', 'e', 's', 't', '.', 'c', 'o', 'm'); + assertEquals(2, hpackWriter.dynamicTableHeaderCount); + // Second time should be an index + hpackWriter.writeHeaders(headerEntries(":authority", "test.com")); + assertBytes(0xbe); + assertEquals(2, hpackWriter.dynamicTableHeaderCount); } @Test