Merge pull request #457 from adriancole/hpack-toggle

In HPACK, specifying the index of an referenced entry dereferences it.

okhttp-protocols/src/main/java/com/squareup/okhttp/internal/BitArray.java

@@ -65,12 +65,12 @@ public final class BitArray {
     data[offset] |= 1L << shiftOf(index);
   }
 
-  public void unset(int index) {
+  public void toggle(int index) {
     if (index < 0) {
       throw new IllegalArgumentException("index < 0: " + index);
     }
     int offset = offsetOf(index);
-    data[offset] &= ~(1L << shiftOf(index));
+    data[offset] ^= 1L << shiftOf(index);
   }
 
   public boolean get(int index) {

okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/HpackDraft05.java

@@ -251,11 +251,8 @@ final class HpackDraft05 {
           HeaderEntry staticEntry = STATIC_HEADER_TABLE[index - headerCount];
           insertIntoHeaderTable(-1, staticEntry);
         }
-      } else if (!referencedHeaders.get(headerTableIndex(index))) {
-        referencedHeaders.set(headerTableIndex(index));
       } else {
-        // TODO: we should throw something that we can coerce to a PROTOCOL_ERROR
-        throw new AssertionError("invalid index " + index);
+        referencedHeaders.toggle(headerTableIndex(index));
       }
     }
 

okhttp-protocols/src/test/java/com/squareup/okhttp/internal/BitArrayTest.java

@@ -20,6 +20,7 @@ import org.junit.Test;
 
 import static java.util.Arrays.asList;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 public class BitArrayTest {
@@ -30,11 +31,13 @@ public class BitArrayTest {
     assertEquals(asList(64), b.toIntegerList());
   }
 
-  @Test public void clearBit() {
+  @Test public void toggleBit() {
     BitArray b = new BitArray();
     b.set(100);
-    b.unset(100);
+    b.toggle(100);
     assertTrue(b.toIntegerList().isEmpty());
+    b.toggle(1);
+    assertEquals(asList(1), b.toIntegerList());
   }
 
   @Test public void shiftLeftExpandsData() {
@@ -106,7 +109,7 @@ public class BitArrayTest {
     b = b.shiftLeft(0xB0B);
     assertEquals(bigIntegerToString(b), a.toString());
 
-    a.unset(64280);
+    a.toggle(64280);
     b = b.clearBit(64280);
     assertEquals(bigIntegerToString(b), a.toString());
   }

okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/HpackDraft05Test.java

@@ -233,6 +233,33 @@ public class HpackDraft05Test {
     assertEquals(byteStringList(":method", "GET"), hpackReader.getAndReset());
   }
 
+  /**
+   * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05#section-3.2.1
+   */
+  @Test public void toggleIndex() throws IOException {
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+
+    // Static table entries are copied to the top of the reference set.
+    out.write(0x82); // == Indexed - Add ==
+                     // idx = 2 -> :method: GET
+    // Specifying an index to an entry in the reference set removes it.
+    out.write(0x81); // == Indexed - Remove ==
+                     // idx = 1 -> :method: GET
+
+    bytesIn.set(out.toByteArray());
+    hpackReader.readHeaders(out.size());
+    hpackReader.emitReferenceSet();
+
+    assertEquals(1, hpackReader.headerCount);
+    assertEquals(42, hpackReader.headerTableByteCount);
+
+    HpackDraft05.HeaderEntry entry = hpackReader.headerTable[headerTableLength() - 1];
+    checkEntry(entry, ":method", "GET", 42);
+    assertHeaderNotReferenced(headerTableLength() - 1);
+
+    assertTrue(hpackReader.getAndReset().isEmpty());
+  }
+
   /**
    * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05#appendix-E.1.4
    */