From 0b3ec2b55d4ecbaf3cbfecee79599a71ac1481d2 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Wed, 29 Jan 2014 10:23:02 -0800 Subject: [PATCH] HPACK: Write headers using indexed names where possible. --- .../okhttp/internal/spdy/HpackDraft05.java | 36 +++++- .../internal/spdy/HpackDraft05Test.java | 104 +++++++++++------- .../internal/spdy/Http20Draft09Test.java | 9 +- 3 files changed, 97 insertions(+), 52 deletions(-) diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/HpackDraft05.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/HpackDraft05.java index 36620689d..0bd8b0677 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/HpackDraft05.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/HpackDraft05.java @@ -1,14 +1,17 @@ package com.squareup.okhttp.internal.spdy; import com.squareup.okhttp.internal.BitArray; -import com.squareup.okhttp.internal.bytes.ByteString; import com.squareup.okhttp.internal.Util; +import com.squareup.okhttp.internal.bytes.ByteString; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import static com.squareup.okhttp.internal.Util.asciiLowerCase; @@ -188,7 +191,7 @@ final class HpackDraft05 { } else { // 0NNNNNNN if (b == 0x40) { // 01000000 readLiteralHeaderWithoutIndexingNewName(); - } else if ((b & 0xe0) == 0x40) { // 01NNNNNN + } else if ((b & 0x40) == 0x40) { // 01NNNNNN int index = readInt(b, PREFIX_6_BITS); readLiteralHeaderWithoutIndexingIndexedName(index - 1); } else if (b == 0) { // 00000000 @@ -375,6 +378,19 @@ final class HpackDraft05 { } } + private static final Map NAME_TO_FIRST_INDEX = nameToFirstIndex(); + + private static Map nameToFirstIndex() { + Map result = + new LinkedHashMap(STATIC_HEADER_TABLE.length); + for (int i = 0; i < STATIC_HEADER_TABLE.length; i++) { + if (!result.containsKey(STATIC_HEADER_TABLE[i].name)) { + result.put(STATIC_HEADER_TABLE[i].name, i); + } + } + return Collections.unmodifiableMap(result); + } + static final class Writer { private final OutputStream out; @@ -383,11 +399,19 @@ final class HpackDraft05 { } void writeHeaders(List
headerBlock) throws IOException { - // TODO: implement a compression strategy. + // TODO: implement index tracking for (int i = 0, size = headerBlock.size(); i < size; i++) { - out.write(0x40); // Literal Header without Indexing - New Name. - writeByteString(headerBlock.get(i).name); - writeByteString(headerBlock.get(i).value); + ByteString name = headerBlock.get(i).name; + Integer staticIndex = NAME_TO_FIRST_INDEX.get(name); + if (staticIndex != null) { + // Literal Header Field without Indexing - Indexed Name. + writeInt(staticIndex + 1, PREFIX_6_BITS, 0x40); + writeByteString(headerBlock.get(i).value); + } else { + out.write(0x40); // Literal Header without Indexing - New Name. + writeByteString(name); + writeByteString(headerBlock.get(i).value); + } } } diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/HpackDraft05Test.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/HpackDraft05Test.java index 9e3bb1ed8..5f011835f 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/HpackDraft05Test.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/HpackDraft05Test.java @@ -27,6 +27,7 @@ import org.junit.Before; import org.junit.Test; import static com.squareup.okhttp.internal.Util.headerEntries; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; @@ -36,9 +37,12 @@ public class HpackDraft05Test { private final MutableByteArrayInputStream bytesIn = new MutableByteArrayInputStream(); private HpackDraft05.Reader hpackReader; + private ByteArrayOutputStream bytesOut = new ByteArrayOutputStream(); + private HpackDraft05.Writer hpackWriter; - @Before public void resetReader() { + @Before public void reset() { hpackReader = newReader(bytesIn); + hpackWriter = new HpackDraft05.Writer(new DataOutputStream(bytesOut)); } /** @@ -167,7 +171,7 @@ public class HpackDraft05Test { /** * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05#appendix-E.1.1 */ - @Test public void decodeLiteralHeaderFieldWithIndexing() throws IOException { + @Test public void readLiteralHeaderFieldWithIndexing() throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); out.write(0x00); // Literal indexed @@ -192,29 +196,60 @@ public class HpackDraft05Test { } /** - * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05#appendix-E.1.2 + * Literal Header Field without Indexing - New Name */ - @Test public void decodeLiteralHeaderFieldWithoutIndexingIndexedName() throws IOException { - ByteArrayOutputStream out = new ByteArrayOutputStream(); + @Test public void literalHeaderFieldWithoutIndexingNewName() throws IOException { + List
headerBlock = headerEntries("custom-key", "custom-header"); - out.write(0x44); // == Literal not indexed == - // Indexed name (idx = 4) -> :path - out.write(0x0c); // Literal value (len = 12) - out.write("/sample/path".getBytes(), 0, 12); + ByteArrayOutputStream expectedBytes = new ByteArrayOutputStream(); - bytesIn.set(out.toByteArray()); + expectedBytes.write(0x40); // Not indexed + expectedBytes.write(0x0a); // Literal name (len = 10) + expectedBytes.write("custom-key".getBytes(), 0, 10); + + expectedBytes.write(0x0d); // Literal value (len = 13) + expectedBytes.write("custom-header".getBytes(), 0, 13); + + hpackWriter.writeHeaders(headerBlock); + assertArrayEquals(expectedBytes.toByteArray(), bytesOut.toByteArray()); + + bytesIn.set(bytesOut.toByteArray()); hpackReader.readHeaders(); hpackReader.emitReferenceSet(); assertEquals(0, hpackReader.headerCount); - assertEquals(headerEntries(":path", "/sample/path"), hpackReader.getAndReset()); + assertEquals(headerBlock, hpackReader.getAndReset()); + } + + /** + * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05#appendix-E.1.2 + */ + @Test public void literalHeaderFieldWithoutIndexingIndexedName() throws IOException { + List
headerBlock = headerEntries(":path", "/sample/path"); + + ByteArrayOutputStream expectedBytes = new ByteArrayOutputStream(); + expectedBytes.write(0x44); // == Literal not indexed == + // Indexed name (idx = 4) -> :path + expectedBytes.write(0x0c); // Literal value (len = 12) + expectedBytes.write("/sample/path".getBytes(), 0, 12); + + hpackWriter.writeHeaders(headerBlock); + assertArrayEquals(expectedBytes.toByteArray(), bytesOut.toByteArray()); + + bytesIn.set(bytesOut.toByteArray()); + hpackReader.readHeaders(); + hpackReader.emitReferenceSet(); + + assertEquals(0, hpackReader.headerCount); + + assertEquals(headerBlock, hpackReader.getAndReset()); } /** * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05#appendix-E.1.3 */ - @Test public void decodeIndexedHeaderField() throws IOException { + @Test public void readIndexedHeaderField() throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); out.write(0x82); // == Indexed - Add == @@ -264,7 +299,7 @@ public class HpackDraft05Test { /** * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05#appendix-E.1.4 */ - @Test public void decodeIndexedHeaderFieldFromStaticTableWithoutBuffering() throws IOException { + @Test public void readIndexedHeaderFieldFromStaticTableWithoutBuffering() throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); out.write(0x82); // == Indexed - Add == @@ -284,24 +319,24 @@ public class HpackDraft05Test { /** * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05#appendix-E.2 */ - @Test public void decodeRequestExamplesWithoutHuffman() throws IOException { + @Test public void readRequestExamplesWithoutHuffman() throws IOException { ByteArrayOutputStream out = firstRequestWithoutHuffman(); bytesIn.set(out.toByteArray()); hpackReader.readHeaders(); hpackReader.emitReferenceSet(); - checkFirstRequestWithoutHuffman(); + checkReadFirstRequestWithoutHuffman(); out = secondRequestWithoutHuffman(); bytesIn.set(out.toByteArray()); hpackReader.readHeaders(); hpackReader.emitReferenceSet(); - checkSecondRequestWithoutHuffman(); + checkReadSecondRequestWithoutHuffman(); out = thirdRequestWithoutHuffman(); bytesIn.set(out.toByteArray()); hpackReader.readHeaders(); hpackReader.emitReferenceSet(); - checkThirdRequestWithoutHuffman(); + checkReadThirdRequestWithoutHuffman(); } private ByteArrayOutputStream firstRequestWithoutHuffman() { @@ -321,7 +356,7 @@ public class HpackDraft05Test { return out; } - private void checkFirstRequestWithoutHuffman() { + private void checkReadFirstRequestWithoutHuffman() { assertEquals(4, hpackReader.headerCount); // [ 1] (s = 57) :authority: www.example.com @@ -366,7 +401,7 @@ public class HpackDraft05Test { return out; } - private void checkSecondRequestWithoutHuffman() { + private void checkReadSecondRequestWithoutHuffman() { assertEquals(5, hpackReader.headerCount); // [ 1] (s = 53) cache-control: no-cache @@ -427,7 +462,7 @@ public class HpackDraft05Test { return out; } - private void checkThirdRequestWithoutHuffman() { + private void checkReadThirdRequestWithoutHuffman() { assertEquals(8, hpackReader.headerCount); // [ 1] (s = 54) custom-key: custom-value @@ -486,24 +521,24 @@ public class HpackDraft05Test { /** * http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-05#appendix-E.3 */ - @Test public void decodeRequestExamplesWithHuffman() throws IOException { + @Test public void readRequestExamplesWithHuffman() throws IOException { ByteArrayOutputStream out = firstRequestWithHuffman(); bytesIn.set(out.toByteArray()); hpackReader.readHeaders(); hpackReader.emitReferenceSet(); - checkFirstRequestWithHuffman(); + checkReadFirstRequestWithHuffman(); out = secondRequestWithHuffman(); bytesIn.set(out.toByteArray()); hpackReader.readHeaders(); hpackReader.emitReferenceSet(); - checkSecondRequestWithHuffman(); + checkReadSecondRequestWithHuffman(); out = thirdRequestWithHuffman(); bytesIn.set(out.toByteArray()); hpackReader.readHeaders(); hpackReader.emitReferenceSet(); - checkThirdRequestWithHuffman(); + checkReadThirdRequestWithHuffman(); } private ByteArrayOutputStream firstRequestWithHuffman() { @@ -528,7 +563,7 @@ public class HpackDraft05Test { return out; } - private void checkFirstRequestWithHuffman() { + private void checkReadFirstRequestWithHuffman() { assertEquals(4, hpackReader.headerCount); // [ 1] (s = 57) :authority: www.example.com @@ -577,7 +612,7 @@ public class HpackDraft05Test { return out; } - private void checkSecondRequestWithHuffman() { + private void checkReadSecondRequestWithHuffman() { assertEquals(5, hpackReader.headerCount); // [ 1] (s = 53) cache-control: no-cache @@ -647,7 +682,7 @@ public class HpackDraft05Test { return out; } - private void checkThirdRequestWithHuffman() { + private void checkReadThirdRequestWithHuffman() { assertEquals(8, hpackReader.headerCount); // [ 1] (s = 54) custom-key: custom-value @@ -703,10 +738,6 @@ public class HpackDraft05Test { "custom-key", "custom-value"), hpackReader.getAndReset()); } - private ByteArrayOutputStream bytesOut = new ByteArrayOutputStream(); - private final HpackDraft05.Writer hpackWriter = - new HpackDraft05.Writer(new DataOutputStream(bytesOut)); - @Test public void readSingleByteInt() throws IOException { assertEquals(10, newReader(byteStream()).readInt(10, 31)); assertEquals(10, newReader(byteStream()).readInt(0xe0 | 10, 31)); @@ -768,17 +799,6 @@ public class HpackDraft05Test { assertSame(ByteString.EMPTY, newReader(byteStream(0)).readByteString(false)); } - @Test public void headersRoundTrip() throws IOException { - List
sentHeaders = headerEntries("name", "value"); - hpackWriter.writeHeaders(sentHeaders); - ByteArrayInputStream bytesIn = new ByteArrayInputStream(bytesOut.toByteArray()); - HpackDraft05.Reader reader = newReader(bytesIn); - reader.readHeaders(); - reader.emitReferenceSet(); - List
receivedHeaders = reader.getAndReset(); - assertEquals(sentHeaders, receivedHeaders); - } - private HpackDraft05.Reader newReader(InputStream input) { return new HpackDraft05.Reader(false, 4096, input); } diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Http20Draft09Test.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Http20Draft09Test.java index 8a8aaedfc..4f94d27c4 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Http20Draft09Test.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Http20Draft09Test.java @@ -219,21 +219,22 @@ public class Http20Draft09Test { // Decoding the first header will cross frame boundaries. byte[] headerBlock = literalHeaders(pushPromise); + int firstFrameLength = headerBlock.length - 1; { // Write the first headers frame. - dataOut.writeShort((headerBlock.length / 2) + 4); + dataOut.writeShort(firstFrameLength + 4); dataOut.write(Http20Draft09.TYPE_PUSH_PROMISE); dataOut.write(0); // no flags dataOut.writeInt(expectedStreamId & 0x7fffffff); dataOut.writeInt(expectedPromisedStreamId & 0x7fffffff); - dataOut.write(headerBlock, 0, headerBlock.length / 2); + dataOut.write(headerBlock, 0, firstFrameLength); } { // Write the continuation frame, specifying no more frames are expected. - dataOut.writeShort(headerBlock.length / 2); + dataOut.writeShort(1); dataOut.write(Http20Draft09.TYPE_CONTINUATION); dataOut.write(Http20Draft09.FLAG_END_HEADERS); dataOut.writeInt(expectedStreamId & 0x7fffffff); - dataOut.write(headerBlock, headerBlock.length / 2, headerBlock.length / 2); + dataOut.write(headerBlock, firstFrameLength, 1); } FrameReader fr = newReader(out);