From 471ed070e9e0b0860b0146f26458c5c0bd8169e0 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 21 Jan 2014 20:53:56 +0100 Subject: [PATCH 1/2] Remove FLAG_END_FLOW_CONTROL as it is no longer in http/2. Backfill test and implementation window update on http/2. --- .../okhttp/internal/spdy/FrameReader.java | 7 +- .../okhttp/internal/spdy/FrameWriter.java | 6 +- .../okhttp/internal/spdy/Http20Draft09.java | 27 ++++-- .../squareup/okhttp/internal/spdy/Spdy3.java | 18 ++-- .../okhttp/internal/spdy/SpdyConnection.java | 14 ++-- .../okhttp/internal/spdy/SpdyStream.java | 4 +- .../okhttp/internal/spdy/BaseTestHandler.java | 2 +- .../internal/spdy/Http20Draft09Test.java | 50 ++++++++++- .../okhttp/internal/spdy/MockSpdyPeer.java | 6 +- .../okhttp/internal/spdy/Spdy3Test.java | 24 +++++- .../internal/spdy/SpdyConnectionTest.java | 83 +++++++++---------- 11 files changed, 167 insertions(+), 74 deletions(-) diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameReader.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameReader.java index 20b03171e..dc03bf299 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameReader.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameReader.java @@ -62,7 +62,12 @@ public interface FrameReader extends Closeable { */ void ping(boolean ack, int payload1, int payload2); void goAway(int lastGoodStreamId, ErrorCode errorCode); - void windowUpdate(int streamId, int deltaWindowSize, boolean endFlowControl); + + /** + * Notifies that an additional {@code windowSizeIncrement} bytes can be + * sent on {@code streamId} or the connection, if {@code streamId} is zero. + */ + void windowUpdate(int streamId, int windowSizeIncrement); void priority(int streamId, int priority); /** diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameWriter.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameWriter.java index ab90db69d..01aff525a 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameWriter.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameWriter.java @@ -82,5 +82,9 @@ public interface FrameWriter extends Closeable { */ void ping(boolean ack, int payload1, int payload2) throws IOException; void goAway(int lastGoodStreamId, ErrorCode errorCode) throws IOException; - void windowUpdate(int streamId, int deltaWindowSize) throws IOException; + /** + * Inform peer that an additional {@code windowSizeIncrement} bytes can be + * sent on {@code streamId} or the connection, if {@code streamId} is zero. + */ + void windowUpdate(int streamId, int windowSizeIncrement) throws IOException; } 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 4589788c1..cae2fd3af 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 @@ -73,7 +73,6 @@ public final class Http20Draft09 implements Variant { static final int FLAG_END_PUSH_PROMISE = 0x4; static final int FLAG_PRIORITY = 0x8; static final int FLAG_ACK = 0x1; - static final int FLAG_END_FLOW_CONTROL = 0x1; @Override public FrameReader newReader(InputStream in, Settings peerSettings, boolean client) { return new Reader(in, peerSettings.getHeaderTableSize(), client); @@ -119,6 +118,9 @@ public final class Http20Draft09 implements Variant { // boolean r = (w1 & 0xc0000000) != 0; // Reserved. int length = (w1 & 0x3fff0000) >> 16; // 14-bit unsigned. + if ((length & 0xffffffffL) > 16383) { + throw new IOException("FRAME_SIZE_ERROR max size is 16383: " + length); + } int type = (w1 & 0xff00) >> 8; int flags = w1 & 0xff; // boolean r = (w2 & 0x80000000) != 0; // Reserved. @@ -282,11 +284,10 @@ public final class Http20Draft09 implements Variant { private void readWindowUpdate(Handler handler, int flags, int length, int streamId) throws IOException { - int w1 = in.readInt(); - // boolean r = (w1 & 0x80000000) != 0; // Reserved. - int windowSizeIncrement = (w1 & 0x7fffffff); - boolean endFlowControl = (flags & FLAG_END_FLOW_CONTROL) != 0; - handler.windowUpdate(streamId, windowSizeIncrement, endFlowControl); + if (length != 4) throw ioException("TYPE_WINDOW_UPDATE length !=4: %s", length); + int increment = (in.readInt() & 0x7fffffff); + if (increment == 0) throw ioException("windowSizeIncrement was 0", increment); + handler.windowUpdate(streamId, increment); } @Override public void close() throws IOException { @@ -436,9 +437,15 @@ public final class Http20Draft09 implements Variant { // TODO } - @Override public synchronized void windowUpdate(int streamId, int deltaWindowSize) + @Override public synchronized void windowUpdate(int streamId, int increment) throws IOException { - // TODO + if (increment == 0 || (increment & 0xffffffffL) > 0x7fffffffL) { + throw new IllegalArgumentException( + "windowSizeIncrement must be between 1 and 0x7fffffff: " + (increment & 0xffffffffL)); + } + out.writeInt(4 << 16 | (TYPE_WINDOW_UPDATE & 0xff) << 8); // No flags. + out.writeInt(streamId); + out.writeInt(increment); } @Override public void close() throws IOException { @@ -447,7 +454,9 @@ public final class Http20Draft09 implements Variant { } private static void checkFrameSize(int bytes) throws IOException { - if (bytes > 16383) throw ioException("FRAME_SIZE_ERROR max size is 16383: %s", bytes); + if ((bytes & 0xffffffffL) > 16383) { + throw new IllegalArgumentException("FRAME_SIZE_ERROR max size is 16383: " + bytes); + } } private static IOException ioException(String message, Object... args) throws IOException { diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java index 4d0bf8455..f837d76e2 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java @@ -252,8 +252,9 @@ final class Spdy3 implements Variant { int w1 = in.readInt(); int w2 = in.readInt(); int streamId = w1 & 0x7fffffff; - int deltaWindowSize = w2 & 0x7fffffff; - handler.windowUpdate(streamId, deltaWindowSize, false); + int increment = w2 & 0x7fffffff; + if (increment == 0) throw ioException("windowSizeIncrement was 0", increment); + handler.windowUpdate(streamId, increment); } private void readPing(Handler handler, int flags, int length) throws IOException { @@ -410,8 +411,9 @@ final class Spdy3 implements Variant { void sendDataFrame(int streamId, int flags, byte[] data, int offset, int byteCount) throws IOException { - if (byteCount > 0xffffff) { - throw new IOException("FRAME_TOO_LARGE max size is 16Mib: " + byteCount); + if ((byteCount & 0xffffffffL) > 0xffffffL) { + throw new IllegalArgumentException( + "FRAME_TOO_LARGE max size is 16Mib: " + (byteCount & 0xffffffffL)); } out.writeInt(streamId & 0x7fffffff); out.writeInt((flags & 0xff) << 24 | byteCount & 0xffffff); @@ -484,15 +486,19 @@ final class Spdy3 implements Variant { out.flush(); } - @Override public synchronized void windowUpdate(int streamId, int deltaWindowSize) + @Override public synchronized void windowUpdate(int streamId, int increment) throws IOException { + if (increment == 0 || (increment & 0xffffffffL) > 0x7fffffffL) { + throw new IllegalArgumentException( + "windowSizeIncrement must be between 1 and 0x7fffffff: " + (increment & 0xffffffffL)); + } int type = TYPE_WINDOW_UPDATE; int flags = 0; int length = 8; out.writeInt(0x80000000 | (VERSION & 0x7fff) << 16 | type & 0xffff); out.writeInt((flags & 0xff) << 24 | length & 0xffffff); out.writeInt(streamId); - out.writeInt(deltaWindowSize); + out.writeInt(increment); out.flush(); } 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 504d49fc0..3056eac18 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 @@ -218,19 +218,19 @@ public final class SpdyConnection implements Closeable { frameWriter.rstStream(streamId, statusCode); } - void writeWindowUpdateLater(final int streamId, final int deltaWindowSize) { + void writeWindowUpdateLater(final int streamId, final int windowSizeIncrement) { executor.submit(new NamedRunnable("OkHttp %s stream %d", hostName, streamId) { @Override public void execute() { try { - writeWindowUpdate(streamId, deltaWindowSize); + writeWindowUpdate(streamId, windowSizeIncrement); } catch (IOException ignored) { } } }); } - void writeWindowUpdate(int streamId, int deltaWindowSize) throws IOException { - frameWriter.windowUpdate(streamId, deltaWindowSize); + void writeWindowUpdate(int streamId, int windowSizeIncrement) throws IOException { + frameWriter.windowUpdate(streamId, windowSizeIncrement); } /** @@ -614,16 +614,16 @@ public final class SpdyConnection implements Closeable { } } - @Override public void windowUpdate(int streamId, int deltaWindowSize, boolean endFlowControl) { + @Override public void windowUpdate(int streamId, int windowSizeIncrement) { if (streamId == 0) { - // TODO: honor whole-stream flow control + // TODO: honor connection-level flow control return; } // TODO: honor endFlowControl SpdyStream stream = getStream(streamId); if (stream != null) { - stream.receiveWindowUpdate(deltaWindowSize); + stream.receiveWindowUpdate(windowSizeIncrement); } } diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java index ba14dbe45..4d00e1556 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java @@ -325,8 +325,8 @@ public final class SpdyStream { notifyAll(); } - synchronized void receiveWindowUpdate(int deltaWindowSize) { - out.unacknowledgedBytes -= deltaWindowSize; + synchronized void receiveWindowUpdate(int windowSizeIncrement) { + out.unacknowledgedBytes -= windowSizeIncrement; notifyAll(); } diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/BaseTestHandler.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/BaseTestHandler.java index cbb6eac50..5eb81f406 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/BaseTestHandler.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/BaseTestHandler.java @@ -54,7 +54,7 @@ class BaseTestHandler implements FrameReader.Handler { } @Override - public void windowUpdate(int streamId, int deltaWindowSize, boolean endFlowControl) { + public void windowUpdate(int streamId, int windowSizeIncrement) { fail(); } 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 31d08421a..4926d998d 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 @@ -348,11 +348,53 @@ public class Http20Draft09Test { try { sendDataFrame(new byte[0x1000000]); fail(); - } catch (IOException e) { + } catch (IllegalArgumentException e) { assertEquals("FRAME_SIZE_ERROR max size is 16383: 16777216", e.getMessage()); } } + @Test public void windowUpdateRoundTrip() throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + DataOutputStream dataOut = new DataOutputStream(out); + + final int expectedWindowSizeIncrement = 0x7fffffff; + + // Compose the expected window update frame. + dataOut.writeShort(4); // length + dataOut.write(Http20Draft09.TYPE_WINDOW_UPDATE); + dataOut.write(0); // No flags. + dataOut.writeInt(expectedStreamId); + dataOut.writeInt(expectedWindowSizeIncrement); + + // Check writer sends the same bytes. + assertArrayEquals(out.toByteArray(), windowUpdate(expectedWindowSizeIncrement)); + + FrameReader fr = newReader(out); + + fr.nextFrame(new BaseTestHandler() { // Consume the window update frame. + public void windowUpdate(int streamId, int windowSizeIncrement) { + assertEquals(expectedStreamId, streamId); + assertEquals(expectedWindowSizeIncrement, windowSizeIncrement); + } + }); + } + + @Test public void badWindowSizeIncrement() throws IOException { + try { + windowUpdate(0); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("windowSizeIncrement must be between 1 and 0x7fffffff: 0", e.getMessage()); + } + try { + windowUpdate(0x80000000); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("windowSizeIncrement must be between 1 and 0x7fffffff: 2147483648", + e.getMessage()); + } + } + private Http20Draft09.Reader newReader(ByteArrayOutputStream out) { return new Http20Draft09.Reader(new ByteArrayInputStream(out.toByteArray()), Variant.HTTP_20_DRAFT_09.initialPeerSettings(false).getHeaderTableSize(), false); @@ -379,4 +421,10 @@ public class Http20Draft09Test { new Http20Draft09.Writer(out, true).sendDataFrame(expectedStreamId, 0, data, offset, byteCount); return out.toByteArray(); } + + private byte[] windowUpdate(int windowSizeIncrement) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + new Http20Draft09.Writer(out, true).windowUpdate(expectedStreamId, windowSizeIncrement); + return out.toByteArray(); + } } diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java index 73a37e01d..3e36b1c71 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java @@ -185,7 +185,7 @@ public final class MockSpdyPeer implements Closeable { public int associatedStreamId; public int priority; public ErrorCode errorCode; - public int deltaWindowSize; + public int windowSizeIncrement; public List
headerBlock; public byte[] data; public Settings settings; @@ -257,11 +257,11 @@ public final class MockSpdyPeer implements Closeable { this.errorCode = errorCode; } - @Override public void windowUpdate(int streamId, int deltaWindowSize, boolean endFlowControl) { + @Override public void windowUpdate(int streamId, int windowSizeIncrement) { if (this.type != -1) throw new IllegalStateException(); this.type = Spdy3.TYPE_WINDOW_UPDATE; this.streamId = streamId; - this.deltaWindowSize = deltaWindowSize; + this.windowSizeIncrement = windowSizeIncrement; } @Override public void priority(int streamId, int priority) { diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Spdy3Test.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Spdy3Test.java index 83d3fd2a4..a7520b360 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Spdy3Test.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Spdy3Test.java @@ -29,11 +29,27 @@ public class Spdy3Test { try { sendDataFrame(new byte[0x1000000]); fail(); - } catch (IOException e) { + } catch (IllegalArgumentException e) { assertEquals("FRAME_TOO_LARGE max size is 16Mib: 16777216", e.getMessage()); } } + @Test public void badWindowSizeIncrement() throws IOException { + try { + windowUpdate(0); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("windowSizeIncrement must be between 1 and 0x7fffffff: 0", e.getMessage()); + } + try { + windowUpdate(0x80000000); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("windowSizeIncrement must be between 1 and 0x7fffffff: 2147483648", + e.getMessage()); + } + } + private byte[] sendDataFrame(byte[] data) throws IOException { return sendDataFrame(data, 0, data.length); } @@ -43,4 +59,10 @@ public class Spdy3Test { new Spdy3.Writer(out, true).sendDataFrame(expectedStreamId, 0, data, offset, byteCount); return out.toByteArray(); } + + private byte[] windowUpdate(int increment) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + new Spdy3.Writer(out, true).windowUpdate(expectedStreamId, increment); + return out.toByteArray(); + } } 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 313bcbc74..31c945067 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 @@ -15,7 +15,6 @@ */ package com.squareup.okhttp.internal.spdy; -import com.squareup.okhttp.Protocol; import com.squareup.okhttp.internal.Base64; import com.squareup.okhttp.internal.Util; import java.io.ByteArrayOutputStream; @@ -27,6 +26,7 @@ import java.util.Arrays; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import org.junit.After; +import org.junit.Ignore; import org.junit.Test; import static com.squareup.okhttp.internal.Util.UTF_8; @@ -189,9 +189,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = - new SpdyConnection.Builder(true, peer.openSocket()).handler(REJECT_INCOMING_STREAMS) - .build(); + SpdyConnection connection = connection(peer, Variant.SPDY3); connection.noop(); // verify the peer received what was expected @@ -206,7 +204,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - new SpdyConnection.Builder(true, peer.openSocket()).handler(REJECT_INCOMING_STREAMS).build(); + connection(peer, Variant.SPDY3); // verify the peer received what was expected MockSpdyPeer.InFrame ping = peer.takeFrame(); @@ -225,7 +223,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - http2Connection(peer); + connection(peer, Variant.HTTP_20_DRAFT_09); // verify the peer received what was expected MockSpdyPeer.InFrame ping = peer.takeFrame(); @@ -243,9 +241,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()) - .handler(REJECT_INCOMING_STREAMS) - .build(); + SpdyConnection connection = connection(peer, Variant.SPDY3); Ping ping = connection.ping(); assertTrue(ping.roundTripTime() > 0); assertTrue(ping.roundTripTime() < TimeUnit.SECONDS.toNanos(1)); @@ -267,7 +263,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = http2Connection(peer); + SpdyConnection connection = connection(peer, Variant.HTTP_20_DRAFT_09); Ping ping = connection.ping(); assertTrue(ping.roundTripTime() > 0); assertTrue(ping.roundTripTime() < TimeUnit.SECONDS.toNanos(1)); @@ -290,7 +286,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - new SpdyConnection.Builder(true, peer.openSocket()).handler(REJECT_INCOMING_STREAMS).build(); + connection(peer, Variant.SPDY3); // verify the peer received what was expected MockSpdyPeer.InFrame ping2 = peer.takeFrame(); @@ -338,9 +334,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()) - .handler(REJECT_INCOMING_STREAMS) - .build(); + SpdyConnection connection = connection(peer, Variant.SPDY3); peer.takeFrame(); // Guarantees that the peer Settings frame has been processed. synchronized (connection) { @@ -365,9 +359,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()) - .handler(REJECT_INCOMING_STREAMS) - .build(); + SpdyConnection connection = connection(peer, Variant.SPDY3); peer.takeFrame(); // Guarantees that the Settings frame has been processed. synchronized (connection) { @@ -391,7 +383,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - new SpdyConnection.Builder(true, peer.openSocket()).handler(REJECT_INCOMING_STREAMS).build(); + connection(peer, Variant.SPDY3); // verify the peer received what was expected MockSpdyPeer.InFrame rstStream = peer.takeFrame(); @@ -411,7 +403,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - new SpdyConnection.Builder(true, peer.openSocket()).handler(REJECT_INCOMING_STREAMS).build(); + connection(peer, Variant.SPDY3); // verify the peer received what was expected MockSpdyPeer.InFrame rstStream = peer.takeFrame(); @@ -433,9 +425,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()) - .handler(REJECT_INCOMING_STREAMS) - .build(); + SpdyConnection connection = connection(peer, Variant.SPDY3); SpdyStream stream = connection.newStream(headerEntries("a", "android"), true, false); OutputStream out = stream.getOutputStream(); out.write("square".getBytes(UTF_8)); @@ -479,9 +469,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()) - .handler(REJECT_INCOMING_STREAMS) - .build(); + SpdyConnection connection = connection(peer, Variant.SPDY3); SpdyStream stream = connection.newStream(headerEntries("a", "android"), true, true); OutputStream out = stream.getOutputStream(); connection.ping().roundTripTime(); // Ensure that the RST_CANCEL has been received. @@ -521,9 +509,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()) - .handler(REJECT_INCOMING_STREAMS) - .build(); + SpdyConnection connection = connection(peer, Variant.SPDY3); SpdyStream stream = connection.newStream(headerEntries("a", "android"), false, true); InputStream in = stream.getInputStream(); OutputStream out = stream.getOutputStream(); @@ -566,9 +552,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()) - .handler(REJECT_INCOMING_STREAMS) - .build(); + SpdyConnection connection = connection(peer, Variant.SPDY3); SpdyStream stream = connection.newStream(headerEntries("a", "android"), true, true); InputStream in = stream.getInputStream(); OutputStream out = stream.getOutputStream(); @@ -610,9 +594,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()) - .handler(REJECT_INCOMING_STREAMS) - .build(); + SpdyConnection connection = connection(peer, Variant.SPDY3); SpdyStream stream = connection.newStream(headerEntries("a", "android"), false, true); InputStream in = stream.getInputStream(); assertStreamData("square", in); @@ -1006,7 +988,24 @@ public final class SpdyConnectionTest { } @Test public void readSendsWindowUpdate() throws Exception { - int windowUpdateThreshold = Variant.SPDY3.initialPeerSettings(true).getInitialWindowSize() / 2; + readSendsWindowUpdate(Variant.SPDY3); + } + + /** + * This test fails on http/2 as it tries to send too large data frame. In + * practice, {@link SpdyStream#OUTPUT_BUFFER_SIZE} prevents us from sending + * too large frames. The test should probably be rewritten to take into + * account max frame size per variant. + */ + @Test @Ignore public void readSendsWindowUpdateHttp2() throws Exception { + readSendsWindowUpdate(Variant.HTTP_20_DRAFT_09); + } + + private void readSendsWindowUpdate(Variant variant) + throws IOException, InterruptedException { + MockSpdyPeer peer = new MockSpdyPeer(variant, false); + int windowUpdateThreshold = variant.initialPeerSettings(true).getInitialWindowSize() / 2; + // Write the mocking script. peer.acceptFrame(); // SYN_STREAM peer.sendFrame().synReply(false, 1, headerEntries("a", "android")); @@ -1018,7 +1017,7 @@ public final class SpdyConnectionTest { peer.play(); // Play it back. - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()).build(); + SpdyConnection connection = connection(peer, variant); SpdyStream stream = connection.newStream(headerEntries("b", "banana"), true, true); assertEquals(windowUpdateThreshold, stream.windowUpdateThreshold); assertEquals(headerEntries("a", "android"), stream.getResponseHeaders()); @@ -1039,7 +1038,7 @@ public final class SpdyConnectionTest { MockSpdyPeer.InFrame windowUpdate = peer.takeFrame(); assertEquals(TYPE_WINDOW_UPDATE, windowUpdate.type); assertEquals(1, windowUpdate.streamId); - assertEquals(windowUpdateThreshold, windowUpdate.deltaWindowSize); + assertEquals(windowUpdateThreshold, windowUpdate.windowSizeIncrement); } } @@ -1112,7 +1111,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = http2Connection(peer); + SpdyConnection connection = connection(peer, Variant.HTTP_20_DRAFT_09); SpdyStream stream = connection.newStream(headerEntries("b", "banana"), true, true); OutputStream out = stream.getOutputStream(); out.write(buff); @@ -1172,7 +1171,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - http2Connection(peer); + connection(peer, Variant.HTTP_20_DRAFT_09); // verify the peer received what was expected MockSpdyPeer.InFrame rstStream = peer.takeFrame(); @@ -1189,7 +1188,7 @@ public final class SpdyConnectionTest { peer.play(); // play it back - SpdyConnection connection = http2Connection(peer); + SpdyConnection connection = connection(peer, Variant.HTTP_20_DRAFT_09); // verify the peer received the ACK MockSpdyPeer.InFrame pingFrame = peer.takeFrame(); @@ -1201,9 +1200,9 @@ public final class SpdyConnectionTest { return connection; } - private SpdyConnection http2Connection(MockSpdyPeer peer) throws IOException { + private SpdyConnection connection(MockSpdyPeer peer, Variant variant) throws IOException { return new SpdyConnection.Builder(true, peer.openSocket()) - .protocol(Protocol.HTTP_2) + .protocol(variant.getProtocol()) .handler(REJECT_INCOMING_STREAMS).build(); } From 28017b150683316ebb60b6d5be94c5c749ceb9bc Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Wed, 22 Jan 2014 12:24:02 +0100 Subject: [PATCH 2/2] Add GOAWAY support to http/2 and backfill tests. --- .../okhttp/internal/spdy/FrameReader.java | 18 ++++- .../okhttp/internal/spdy/FrameWriter.java | 18 ++++- .../okhttp/internal/spdy/Http20Draft09.java | 42 ++++++---- .../squareup/okhttp/internal/spdy/Spdy3.java | 24 +++--- .../okhttp/internal/spdy/SpdyConnection.java | 10 ++- .../okhttp/internal/spdy/SpdyStream.java | 4 +- .../okhttp/internal/spdy/BaseTestHandler.java | 5 +- .../internal/spdy/Http20Draft09Test.java | 77 +++++++++++++++++-- .../okhttp/internal/spdy/MockSpdyPeer.java | 8 +- .../okhttp/internal/spdy/Spdy3Test.java | 53 ++++++++++++- .../internal/spdy/SpdyConnectionTest.java | 15 +++- 11 files changed, 224 insertions(+), 50 deletions(-) diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameReader.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameReader.java index dc03bf299..36e2e152a 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameReader.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameReader.java @@ -61,13 +61,25 @@ public interface FrameReader extends Closeable { * set. The data is opaque binary, and there are no rules on the content. */ void ping(boolean ack, int payload1, int payload2); - void goAway(int lastGoodStreamId, ErrorCode errorCode); + + /** + * The peer tells us to stop creating streams. It is safe to replay + * streams with {@code ID > lastGoodStreamId} on a new connection. In- + * flight streams with {@code ID <= lastGoodStreamId} can only be replayed + * on a new connection if they are idempotent. + * + * @param lastGoodStreamId the last stream ID the peer processed before + * sending this message. If {@lastGoodStreamId} is zero, the peer processed no frames. + * @param errorCode reason for closing the connection. + * @param debugData only valid for http/2; opaque debug data to send. + */ + void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData); /** * Notifies that an additional {@code windowSizeIncrement} bytes can be - * sent on {@code streamId} or the connection, if {@code streamId} is zero. + * sent on {@code streamId}, or the connection if {@code streamId} is zero. */ - void windowUpdate(int streamId, int windowSizeIncrement); + void windowUpdate(int streamId, long windowSizeIncrement); void priority(int streamId, int priority); /** diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameWriter.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameWriter.java index 01aff525a..e508c96f1 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameWriter.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameWriter.java @@ -65,6 +65,7 @@ public interface FrameWriter extends Closeable { */ void data(boolean outFinished, int streamId, byte[] data, int offset, int byteCount) throws IOException; + /** Write okhttp's settings to the peer. */ void settings(Settings okHttpSettings) throws IOException; void noop() throws IOException; @@ -81,10 +82,21 @@ public interface FrameWriter extends Closeable { * sent. The data is opaque binary, and there are no rules on the content. */ void ping(boolean ack, int payload1, int payload2) throws IOException; - void goAway(int lastGoodStreamId, ErrorCode errorCode) throws IOException; + + /** + * Tell the peer to stop creating streams and that we last processed + * {@code lastGoodStreamId}, or zero if no streams were processed. + * + * @param lastGoodStreamId the last stream ID processed, or zero if no + * streams were processed. + * @param errorCode reason for closing the connection. + * @param debugData only valid for http/2; opaque debug data to send. + */ + void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData) throws IOException; + /** * Inform peer that an additional {@code windowSizeIncrement} bytes can be - * sent on {@code streamId} or the connection, if {@code streamId} is zero. + * sent on {@code streamId}, or the connection if {@code streamId} is zero. */ - void windowUpdate(int streamId, int windowSizeIncrement) throws IOException; + void windowUpdate(int streamId, long windowSizeIncrement) throws IOException; } 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 cae2fd3af..f3c54844d 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 @@ -118,7 +118,7 @@ public final class Http20Draft09 implements Variant { // boolean r = (w1 & 0xc0000000) != 0; // Reserved. int length = (w1 & 0x3fff0000) >> 16; // 14-bit unsigned. - if ((length & 0xffffffffL) > 16383) { + if (length > 16383) { throw new IOException("FRAME_SIZE_ERROR max size is 16383: " + length); } int type = (w1 & 0xff00) >> 8; @@ -269,23 +269,26 @@ public final class Http20Draft09 implements Variant { private void readGoAway(Handler handler, int flags, int length, int streamId) throws IOException { if (length < 8) throw ioException("TYPE_GOAWAY length < 8: %s", length); + if (streamId != 0) throw ioException("TYPE_GOAWAY streamId != 0"); int lastStreamId = in.readInt(); int errorCodeInt = in.readInt(); int opaqueDataLength = length - 8; ErrorCode errorCode = ErrorCode.fromHttp2(errorCodeInt); if (errorCode == null) { - throw ioException("TYPE_RST_STREAM unexpected error code: %d", errorCodeInt); + throw ioException("TYPE_GOAWAY unexpected error code: %d", errorCodeInt); } - if (Util.skipByReading(in, opaqueDataLength) != opaqueDataLength) { - throw new IOException("TYPE_GOAWAY opaque data was truncated"); + byte[] debugData = Util.EMPTY_BYTE_ARRAY; + if (opaqueDataLength > 0) { // Must read debug data in order to not corrupt the connection. + debugData = new byte[opaqueDataLength]; + Util.readFully(in, debugData); } - handler.goAway(lastStreamId, errorCode); + handler.goAway(lastStreamId, errorCode, debugData); } private void readWindowUpdate(Handler handler, int flags, int length, int streamId) throws IOException { if (length != 4) throw ioException("TYPE_WINDOW_UPDATE length !=4: %s", length); - int increment = (in.readInt() & 0x7fffffff); + long increment = (in.readInt() & 0x7fffffff); if (increment == 0) throw ioException("windowSizeIncrement was 0", increment); handler.windowUpdate(streamId, increment); } @@ -432,20 +435,33 @@ public final class Http20Draft09 implements Variant { out.writeInt(payload2); } - @Override public synchronized void goAway(int lastGoodStreamId, ErrorCode errorCode) + @Override + public synchronized void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData) throws IOException { - // TODO + if (errorCode.httpCode == -1) { + throw new IllegalArgumentException("errorCode.httpCode == -1"); + } + int length = 8 + debugData.length; + checkFrameSize(length); + out.writeInt((length & 0x3fff) << 16 | (TYPE_GOAWAY & 0xff) << 8); + out.writeInt(0); // connection-level + out.writeInt(lastGoodStreamId); + out.writeInt(errorCode.httpCode); + if (debugData.length > 0) { + out.write(debugData); + } + out.flush(); } - @Override public synchronized void windowUpdate(int streamId, int increment) + @Override public synchronized void windowUpdate(int streamId, long increment) throws IOException { - if (increment == 0 || (increment & 0xffffffffL) > 0x7fffffffL) { + if (increment == 0 || increment > 0x7fffffffL) { throw new IllegalArgumentException( - "windowSizeIncrement must be between 1 and 0x7fffffff: " + (increment & 0xffffffffL)); + "windowSizeIncrement must be between 1 and 0x7fffffff: " + increment); } out.writeInt(4 << 16 | (TYPE_WINDOW_UPDATE & 0xff) << 8); // No flags. out.writeInt(streamId); - out.writeInt(increment); + out.writeInt((int) increment); } @Override public void close() throws IOException { @@ -454,7 +470,7 @@ public final class Http20Draft09 implements Variant { } private static void checkFrameSize(int bytes) throws IOException { - if ((bytes & 0xffffffffL) > 16383) { + if (bytes > 16383) { throw new IllegalArgumentException("FRAME_SIZE_ERROR max size is 16383: " + bytes); } } diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java index f837d76e2..184b901b3 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java @@ -252,7 +252,7 @@ final class Spdy3 implements Variant { int w1 = in.readInt(); int w2 = in.readInt(); int streamId = w1 & 0x7fffffff; - int increment = w2 & 0x7fffffff; + long increment = w2 & 0x7fffffff; if (increment == 0) throw ioException("windowSizeIncrement was 0", increment); handler.windowUpdate(streamId, increment); } @@ -272,7 +272,7 @@ final class Spdy3 implements Variant { if (errorCode == null) { throw ioException("TYPE_GOAWAY unexpected error code: %d", errorCodeInt); } - handler.goAway(lastGoodStreamId, errorCode); + handler.goAway(lastGoodStreamId, errorCode, Util.EMPTY_BYTE_ARRAY); } private void readSettings(Handler handler, int flags, int length) throws IOException { @@ -411,9 +411,8 @@ final class Spdy3 implements Variant { void sendDataFrame(int streamId, int flags, byte[] data, int offset, int byteCount) throws IOException { - if ((byteCount & 0xffffffffL) > 0xffffffL) { - throw new IllegalArgumentException( - "FRAME_TOO_LARGE max size is 16Mib: " + (byteCount & 0xffffffffL)); + if (byteCount > 0xffffffL) { + throw new IllegalArgumentException("FRAME_TOO_LARGE max size is 16Mib: " + byteCount); } out.writeInt(streamId & 0x7fffffff); out.writeInt((flags & 0xff) << 24 | byteCount & 0xffffff); @@ -473,9 +472,12 @@ final class Spdy3 implements Variant { out.flush(); } - @Override public synchronized void goAway(int lastGoodStreamId, ErrorCode errorCode) + @Override + public synchronized void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[] ignored) throws IOException { - if (errorCode.spdyGoAwayCode == -1) throw new IllegalArgumentException(); + if (errorCode.spdyGoAwayCode == -1) { + throw new IllegalArgumentException("errorCode.spdyGoAwayCode == -1"); + } int type = TYPE_GOAWAY; int flags = 0; int length = 8; @@ -486,11 +488,11 @@ final class Spdy3 implements Variant { out.flush(); } - @Override public synchronized void windowUpdate(int streamId, int increment) + @Override public synchronized void windowUpdate(int streamId, long increment) throws IOException { - if (increment == 0 || (increment & 0xffffffffL) > 0x7fffffffL) { + if (increment == 0 || increment > 0x7fffffffL) { throw new IllegalArgumentException( - "windowSizeIncrement must be between 1 and 0x7fffffff: " + (increment & 0xffffffffL)); + "windowSizeIncrement must be between 1 and 0x7fffffff: " + increment); } int type = TYPE_WINDOW_UPDATE; int flags = 0; @@ -498,7 +500,7 @@ final class Spdy3 implements Variant { out.writeInt(0x80000000 | (VERSION & 0x7fff) << 16 | type & 0xffff); out.writeInt((flags & 0xff) << 24 | length & 0xffffff); out.writeInt(streamId); - out.writeInt(increment); + out.writeInt((int) increment); out.flush(); } 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 3056eac18..c8cb75093 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 @@ -303,7 +303,8 @@ public final class SpdyConnection implements Closeable { shutdown = true; lastGoodStreamId = this.lastGoodStreamId; } - frameWriter.goAway(lastGoodStreamId, statusCode); + // TODO: propagate exception message into debugData + frameWriter.goAway(lastGoodStreamId, statusCode, Util.EMPTY_BYTE_ARRAY); } } @@ -597,7 +598,10 @@ public final class SpdyConnection implements Closeable { } } - @Override public void goAway(int lastGoodStreamId, ErrorCode errorCode) { + @Override + public void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData) { + if (debugData.length > 0) { // TODO: log the debugData + } synchronized (SpdyConnection.this) { shutdown = true; @@ -614,7 +618,7 @@ public final class SpdyConnection implements Closeable { } } - @Override public void windowUpdate(int streamId, int windowSizeIncrement) { + @Override public void windowUpdate(int streamId, long windowSizeIncrement) { if (streamId == 0) { // TODO: honor connection-level flow control return; diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java index 4d00e1556..68ab921d6 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java @@ -325,7 +325,7 @@ public final class SpdyStream { notifyAll(); } - synchronized void receiveWindowUpdate(int windowSizeIncrement) { + synchronized void receiveWindowUpdate(long windowSizeIncrement) { out.unacknowledgedBytes -= windowSizeIncrement; notifyAll(); } @@ -594,7 +594,7 @@ public final class SpdyStream { * acknowledged with an incoming {@code WINDOW_UPDATE} frame. Writes * block if they cause this to exceed the {@code WINDOW_SIZE}. */ - private int unacknowledgedBytes = 0; + private long unacknowledgedBytes = 0; @Override public void write(int b) throws IOException { Util.writeSingleByte(this, b); diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/BaseTestHandler.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/BaseTestHandler.java index 5eb81f406..dc08d2f6f 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/BaseTestHandler.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/BaseTestHandler.java @@ -49,12 +49,13 @@ class BaseTestHandler implements FrameReader.Handler { fail(); } - @Override public void goAway(int lastGoodStreamId, ErrorCode errorCode) { + @Override + public void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData) { fail(); } @Override - public void windowUpdate(int streamId, int windowSizeIncrement) { + public void windowUpdate(int streamId, long windowSizeIncrement) { fail(); } 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 4926d998d..8f7c91714 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 @@ -357,14 +357,14 @@ public class Http20Draft09Test { ByteArrayOutputStream out = new ByteArrayOutputStream(); DataOutputStream dataOut = new DataOutputStream(out); - final int expectedWindowSizeIncrement = 0x7fffffff; + final long expectedWindowSizeIncrement = 0x7fffffff; // Compose the expected window update frame. dataOut.writeShort(4); // length dataOut.write(Http20Draft09.TYPE_WINDOW_UPDATE); dataOut.write(0); // No flags. dataOut.writeInt(expectedStreamId); - dataOut.writeInt(expectedWindowSizeIncrement); + dataOut.writeInt((int) expectedWindowSizeIncrement); // Check writer sends the same bytes. assertArrayEquals(out.toByteArray(), windowUpdate(expectedWindowSizeIncrement)); @@ -372,7 +372,7 @@ public class Http20Draft09Test { FrameReader fr = newReader(out); fr.nextFrame(new BaseTestHandler() { // Consume the window update frame. - public void windowUpdate(int streamId, int windowSizeIncrement) { + @Override public void windowUpdate(int streamId, long windowSizeIncrement) { assertEquals(expectedStreamId, streamId); assertEquals(expectedWindowSizeIncrement, windowSizeIncrement); } @@ -387,7 +387,7 @@ public class Http20Draft09Test { assertEquals("windowSizeIncrement must be between 1 and 0x7fffffff: 0", e.getMessage()); } try { - windowUpdate(0x80000000); + windowUpdate(0x80000000L); fail(); } catch (IllegalArgumentException e) { assertEquals("windowSizeIncrement must be between 1 and 0x7fffffff: 2147483648", @@ -395,6 +395,66 @@ public class Http20Draft09Test { } } + @Test public void goAwayWithoutDebugDataRoundTrip() throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + DataOutputStream dataOut = new DataOutputStream(out); + + final ErrorCode expectedError = ErrorCode.PROTOCOL_ERROR; + + // Compose the expected GOAWAY frame without debug data. + dataOut.writeShort(8); // Without debug data there's only 2 32-bit fields. + dataOut.write(Http20Draft09.TYPE_GOAWAY); + dataOut.write(0); // no flags. + dataOut.writeInt(0); // connection-scope + dataOut.writeInt(expectedStreamId); // last good stream. + dataOut.writeInt(expectedError.httpCode); + + // Check writer sends the same bytes. + assertArrayEquals(out.toByteArray(), + sendGoAway(expectedStreamId, expectedError, Util.EMPTY_BYTE_ARRAY)); + + FrameReader fr = newReader(out); + + fr.nextFrame(new BaseTestHandler() { // Consume the go away frame. + @Override public void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData) { + assertEquals(expectedStreamId, lastGoodStreamId); + assertEquals(expectedError, errorCode); + assertEquals(0, debugData.length); + } + }); + } + + @Test public void goAwayWithDebugDataRoundTrip() throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + DataOutputStream dataOut = new DataOutputStream(out); + + final ErrorCode expectedError = ErrorCode.PROTOCOL_ERROR; + final byte[] expectedData = new byte[8]; + Arrays.fill(expectedData, (byte) '*'); + + // Compose the expected GOAWAY frame without debug data. + dataOut.writeShort(8 + expectedData.length); + dataOut.write(Http20Draft09.TYPE_GOAWAY); + dataOut.write(0); // no flags. + dataOut.writeInt(0); // connection-scope + dataOut.writeInt(0); // never read any stream! + dataOut.writeInt(expectedError.httpCode); + dataOut.write(expectedData); + + // Check writer sends the same bytes. + assertArrayEquals(out.toByteArray(), sendGoAway(0, expectedError, expectedData)); + + FrameReader fr = newReader(out); + + fr.nextFrame(new BaseTestHandler() { // Consume the go away frame. + @Override public void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData) { + assertEquals(0, lastGoodStreamId); + assertEquals(expectedError, errorCode); + assertArrayEquals(expectedData, debugData); + } + }); + } + private Http20Draft09.Reader newReader(ByteArrayOutputStream out) { return new Http20Draft09.Reader(new ByteArrayInputStream(out.toByteArray()), Variant.HTTP_20_DRAFT_09.initialPeerSettings(false).getHeaderTableSize(), false); @@ -412,6 +472,13 @@ public class Http20Draft09Test { return out.toByteArray(); } + private byte[] sendGoAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData) + throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + new Http20Draft09.Writer(out, true).goAway(lastGoodStreamId, errorCode, debugData); + return out.toByteArray(); + } + private byte[] sendDataFrame(byte[] data) throws IOException { return sendDataFrame(data, 0, data.length); } @@ -422,7 +489,7 @@ public class Http20Draft09Test { return out.toByteArray(); } - private byte[] windowUpdate(int windowSizeIncrement) throws IOException { + private byte[] windowUpdate(long windowSizeIncrement) throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); new Http20Draft09.Writer(out, true).windowUpdate(expectedStreamId, windowSizeIncrement); return out.toByteArray(); diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java index 3e36b1c71..7ce058e82 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java @@ -185,7 +185,7 @@ public final class MockSpdyPeer implements Closeable { public int associatedStreamId; public int priority; public ErrorCode errorCode; - public int windowSizeIncrement; + public long windowSizeIncrement; public List
headerBlock; public byte[] data; public Settings settings; @@ -250,14 +250,16 @@ public final class MockSpdyPeer implements Closeable { this.type = Spdy3.TYPE_NOOP; } - @Override public void goAway(int lastGoodStreamId, ErrorCode errorCode) { + @Override + public void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData) { if (this.type != -1) throw new IllegalStateException(); this.type = Spdy3.TYPE_GOAWAY; this.streamId = lastGoodStreamId; this.errorCode = errorCode; + this.data = debugData; } - @Override public void windowUpdate(int streamId, int windowSizeIncrement) { + @Override public void windowUpdate(int streamId, long windowSizeIncrement) { if (this.type != -1) throw new IllegalStateException(); this.type = Spdy3.TYPE_WINDOW_UPDATE; this.streamId = streamId; diff --git a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Spdy3Test.java b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Spdy3Test.java index a7520b360..c6adebb51 100644 --- a/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Spdy3Test.java +++ b/okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Spdy3Test.java @@ -15,10 +15,14 @@ */ package com.squareup.okhttp.internal.spdy; +import com.squareup.okhttp.internal.Util; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.DataOutputStream; import java.io.IOException; import org.junit.Test; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -30,7 +34,7 @@ public class Spdy3Test { sendDataFrame(new byte[0x1000000]); fail(); } catch (IllegalArgumentException e) { - assertEquals("FRAME_TOO_LARGE max size is 16Mib: 16777216", e.getMessage()); + assertEquals("FRAME_TOO_LARGE max size is 16Mib: " + 0x1000000L, e.getMessage()); } } @@ -42,7 +46,7 @@ public class Spdy3Test { assertEquals("windowSizeIncrement must be between 1 and 0x7fffffff: 0", e.getMessage()); } try { - windowUpdate(0x80000000); + windowUpdate(0x80000000L); fail(); } catch (IllegalArgumentException e) { assertEquals("windowSizeIncrement must be between 1 and 0x7fffffff: 2147483648", @@ -50,6 +54,42 @@ public class Spdy3Test { } } + @Test public void goAwayRoundTrip() throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + DataOutputStream dataOut = new DataOutputStream(out); + + final ErrorCode expectedError = ErrorCode.PROTOCOL_ERROR; + + // Compose the expected GOAWAY frame without debug data + // |C| Version(15bits) | Type(16bits) | + dataOut.writeInt(0x80000000 | (Spdy3.VERSION & 0x7fff) << 16 | Spdy3.TYPE_GOAWAY & 0xffff); + // | Flags (8) | Length (24 bits) | + dataOut.writeInt(8); // no flags and length is 8. + dataOut.writeInt(expectedStreamId); // last good stream. + dataOut.writeInt(expectedError.spdyGoAwayCode); + + // Check writer sends the same bytes. + assertArrayEquals(out.toByteArray(), + sendGoAway(expectedStreamId, expectedError, Util.EMPTY_BYTE_ARRAY)); + + // SPDY/3 does not send debug data, so bytes should be same! + assertArrayEquals(out.toByteArray(), sendGoAway(expectedStreamId, expectedError, new byte[8])); + + FrameReader fr = newReader(out); + + fr.nextFrame(new BaseTestHandler() { // Consume the goAway frame. + @Override public void goAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData) { + assertEquals(expectedStreamId, lastGoodStreamId); + assertEquals(expectedError, errorCode); + assertEquals(0, debugData.length); + } + }); + } + + private Spdy3.Reader newReader(ByteArrayOutputStream out) { + return new Spdy3.Reader(new ByteArrayInputStream(out.toByteArray()), false); + } + private byte[] sendDataFrame(byte[] data) throws IOException { return sendDataFrame(data, 0, data.length); } @@ -60,9 +100,16 @@ public class Spdy3Test { return out.toByteArray(); } - private byte[] windowUpdate(int increment) throws IOException { + private byte[] windowUpdate(long increment) throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); new Spdy3.Writer(out, true).windowUpdate(expectedStreamId, increment); return out.toByteArray(); } + + private byte[] sendGoAway(int lastGoodStreamId, ErrorCode errorCode, byte[] debugData) + throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + new Spdy3.Writer(out, true).goAway(lastGoodStreamId, errorCode, debugData); + return out.toByteArray(); + } } 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 31c945067..d89d6648c 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 @@ -754,18 +754,29 @@ public final class SpdyConnectionTest { assertEquals(2, ping.payload1); } + @Test public void receiveGoAway() throws Exception { + receiveGoAway(Variant.SPDY3); + } + + @Test public void receiveGoAwayHttp2() throws Exception { + receiveGoAway(Variant.HTTP_20_DRAFT_09); + } + + private void receiveGoAway(Variant variant) throws Exception { + MockSpdyPeer peer = new MockSpdyPeer(variant, false); + // write the mocking script peer.acceptFrame(); // SYN_STREAM 1 peer.acceptFrame(); // SYN_STREAM 3 - peer.sendFrame().goAway(1, PROTOCOL_ERROR); + peer.sendFrame().goAway(1, PROTOCOL_ERROR, Util.EMPTY_BYTE_ARRAY); peer.acceptFrame(); // PING peer.sendFrame().ping(true, 1, 0); peer.acceptFrame(); // DATA STREAM 1 peer.play(); // play it back - SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()).build(); + SpdyConnection connection = connection(peer, variant); SpdyStream stream1 = connection.newStream(headerEntries("a", "android"), true, true); SpdyStream stream2 = connection.newStream(headerEntries("b", "banana"), true, true); connection.ping().roundTripTime(); // Ensure that the GO_AWAY has been received.