Skip to content

Commit

Permalink
Add support for dynamic table size changes to HPACK Encoder.
Browse files Browse the repository at this point in the history
  • Loading branch information
dave-r12 committed Jul 7, 2016
1 parent f491389 commit 1267cb9
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 20 deletions.
92 changes: 92 additions & 0 deletions okhttp-tests/src/test/java/okhttp3/internal/framed/HpackTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,98 @@ private void checkReadThirdRequestWithHuffman() {
assertEquals(ByteString.EMPTY, newReader(byteStream(0)).readByteString());
}

@Test public void emitsDynamicTableSizeUpdate() throws IOException {
hpackWriter.setHeaderTableSizeSetting(2048);
hpackWriter.writeHeaders(Arrays.asList(new Header("foo", "bar")));
assertBytes(
0x3F, 0xE1, 0xF, // Dynamic table size update (size = 2048).
0x40, 3, 'f', 'o', 'o', 3, 'b', 'a', 'r');

hpackWriter.setHeaderTableSizeSetting(8192);
hpackWriter.writeHeaders(Arrays.asList(new Header("bar", "foo")));
assertBytes(
0x3F, 0xE1, 0x3F, // Dynamic table size update (size = 8192).
0x40, 3, 'b', 'a', 'r', 3, 'f', 'o', 'o');

// No more dynamic table updates should be emitted.
hpackWriter.writeHeaders(Arrays.asList(new Header("far", "boo")));
assertBytes(0x40, 3, 'f', 'a', 'r', 3, 'b', 'o', 'o');
}

@Test public void noDynamicTableSizeUpdateWhenSizeIsEqual() throws IOException {
int currentSize = hpackWriter.headerTableSizeSetting;
hpackWriter.setHeaderTableSizeSetting(currentSize);
hpackWriter.writeHeaders(Arrays.asList(new Header("foo", "bar")));

assertBytes(0x40, 3, 'f', 'o', 'o', 3, 'b', 'a', 'r');
}

@Test public void growDynamicTableSize() throws IOException {
hpackWriter.setHeaderTableSizeSetting(8192);
hpackWriter.setHeaderTableSizeSetting(16384);
hpackWriter.writeHeaders(Arrays.asList(new Header("foo", "bar")));

assertBytes(
0x3F, 0xE1, 0x7F, // Dynamic table size update (size = 16384).
0x40, 3, 'f', 'o', 'o', 3, 'b', 'a', 'r');
}

@Test public void shrinkDynamicTableSize() throws IOException {
hpackWriter.setHeaderTableSizeSetting(2048);
hpackWriter.setHeaderTableSizeSetting(0);
hpackWriter.writeHeaders(Arrays.asList(new Header("foo", "bar")));

assertBytes(
0x20, // Dynamic size update (size = 0).
0x40, 3, 'f', 'o', 'o', 3, 'b', 'a', 'r');
}

@Test public void manyDynamicTableSizeChanges() throws IOException {
hpackWriter.setHeaderTableSizeSetting(16384);
hpackWriter.setHeaderTableSizeSetting(8096);
hpackWriter.setHeaderTableSizeSetting(0);
hpackWriter.setHeaderTableSizeSetting(4096);
hpackWriter.setHeaderTableSizeSetting(2048);
hpackWriter.writeHeaders(Arrays.asList(new Header("foo", "bar")));

assertBytes(
0x20, // Dynamic size update (size = 0).
0x3F, 0xE1, 0xF, // Dynamic size update (size = 2048).
0x40, 3, 'f', 'o', 'o', 3, 'b', 'a', 'r');
}

@Test public void dynamicTableEvictionWhenSizeLowered() throws IOException {
List<Header> headerBlock =
headerEntries(
"custom-key1", "custom-header",
"custom-key2", "custom-header");
hpackWriter.writeHeaders(headerBlock);
assertEquals(2, hpackWriter.headerCount);

hpackWriter.setHeaderTableSizeSetting(56);
assertEquals(1, hpackWriter.headerCount);

hpackWriter.setHeaderTableSizeSetting(0);
assertEquals(0, hpackWriter.headerCount);
}

@Test public void noEvictionOnDynamicTableSizeIncrease() throws IOException {
List<Header> headerBlock =
headerEntries(
"custom-key1", "custom-header",
"custom-key2", "custom-header");
hpackWriter.writeHeaders(headerBlock);
assertEquals(2, hpackWriter.headerCount);

hpackWriter.setHeaderTableSizeSetting(8192);
assertEquals(2, hpackWriter.headerCount);
}

@Test public void dynamicTableSizeHasAnUpperBound() {
hpackWriter.setHeaderTableSizeSetting(1048576);
assertEquals(16384, hpackWriter.maxDynamicTableByteCount);
}

