Revert "okhttp: Optimize HPACK to index :path"

This reverts commit 397d3e7.

Author: kannanjgithub

okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/framed/Hpack.java

@@ -490,14 +490,9 @@ void writeHeaders(List<io.grpc.okhttp.internal.framed.Header> headerBlock) throw
           writeByteString(name);
           writeByteString(value);
           insertIntoDynamicTable(header);
-        } 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.
+        } 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.
           writeInt(headerNameIndex, PREFIX_4_BITS, 0);
           writeByteString(value);
         } else {

okhttp/third_party/okhttp/test/java/io/grpc/okhttp/internal/framed/HpackTest.java

@@ -272,18 +272,14 @@ public void readerEviction() throws IOException {
 
   /**
    * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-12#appendix-C.2.2
-   *
-   * <p>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<Header> headerBlock = headerEntries(":method", "PUT");
+    List<Header> headerBlock = headerEntries(":path", "/sample/path");
 
-    bytesIn.writeByte(0x02); // == Literal not indexed ==
-    // Indexed name (idx = 2) -> :method
-    bytesIn.writeByte(0x03); // Literal value (len = 3)
-    bytesIn.writeUtf8("PUT");
+    bytesIn.writeByte(0x04); // == Literal not indexed ==
+    // Indexed name (idx = 4) -> :path
+    bytesIn.writeByte(0x0c); // Literal value (len = 12)
+    bytesIn.writeUtf8("/sample/path");
 
     hpackWriter.writeHeaders(headerBlock);
     assertEquals(bytesIn, bytesOut);
@@ -1108,29 +1104,14 @@ public void dynamicTableIndexedHeader() throws IOException {
   }
 
   @Test
-  public void pseudoHeaderIndexing() throws IOException {
-    // :method is not indexed (unless it's GET or POST, which are in the static table)
+  public void doNotIndexPseudoHeaders() throws IOException {
     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(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);
+    assertBytes(0x04, 7, '/', 'o', 'k', 'h', 't', 't', 'p');
+    assertEquals(0, hpackWriter.dynamicTableHeaderCount);
   }
 
   @Test