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 75c77240a..feeb8ef7f 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 @@ -133,7 +133,7 @@ final class HpackDraft05 { /** * Called by the reader when the peer sent a new header table size setting. - * + *

* Evicts entries or clears the table as needed. */ void maxHeaderTableByteCount(int newMaxHeaderTableByteCount) { diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Http20Draft09.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Http20Draft09.java index 787565604..cb108ad2b 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Http20Draft09.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Http20Draft09.java @@ -39,20 +39,18 @@ public final class Http20Draft09 implements Variant { // http://tools.ietf.org/html/draft-ietf-httpbis-http2-09#section-6.5 @Override public Settings defaultOkHttpSettings(boolean client) { - Settings settings = new Settings(); - settings.set(Settings.HEADER_TABLE_SIZE, 0, 4096); - if (!client) { // client doesn't send push requests. - settings.set(Settings.ENABLE_PUSH, 0, 0); // TODO: support writing push. + Settings settings = initialPeerSettings(client); + if (client) { // TODO: we don't yet support reading push. + settings.set(Settings.ENABLE_PUSH, 0, 0); } - settings.set(Settings.INITIAL_WINDOW_SIZE, 0, 65535); return settings; } @Override public Settings initialPeerSettings(boolean client) { Settings settings = new Settings(); settings.set(Settings.HEADER_TABLE_SIZE, 0, 4096); - if (client) { // server doesn't read push requests. - settings.set(Settings.ENABLE_PUSH, 0, 0); // TODO: support reading push. + if (client) { // client specifies whether or not it accepts push. + settings.set(Settings.ENABLE_PUSH, 0, 1); } settings.set(Settings.INITIAL_WINDOW_SIZE, 0, 65535); return settings; diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyConnection.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyConnection.java index 5ed07bbf4..5cb64355b 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyConnection.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyConnection.java @@ -82,7 +82,10 @@ public final class SpdyConnection implements Closeable { private Map pings; private int nextPingId; + // TODO: Do we want to dynamically adjust settings, or KISS and only set once? + // Settings we might send include toggling push, adjusting compression table size. final Settings okHttpSettings; + // TODO: MWS will need to guard on this setting before attempting to push. final Settings peerSettings; final FrameReader frameReader; final FrameWriter frameWriter; diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Variant.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Variant.java index 116f9ea1e..f7367302c 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Variant.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Variant.java @@ -28,7 +28,7 @@ interface Variant { Protocol getProtocol(); /** - * Default settings used for sending frames to the peer. + * Default settings used for reading or writing frames to the peer. * @param client true if these settings apply to writing requests, false if responses. */ Settings defaultOkHttpSettings(boolean client); @@ -47,9 +47,8 @@ interface Variant { FrameReader newReader(InputStream in, Settings peerSettings, boolean client); /** - * @param okHttpSettings settings configured locally. + * @param okHttpSettings settings sent to the peer, such compression header table size. * @param client true if this is the HTTP client's writer, writing frames to a server. */ FrameWriter newWriter(OutputStream out, Settings okHttpSettings, boolean client); - } 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 3dc4c085d..1bc912b2a 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 @@ -138,12 +138,14 @@ public class Http20Draft09Test { final int reducedTableSizeBytes = 16; - dataOut.writeShort(8); // 1 setting = 4 bytes for the code and 4 for the value. + dataOut.writeShort(16); // 2 settings * 4 bytes for the code and 4 for the value. dataOut.write(Http20Draft09.TYPE_SETTINGS); dataOut.write(0); // No flags dataOut.writeInt(0 & 0x7fffffff); // Settings are always on the connection stream 0. dataOut.writeInt(Settings.HEADER_TABLE_SIZE & 0xffffff); dataOut.writeInt(reducedTableSizeBytes); + dataOut.writeInt(Settings.ENABLE_PUSH & 0xffffff); + dataOut.writeInt(0); final Http20Draft09.Reader fr = newReader(out); @@ -152,6 +154,7 @@ public class Http20Draft09Test { @Override public void settings(boolean clearPrevious, Settings settings) { assertFalse(clearPrevious); // No clearPrevious in http/2. assertEquals(reducedTableSizeBytes, settings.getHeaderTableSize()); + assertEquals(false, settings.getEnablePush(true)); } }); } diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java index 928ff23c2..97ffe55c0 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java @@ -254,35 +254,33 @@ public final class SpdyConnectionTest { assertEquals(4, ping4.streamId); } - @Test public void http2SettingsAck() throws Exception { - MockSpdyPeer peer = new MockSpdyPeer(Variant.HTTP_20_DRAFT_09, false); - // write the mocking script - Settings settings = new Settings(); - settings.set(Settings.HEADER_TABLE_SIZE, PERSIST_VALUE, 1024); - peer.sendFrame().settings(settings); - peer.acceptFrame(); // ACK - peer.play(); + @Test public void peerHttp2ServerZerosCompressionTable() throws Exception { + boolean client = false; // Peer is server, so we are client. + Settings settings = Variant.HTTP_20_DRAFT_09.initialPeerSettings(client); + settings.set(Settings.HEADER_TABLE_SIZE, PERSIST_VALUE, 0); - // play it back - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()) - .protocol(Protocol.HTTP_2) - .handler(REJECT_INCOMING_STREAMS) - .build(); - - // verify the peer received the ACK - MockSpdyPeer.InFrame pingFrame = peer.takeFrame(); - assertEquals(TYPE_SETTINGS, pingFrame.type); - assertEquals(0, pingFrame.streamId); - // TODO: check for ACK flag. - assertEquals(0, pingFrame.settings.size()); + SpdyConnection connection = sendHttp2SettingsAndCheckForAck(client, settings); // verify the peer's settings were read and applied. synchronized (connection) { - assertEquals(1024, connection.peerSettings.getHeaderTableSize()); + assertEquals(0, connection.peerSettings.getHeaderTableSize()); Http20Draft09.Reader frameReader = (Http20Draft09.Reader) connection.frameReader; - assertEquals(1024, frameReader.hpackReader.maxHeaderTableByteCount()); + assertEquals(0, frameReader.hpackReader.maxHeaderTableByteCount()); + // TODO: when supported, check the frameWriter's compression table is unaffected. + } + } + + @Test public void peerHttp2ClientDisablesPush() throws Exception { + boolean client = false; // Peer is client, so we are server. + Settings settings = Variant.HTTP_20_DRAFT_09.initialPeerSettings(client); + settings.set(Settings.ENABLE_PUSH, 0, 0); // The peer client disables push. + + SpdyConnection connection = sendHttp2SettingsAndCheckForAck(client, settings); + + // verify the peer's settings were read and applied. + synchronized (connection) { + assertFalse(connection.peerSettings.getEnablePush(true)); } - peer.close(); } @Test public void serverSendsSettingsToClient() throws Exception { @@ -1077,6 +1075,29 @@ public final class SpdyConnectionTest { assertStreamData("robot", stream.getInputStream()); } + private SpdyConnection sendHttp2SettingsAndCheckForAck(boolean client, Settings settings) + throws IOException, InterruptedException { + MockSpdyPeer peer = new MockSpdyPeer(Variant.HTTP_20_DRAFT_09, client); + peer.sendFrame().settings(settings); + peer.acceptFrame(); // ACK + peer.play(); + + // play it back + SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()) + .protocol(Protocol.HTTP_2) + .handler(REJECT_INCOMING_STREAMS) + .build(); + + // verify the peer received the ACK + MockSpdyPeer.InFrame pingFrame = peer.takeFrame(); + assertEquals(TYPE_SETTINGS, pingFrame.type); + assertEquals(0, pingFrame.streamId); + // TODO: check for ACK flag. + assertEquals(0, pingFrame.settings.size()); + peer.close(); + return connection; + } + private void writeAndClose(SpdyStream stream, String data) throws IOException { OutputStream out = stream.getOutputStream(); out.write(data.getBytes("UTF-8"));