private Hpack.Reader newReader(Buffer source) {
return new Hpack.Reader(4096, source);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,20 +138,18 @@ public final class Http2ConnectionTest {
assertEquals(3368, stream.bytesLeftInWriteWindow);
}

@Test
@Ignore("Working through making this work with the new HPACK encoder.")
public void peerHttp2ServerZerosCompressionTable() throws Exception {
@Test public void peerHttp2ServerZerosCompressionTable() throws Exception {
boolean client = false; // Peer is server, so we are client.
Settings settings = new Settings();
settings.set(HEADER_TABLE_SIZE, PERSIST_VALUE, 0);

FramedConnection connection = sendHttp2SettingsAndCheckForAck(client, settings);

// verify the peer's settings were read and applied.
// Verify the peer's settings were read and applied.
assertEquals(0, connection.peerSettings.getHeaderTableSize());
Http2.Reader frameReader = (Http2.Reader) connection.readerRunnable.frameReader;
assertEquals(0, frameReader.hpackReader.maxDynamicTableByteCount());
// TODO: when supported, check the frameWriter's compression table is unaffected.
Http2.Writer frameWriter = (Http2.Writer) connection.frameWriter;
assertEquals(0, frameWriter.hpackWriter.dynamicTableByteCount);
assertEquals(0, frameWriter.hpackWriter.headerTableSizeSetting);
}

@Test public void peerHttp2ClientDisablesPush() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ public void pushPromise(int streamId, int promisedStreamId, List<Header> headerB

Http2.Writer writer = new Http2.Writer(new Buffer(), true);

writer.ackSettings(new Settings().set(Settings.MAX_FRAME_SIZE, 0, newMaxFrameSize));
writer.applyAndAckSettings(new Settings().set(Settings.MAX_FRAME_SIZE, 0, newMaxFrameSize));

assertEquals(newMaxFrameSize, writer.maxDataLength());
writer.frameHeader(0, newMaxFrameSize, Http2.TYPE_DATA, FLAG_NONE);
Expand Down
4 changes: 2 additions & 2 deletions okhttp/src/main/java/okhttp3/internal/framed/FrameWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ public interface FrameWriter extends Closeable {
/** HTTP/2 only. */
void connectionPreface() throws IOException;

/** Informs the peer that we've applied its latest settings. */
void ackSettings(Settings peerSettings) throws IOException;
/** Applies {@code peerSettings} and then sends a settings ACK. */
void applyAndAckSettings(Settings peerSettings) throws IOException;

/**
* HTTP/2 only. Send a push promise header block.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ private Reader(FrameReader frameReader) {
if (clearPrevious) peerSettings.clear();
peerSettings.merge(newSettings);
if (getProtocol() == Protocol.HTTP_2) {
ackSettingsLater(newSettings);
applyAndAckSettings(newSettings);
}
int peerInitialWindowSize = peerSettings.getInitialWindowSize(DEFAULT_INITIAL_WINDOW_SIZE);
if (peerInitialWindowSize != -1 && peerInitialWindowSize != priorWriteWindowSize) {
Expand Down Expand Up @@ -728,11 +728,11 @@ private Reader(FrameReader frameReader) {
}
}

private void ackSettingsLater(final Settings peerSettings) {
private void applyAndAckSettings(final Settings peerSettings) {
executor.execute(new NamedRunnable("OkHttp %s ACK Settings", hostname) {
@Override public void execute() {
try {
frameWriter.ackSettings(peerSettings);
frameWriter.applyAndAckSettings(peerSettings);
} catch (IOException ignored) {
}
}
Expand Down
56 changes: 52 additions & 4 deletions okhttp/src/main/java/okhttp3/internal/framed/Hpack.java
Original file line number Diff line number Diff line change
Expand Up @@ -362,11 +362,26 @@ static final class Writer {
private static final byte SEPARATED_TOKEN = ':';
private static final int SETTINGS_HEADER_TABLE_SIZE = 4096;

/**
* The decoder has ultimate control of the maximum size of the dynamic table but we can choose
* to use less. We'll put a cap at 16K. This is arbitrary but should be enough for most
* purposes.
*/
private static final int SETTINGS_HEADER_TABLE_SIZE_LIMIT = 16384;

private final Buffer out;
private final Map<ByteString, Integer> headerStringToDynamicIndex = new LinkedHashMap<>();

private int headerTableSizeSetting;
private int maxDynamicTableByteCount;
/**
* In the scenario where the dynamic table size changes multiple times between transmission of
* header blocks, we need to keep track of the smallest value in that interval.
*/
private int smallestHeaderTableSizeSetting = Integer.MAX_VALUE;
private boolean emitDynamicTableSizeUpdate;

int headerTableSizeSetting;
int maxDynamicTableByteCount;

// Visible for testing.
Header[] dynamicTable = new Header[8];
// Array is populated back to front, so new entries always have lowest index.
Expand All @@ -378,8 +393,6 @@ static final class Writer {
this(SETTINGS_HEADER_TABLE_SIZE, out);
}

// This is the only method to set the dynamic table size.
// Don't support to change the dynamic table size after Writer constructed.
Writer(int headerTableSizeSetting, Buffer out) {
this.headerTableSizeSetting = headerTableSizeSetting;
this.maxDynamicTableByteCount = headerTableSizeSetting;
Expand Down Expand Up @@ -456,6 +469,15 @@ private void insertIntoDynamicTable(Header entry) {
/** This does not use "never indexed" semantics for sensitive headers. */
// http://tools.ietf.org/html/draft-ietf-httpbis-header-compression-12#section-6.2.3
void writeHeaders(List<Header> headerBlock) throws IOException {
if (emitDynamicTableSizeUpdate) {
if (smallestHeaderTableSizeSetting < maxDynamicTableByteCount) {
// Multiple dynamic table size updates!
writeInt(smallestHeaderTableSizeSetting, PREFIX_5_BITS, 0x20);
}
emitDynamicTableSizeUpdate = false;
smallestHeaderTableSizeSetting = Integer.MAX_VALUE;
writeInt(maxDynamicTableByteCount, PREFIX_5_BITS, 0x20);
}
// TODO: implement index tracking
for (int i = 0, size = headerBlock.size(); i < size; i++) {
Header header = headerBlock.get(i);
Expand Down Expand Up @@ -509,6 +531,32 @@ void writeByteString(ByteString data) throws IOException {
writeInt(data.size(), PREFIX_7_BITS, 0);
out.write(data);
}

void setHeaderTableSizeSetting(int headerTableSizeSetting) {
this.headerTableSizeSetting = headerTableSizeSetting;
int effectiveHeaderTableSize = Math.min(headerTableSizeSetting,
SETTINGS_HEADER_TABLE_SIZE_LIMIT);

if (maxDynamicTableByteCount == effectiveHeaderTableSize) return; // No change.

if (effectiveHeaderTableSize < maxDynamicTableByteCount) {
smallestHeaderTableSizeSetting = Math.min(smallestHeaderTableSizeSetting,
effectiveHeaderTableSize);
}
emitDynamicTableSizeUpdate = true;
maxDynamicTableByteCount = effectiveHeaderTableSize;
adjustDynamicTableByteCount();
}

private void adjustDynamicTableByteCount() {
if (maxDynamicTableByteCount < dynamicTableByteCount) {
if (maxDynamicTableByteCount == 0) {
clearDynamicTable();
} else {
evictToRecoverBytes(dynamicTableByteCount - maxDynamicTableByteCount);
}
}
}
}

/**
Expand Down
9 changes: 7 additions & 2 deletions okhttp/src/main/java/okhttp3/internal/framed/Http2.java
Original file line number Diff line number Diff line change
Expand Up @@ -359,10 +359,11 @@ static final class Writer implements FrameWriter {
private final BufferedSink sink;
private final boolean client;
private final Buffer hpackBuffer;
private final Hpack.Writer hpackWriter;
private int maxFrameSize;
private boolean closed;

final Hpack.Writer hpackWriter;

Writer(BufferedSink sink, boolean client) {
this.sink = sink;
this.client = client;
Expand All @@ -376,9 +377,13 @@ static final class Writer implements FrameWriter {
sink.flush();
}

@Override public synchronized void ackSettings(Settings peerSettings) throws IOException {
@Override public synchronized void applyAndAckSettings(Settings peerSettings)
throws IOException {
if (closed) throw new IOException("closed");
this.maxFrameSize = peerSettings.getMaxFrameSize(maxFrameSize);
if (peerSettings.getHeaderTableSize() > -1) {
hpackWriter.setHeaderTableSizeSetting(peerSettings.getHeaderTableSize());
}
int length = 0;
byte type = TYPE_SETTINGS;
byte flags = FLAG_ACK;
Expand Down
2 changes: 1 addition & 1 deletion okhttp/src/main/java/okhttp3/internal/framed/Spdy3.java
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ static final class Writer implements FrameWriter {
headerBlockOut = Okio.buffer(new DeflaterSink(headerBlockBuffer, deflater));
}

@Override public void ackSettings(Settings peerSettings) {
@Override public void applyAndAckSettings(Settings peerSettings) {
// Do nothing: no ACK for SPDY/3 settings.
}

Expand Down

0 comments on commit 1267cb9

Please sign in to comment.