From b9580d68e9bc34eef397e9bc7f13091f1e3f1528 Mon Sep 17 00:00:00 2001 From: jwilson Date: Sat, 19 Jan 2013 11:37:24 -0500 Subject: [PATCH] Be more careful around IOExceptions. This adds a first test where a frame is truncated. That has some unpleasant consequences for the other tests because it means the MockSpdyPeer is more aggressive about closing the socket when all frames have been sent. It also reduces the use of exceptions for flow control when handling bogus incoming frames. This is a bit worse and a bit better; my goal is to make it easier to differentiate between protocol-level problems (bogus frames) from transport-level problems (closed sockets and EOF streams). --- .../com/squareup/okhttp/internal/Util.java | 11 ++ .../okhttp/internal/spdy/SpdyConnection.java | 76 ++++++----- .../okhttp/internal/spdy/SpdyReader.java | 2 +- .../okhttp/internal/spdy/SpdyStream.java | 54 +++++--- .../okhttp/internal/spdy/SpdyWriter.java | 2 +- .../okhttp/internal/spdy/Threads.java | 34 ----- .../okhttp/internal/spdy/MockSpdyPeer.java | 24 +++- .../internal/spdy/SpdyConnectionTest.java | 122 +++++++++++------- 8 files changed, 193 insertions(+), 132 deletions(-) delete mode 100644 src/main/java/com/squareup/okhttp/internal/spdy/Threads.java diff --git a/src/main/java/com/squareup/okhttp/internal/Util.java b/src/main/java/com/squareup/okhttp/internal/Util.java index f47362c02..d28db1e9e 100644 --- a/src/main/java/com/squareup/okhttp/internal/Util.java +++ b/src/main/java/com/squareup/okhttp/internal/Util.java @@ -28,6 +28,7 @@ import java.net.URI; import java.net.URL; import java.nio.ByteOrder; import java.nio.charset.Charset; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicReference; /** @@ -313,4 +314,14 @@ public final class Util { } return result.toString(); } + + public static ThreadFactory newThreadFactory(final String name, final boolean daemon) { + return new ThreadFactory() { + @Override public Thread newThread(Runnable r) { + Thread result = new Thread(r, name); + result.setDaemon(daemon); + return result; + } + }; + } } diff --git a/src/main/java/com/squareup/okhttp/internal/spdy/SpdyConnection.java b/src/main/java/com/squareup/okhttp/internal/spdy/SpdyConnection.java index 1f8b3b633..0f10cf3bc 100644 --- a/src/main/java/com/squareup/okhttp/internal/spdy/SpdyConnection.java +++ b/src/main/java/com/squareup/okhttp/internal/spdy/SpdyConnection.java @@ -117,11 +117,11 @@ public final class SpdyConnection implements Closeable { String prefix = builder.client ? "Spdy Client " : "Spdy Server "; readExecutor = new ThreadPoolExecutor(1, 1, 60, TimeUnit.SECONDS, - new SynchronousQueue(), Threads.newThreadFactory(prefix + "Reader", false)); + new SynchronousQueue(), Util.newThreadFactory(prefix + "Reader", false)); writeExecutor = new ThreadPoolExecutor(0, 1, 60, TimeUnit.SECONDS, - new LinkedBlockingQueue(), Threads.newThreadFactory(prefix + "Writer", false)); + new LinkedBlockingQueue(), Util.newThreadFactory(prefix + "Writer", false)); callbackExecutor = new ThreadPoolExecutor(0, Integer.MAX_VALUE, 60, TimeUnit.SECONDS, - new SynchronousQueue(), Threads.newThreadFactory(prefix + "Callbacks", false)); + new SynchronousQueue(), Util.newThreadFactory(prefix + "Callbacks", false)); readExecutor.execute(new Reader()); } @@ -317,7 +317,17 @@ public final class SpdyConnection implements Closeable { * internal executor services. */ @Override public void close() throws IOException { - shutdown(GOAWAY_OK); + close(GOAWAY_OK, SpdyStream.RST_CANCEL); + } + + private void close(int shutdownStatusCode, int rstStatusCode) throws IOException { + assert (!Thread.holdsLock(this)); + IOException thrown = null; + try { + shutdown(shutdownStatusCode); + } catch (IOException e) { + thrown = e; + } SpdyStream[] streamsToClose = null; Ping[] pingsToCancel = null; @@ -335,8 +345,9 @@ public final class SpdyConnection implements Closeable { if (streamsToClose != null) { for (SpdyStream stream : streamsToClose) { try { - stream.close(SpdyStream.RST_CANCEL); - } catch (Throwable ignored) { + stream.close(rstStatusCode); + } catch (IOException e) { + if (thrown != null) thrown = e; } } } @@ -350,7 +361,17 @@ public final class SpdyConnection implements Closeable { writeExecutor.shutdown(); callbackExecutor.shutdown(); readExecutor.shutdown(); - Util.closeAll(spdyReader, spdyWriter); + try { + spdyReader.close(); + } catch (IOException e) { + thrown = e; + } + try { + spdyWriter.close(); + } catch (IOException e) { + if (thrown == null) thrown = e; + } + if (thrown != null) throw thrown; } public static class Builder { @@ -389,15 +410,21 @@ public final class SpdyConnection implements Closeable { private class Reader implements Runnable, SpdyReader.Handler { @Override public void run() { + int shutdownStatusCode = GOAWAY_INTERNAL_ERROR; + int rstStatusCode = SpdyStream.RST_INTERNAL_ERROR; try { while (spdyReader.nextFrame(this)) { } - } catch (ProtocolException e) { - shutdownLater(GOAWAY_PROTOCOL_ERROR); + shutdownStatusCode = GOAWAY_OK; + rstStatusCode = SpdyStream.RST_CANCEL; } catch (IOException e) { - throw new RuntimeException(e); + shutdownStatusCode = GOAWAY_PROTOCOL_ERROR; + rstStatusCode = SpdyStream.RST_PROTOCOL_ERROR; } finally { - Util.closeQuietly(SpdyConnection.this); + try { + close(shutdownStatusCode, rstStatusCode); + } catch (IOException ignored) { + } } } @@ -409,14 +436,9 @@ public final class SpdyConnection implements Closeable { Util.skipByReading(in, length); return; } - try { - dataStream.receiveData(in, length); - if ((flags & SpdyConnection.FLAG_FIN) != 0) { - dataStream.receiveFin(); - } - } catch (ProtocolException e) { - Util.skipByReading(in, length); - dataStream.closeLater(SpdyStream.RST_FLOW_CONTROL_ERROR); + dataStream.receiveData(in, length); + if ((flags & SpdyConnection.FLAG_FIN) != 0) { + dataStream.receiveFin(); } } @@ -456,13 +478,9 @@ public final class SpdyConnection implements Closeable { writeSynResetLater(streamId, SpdyStream.RST_INVALID_STREAM); return; } - try { - replyStream.receiveReply(nameValueBlock); - if ((flags & SpdyConnection.FLAG_FIN) != 0) { - replyStream.receiveFin(); - } - } catch (ProtocolException e) { - replyStream.closeLater(SpdyStream.RST_STREAM_IN_USE); + replyStream.receiveReply(nameValueBlock); + if ((flags & SpdyConnection.FLAG_FIN) != 0) { + replyStream.receiveFin(); } } @@ -470,11 +488,7 @@ public final class SpdyConnection implements Closeable { throws IOException { SpdyStream replyStream = getStream(streamId); if (replyStream != null) { - try { - replyStream.receiveHeaders(nameValueBlock); - } catch (ProtocolException e) { - replyStream.closeLater(SpdyStream.RST_PROTOCOL_ERROR); - } + replyStream.receiveHeaders(nameValueBlock); } } diff --git a/src/main/java/com/squareup/okhttp/internal/spdy/SpdyReader.java b/src/main/java/com/squareup/okhttp/internal/spdy/SpdyReader.java index ef373acf4..d5abd3d70 100644 --- a/src/main/java/com/squareup/okhttp/internal/spdy/SpdyReader.java +++ b/src/main/java/com/squareup/okhttp/internal/spdy/SpdyReader.java @@ -30,7 +30,7 @@ import java.util.zip.Inflater; import java.util.zip.InflaterInputStream; /** - * Read version 2 SPDY frames. + * Read spdy/3 frames. */ final class SpdyReader implements Closeable { static final byte[] DICTIONARY = ("\u0000\u0000\u0000\u0007options\u0000\u0000\u0000\u0004hea" diff --git a/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java b/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java index bdf29f71b..48fa17b2a 100644 --- a/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java +++ b/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java @@ -23,7 +23,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.InterruptedIOException; import java.io.OutputStream; -import java.net.ProtocolException; import java.net.SocketTimeoutException; import static java.nio.ByteOrder.BIG_ENDIAN; import java.util.ArrayList; @@ -101,6 +100,8 @@ public final class SpdyStream { SpdyStream(int id, SpdyConnection connection, int flags, int priority, int slot, List requestHeaders, Settings settings) { + if (connection == null) throw new NullPointerException("connection == null"); + if (requestHeaders == null) throw new NullPointerException("requestHeaders == null"); this.id = id; this.connection = connection; this.priority = priority; @@ -124,7 +125,8 @@ public final class SpdyStream { * Returns true if this stream is open. A stream is open until either: *
    *
  • A {@code SYN_RESET} frame abnormally terminates the stream. - *
  • Both input and output streams have transmitted all data. + *
  • Both input and output streams have transmitted all data and + * headers. *
* Note that the input stream may continue to yield data even after a stream * reports itself as not open. This is because input data is buffered. @@ -133,7 +135,7 @@ public final class SpdyStream { if (rstStatusCode != -1) { return false; } - if ((in.finished || in.closed) && (out.finished || out.closed)) { + if ((in.finished || in.closed) && (out.finished || out.closed) && responseHeaders != null) { return false; } return true; @@ -287,25 +289,39 @@ public final class SpdyStream { void receiveReply(List strings) throws IOException { assert (!Thread.holdsLock(SpdyStream.this)); + boolean streamInUseError = false; + boolean open = true; synchronized (this) { - if (!isLocallyInitiated() || responseHeaders != null) { - throw new ProtocolException(); + if (isLocallyInitiated() && responseHeaders == null) { + responseHeaders = strings; + open = isOpen(); + notifyAll(); + } else { + streamInUseError = true; } - responseHeaders = strings; - notifyAll(); + } + if (streamInUseError) { + closeLater(SpdyStream.RST_STREAM_IN_USE); + } else if (!open) { + connection.removeStream(id); } } void receiveHeaders(List headers) throws IOException { assert (!Thread.holdsLock(SpdyStream.this)); + boolean protocolError = false; synchronized (this) { - if (responseHeaders == null) { - throw new ProtocolException(); + if (responseHeaders != null) { + List newHeaders = new ArrayList(); + newHeaders.addAll(responseHeaders); + newHeaders.addAll(headers); + this.responseHeaders = newHeaders; + } else { + protocolError = true; } - List newHeaders = new ArrayList(); - newHeaders.addAll(responseHeaders); - newHeaders.addAll(headers); - this.responseHeaders = newHeaders; + } + if (protocolError) { + closeLater(SpdyStream.RST_PROTOCOL_ERROR); } } @@ -513,14 +529,20 @@ public final class SpdyStream { int limit; int firstNewByte; boolean finished; + boolean flowControlError; synchronized (SpdyStream.this) { finished = this.finished; pos = this.pos; firstNewByte = this.limit; limit = this.limit; - if (byteCount > buffer.length - available()) { - throw new ProtocolException(); - } + flowControlError = byteCount > buffer.length - available(); + } + + // If the peer sends more data than we can handle, discard it and close the connection. + if (flowControlError) { + Util.skipByReading(in, byteCount); + closeLater(SpdyStream.RST_FLOW_CONTROL_ERROR); + return; } // Discard data received after the stream is finished. It's probably a benign race. diff --git a/src/main/java/com/squareup/okhttp/internal/spdy/SpdyWriter.java b/src/main/java/com/squareup/okhttp/internal/spdy/SpdyWriter.java index 43b2fd511..b884acbde 100644 --- a/src/main/java/com/squareup/okhttp/internal/spdy/SpdyWriter.java +++ b/src/main/java/com/squareup/okhttp/internal/spdy/SpdyWriter.java @@ -27,7 +27,7 @@ import java.util.List; import java.util.zip.Deflater; /** - * Write version 2 SPDY frames. + * Write spdy/3 frames. */ final class SpdyWriter implements Closeable { final DataOutputStream out; diff --git a/src/main/java/com/squareup/okhttp/internal/spdy/Threads.java b/src/main/java/com/squareup/okhttp/internal/spdy/Threads.java deleted file mode 100644 index ef1d4f3df..000000000 --- a/src/main/java/com/squareup/okhttp/internal/spdy/Threads.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (C) 2011 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.squareup.okhttp.internal.spdy; - -import java.util.concurrent.ThreadFactory; - -final class Threads { - public static ThreadFactory newThreadFactory(final String name, final boolean daemon) { - return new ThreadFactory() { - @Override public Thread newThread(Runnable r) { - Thread result = new Thread(r, name); - result.setDaemon(daemon); - return result; - } - }; - } - - private Threads() { - } -} diff --git a/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java b/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java index 11a36dd67..23e4bf579 100644 --- a/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java +++ b/src/test/java/com/squareup/okhttp/internal/spdy/MockSpdyPeer.java @@ -43,7 +43,7 @@ public final class MockSpdyPeer implements Closeable { private final BlockingQueue inFrames = new LinkedBlockingQueue(); private int port; private final Executor executor = Executors.newCachedThreadPool( - Threads.newThreadFactory("MockSpdyPeer", true)); + Util.newThreadFactory("MockSpdyPeer", true)); private ServerSocket serverSocket; private Socket socket; @@ -52,8 +52,17 @@ public final class MockSpdyPeer implements Closeable { } public SpdyWriter sendFrame() { - OutFrame frame = new OutFrame(frameCount++, bytesOut.size()); - outFrames.add(frame); + outFrames.add(new OutFrame(frameCount++, bytesOut.size(), Integer.MAX_VALUE)); + return spdyWriter; + } + + /** + * Sends a frame, truncated to {@code truncateToLength} bytes. This is only + * useful for testing error handling as the truncated frame will be + * malformed. + */ + public SpdyWriter sendTruncatedFrame(int truncateToLength) { + outFrames.add(new OutFrame(frameCount++, bytesOut.size(), truncateToLength)); return spdyWriter; } @@ -99,6 +108,7 @@ public final class MockSpdyPeer implements Closeable { if (nextOutFrame != null && nextOutFrame.sequence == i) { int start = nextOutFrame.start; + int truncateToLength = nextOutFrame.truncateToLength; int end; if (outFramesIterator.hasNext()) { nextOutFrame = outFramesIterator.next(); @@ -108,7 +118,8 @@ public final class MockSpdyPeer implements Closeable { } // write a frame - out.write(outBytes, start, end - start); + int length = Math.min(end - start, truncateToLength); + out.write(outBytes, start, length); } else { // read a frame @@ -117,6 +128,7 @@ public final class MockSpdyPeer implements Closeable { inFrames.add(inFrame); } } + Util.closeQuietly(socket); } public Socket openSocket() throws IOException { @@ -139,10 +151,12 @@ public final class MockSpdyPeer implements Closeable { private static class OutFrame { private final int sequence; private final int start; + private final int truncateToLength; - private OutFrame(int sequence, int start) { + private OutFrame(int sequence, int start, int truncateToLength) { this.sequence = sequence; this.start = start; + this.truncateToLength = truncateToLength; } } diff --git a/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java b/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java index 72cd1b959..c8f79b3de 100644 --- a/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java +++ b/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java @@ -16,6 +16,7 @@ package com.squareup.okhttp.internal.spdy; +import com.squareup.okhttp.internal.Util; import static com.squareup.okhttp.internal.Util.UTF_8; import static com.squareup.okhttp.internal.spdy.Settings.PERSIST_VALUE; import static com.squareup.okhttp.internal.spdy.SpdyConnection.FLAG_FIN; @@ -65,10 +66,10 @@ public final class SpdyConnectionTest { @Test public void clientCreatesStreamAndServerReplies() throws Exception { // write the mocking script - peer.acceptFrame(); + peer.acceptFrame(); // SYN_STREAM peer.sendFrame().synReply(0, 1, Arrays.asList("a", "android")); peer.sendFrame().data(SpdyConnection.FLAG_FIN, 1, "robot".getBytes("UTF-8")); - peer.acceptFrame(); + peer.acceptFrame(); // DATA peer.play(); // play it back @@ -90,19 +91,21 @@ public final class SpdyConnectionTest { assertTrue(Arrays.equals("c3po".getBytes("UTF-8"), requestData.data)); } - @Test public void headersOnlyStreamIsClosedImmediately() throws Exception { - peer.acceptFrame(); // SYN STREAM + @Test public void headersOnlyStreamIsClosedAfterReplyHeaders() throws Exception { + peer.acceptFrame(); // SYN_STREAM peer.sendFrame().synReply(0, 1, Arrays.asList("b", "banana")); peer.play(); SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()).build(); - connection.newStream(Arrays.asList("a", "android"), false, false); + SpdyStream stream = connection.newStream(Arrays.asList("a", "android"), false, false); + assertEquals(1, connection.openStreamCount()); + assertEquals(Arrays.asList("b", "banana"), stream.getResponseHeaders()); assertEquals(0, connection.openStreamCount()); } @Test public void clientCreatesStreamAndServerRepliesWithFin() throws Exception { // write the mocking script - peer.acceptFrame(); // SYN STREAM + peer.acceptFrame(); // SYN_STREAM peer.acceptFrame(); // PING peer.sendFrame().synReply(FLAG_FIN, 1, Arrays.asList("a", "android")); peer.sendFrame().ping(0, 1); @@ -125,7 +128,7 @@ public final class SpdyConnectionTest { @Test public void serverCreatesStreamAndClientReplies() throws Exception { // write the mocking script peer.sendFrame().synStream(0, 2, 0, 5, 129, Arrays.asList("a", "android")); - peer.acceptFrame(); + peer.acceptFrame(); // SYN_REPLY peer.play(); // play it back @@ -157,7 +160,7 @@ public final class SpdyConnectionTest { @Test public void replyWithNoData() throws Exception { // write the mocking script peer.sendFrame().synStream(0, 2, 0, 0, 0, Arrays.asList("a", "android")); - peer.acceptFrame(); + peer.acceptFrame(); // SYN_REPLY peer.play(); // play it back @@ -182,7 +185,7 @@ public final class SpdyConnectionTest { @Test public void noop() throws Exception { // write the mocking script - peer.acceptFrame(); + peer.acceptFrame(); // NOOP peer.play(); // play it back @@ -200,7 +203,7 @@ public final class SpdyConnectionTest { @Test public void serverPingsClient() throws Exception { // write the mocking script peer.sendFrame().ping(0, 2); - peer.acceptFrame(); + peer.acceptFrame(); // PING peer.play(); // play it back @@ -217,7 +220,7 @@ public final class SpdyConnectionTest { @Test public void clientPingsServer() throws Exception { // write the mocking script - peer.acceptFrame(); + peer.acceptFrame(); // PING peer.sendFrame().ping(0, 1); peer.play(); @@ -239,10 +242,10 @@ public final class SpdyConnectionTest { @Test public void unexpectedPingIsNotReturned() throws Exception { // write the mocking script peer.sendFrame().ping(0, 2); - peer.acceptFrame(); + peer.acceptFrame(); // PING peer.sendFrame().ping(0, 3); // This ping will not be returned. peer.sendFrame().ping(0, 4); - peer.acceptFrame(); + peer.acceptFrame(); // PING peer.play(); // play it back @@ -263,7 +266,7 @@ public final class SpdyConnectionTest { settings.set(Settings.MAX_CONCURRENT_STREAMS, PERSIST_VALUE, 10); peer.sendFrame().settings(Settings.FLAG_CLEAR_PREVIOUSLY_PERSISTED_SETTINGS, settings); peer.sendFrame().ping(0, 2); - peer.acceptFrame(); + peer.acceptFrame(); // PING peer.play(); // play it back @@ -360,10 +363,11 @@ public final class SpdyConnectionTest { @Test public void clientClosesClientOutputStream() throws Exception { // write the mocking script peer.acceptFrame(); // SYN_STREAM + peer.sendFrame().synReply(0, 1, Arrays.asList("b", "banana")); peer.acceptFrame(); // TYPE_DATA peer.acceptFrame(); // TYPE_DATA with FLAG_FIN - peer.sendFrame().ping(0, 2); - peer.acceptFrame(); // PING response + peer.acceptFrame(); // PING + peer.sendFrame().ping(0, 1); peer.play(); // play it back @@ -382,6 +386,7 @@ public final class SpdyConnectionTest { } catch (Exception expected) { assertEquals("stream closed", expected.getMessage()); } + connection.ping().roundTripTime(); // Ensure that the SYN_REPLY has been received. assertEquals(0, connection.openStreamCount()); // verify the peer received what was expected @@ -397,7 +402,7 @@ public final class SpdyConnectionTest { assertEquals(FLAG_FIN, fin.flags); MockSpdyPeer.InFrame ping = peer.takeFrame(); assertEquals(TYPE_PING, ping.type); - assertEquals(2, ping.streamId); + assertEquals(1, ping.streamId); } @Test public void serverClosesClientOutputStream() throws Exception { @@ -406,6 +411,7 @@ public final class SpdyConnectionTest { peer.sendFrame().rstStream(1, SpdyStream.RST_CANCEL); peer.acceptFrame(); // PING peer.sendFrame().ping(0, 1); + peer.acceptFrame(); // DATA peer.play(); // play it back @@ -431,6 +437,10 @@ public final class SpdyConnectionTest { MockSpdyPeer.InFrame ping = peer.takeFrame(); assertEquals(TYPE_PING, ping.type); assertEquals(1, ping.streamId); + MockSpdyPeer.InFrame data = peer.takeFrame(); + assertEquals(TYPE_DATA, data.type); + assertEquals(1, data.streamId); + assertEquals(FLAG_FIN, data.flags); } /** @@ -523,6 +533,7 @@ public final class SpdyConnectionTest { @Test public void serverClosesClientInputStream() throws Exception { // write the mocking script peer.acceptFrame(); // SYN_STREAM + peer.sendFrame().synReply(0, 1, Arrays.asList("b", "banana")); peer.sendFrame().data(FLAG_FIN, 1, "square".getBytes(UTF_8)); peer.play(); @@ -543,12 +554,12 @@ public final class SpdyConnectionTest { @Test public void remoteDoubleSynReply() throws Exception { // write the mocking script - peer.acceptFrame(); + peer.acceptFrame(); // SYN_STREAM peer.sendFrame().synReply(0, 1, Arrays.asList("a", "android")); peer.acceptFrame(); // PING peer.sendFrame().synReply(0, 1, Arrays.asList("b", "banana")); peer.sendFrame().ping(0, 1); - peer.acceptFrame(); // RST STREAM + peer.acceptFrame(); // RST_STREAM peer.play(); // play it back @@ -559,8 +570,8 @@ public final class SpdyConnectionTest { try { stream.getInputStream().read(); fail(); - } catch (IOException e) { - assertEquals("stream was reset: STREAM_IN_USE", e.getMessage()); + } catch (IOException expected) { + assertEquals("stream was reset: STREAM_IN_USE", expected.getMessage()); } // verify the peer received what was expected @@ -578,9 +589,9 @@ public final class SpdyConnectionTest { @Test public void remoteDoubleSynStream() throws Exception { // write the mocking script peer.sendFrame().synStream(0, 2, 0, 0, 0, Arrays.asList("a", "android")); - peer.acceptFrame(); + peer.acceptFrame(); // SYN_REPLY peer.sendFrame().synStream(0, 2, 0, 0, 0, Arrays.asList("b", "banana")); - peer.acceptFrame(); + peer.acceptFrame(); // RST_STREAM peer.play(); // play it back @@ -610,12 +621,12 @@ public final class SpdyConnectionTest { @Test public void remoteSendsDataAfterInFinished() throws Exception { // write the mocking script - peer.acceptFrame(); + peer.acceptFrame(); // SYN_STREAM peer.sendFrame().synReply(0, 1, Arrays.asList("a", "android")); peer.sendFrame().data(SpdyConnection.FLAG_FIN, 1, "robot".getBytes("UTF-8")); peer.sendFrame().data(SpdyConnection.FLAG_FIN, 1, "c3po".getBytes("UTF-8")); // Ignored. peer.sendFrame().ping(0, 2); // Ping just to make sure the stream was fastforwarded. - peer.acceptFrame(); + peer.acceptFrame(); // PING peer.play(); // play it back @@ -635,12 +646,12 @@ public final class SpdyConnectionTest { @Test public void remoteSendsTooMuchData() throws Exception { // write the mocking script - peer.acceptFrame(); + peer.acceptFrame(); // SYN_STREAM peer.sendFrame().synReply(0, 1, Arrays.asList("b", "banana")); peer.sendFrame().data(0, 1, new byte[64 * 1024 + 1]); - peer.acceptFrame(); + peer.acceptFrame(); // RST_STREAM peer.sendFrame().ping(0, 2); // Ping just to make sure the stream was fastforwarded. - peer.acceptFrame(); + peer.acceptFrame(); // PING peer.play(); // play it back @@ -663,10 +674,10 @@ public final class SpdyConnectionTest { @Test public void remoteSendsRefusedStreamBeforeReplyHeaders() throws Exception { // write the mocking script - peer.acceptFrame(); + peer.acceptFrame(); // SYN_STREAM peer.sendFrame().rstStream(1, RST_REFUSED_STREAM); peer.sendFrame().ping(0, 2); - peer.acceptFrame(); + peer.acceptFrame(); // PING peer.play(); // play it back @@ -691,8 +702,8 @@ public final class SpdyConnectionTest { @Test public void receiveGoAway() throws Exception { // write the mocking script - peer.acceptFrame(); // SYN STREAM 1 - peer.acceptFrame(); // SYN STREAM 3 + peer.acceptFrame(); // SYN_STREAM 1 + peer.acceptFrame(); // SYN_STREAM 3 peer.sendFrame().goAway(0, 1, GOAWAY_PROTOCOL_ERROR); peer.acceptFrame(); // PING peer.sendFrame().ping(0, 1); @@ -736,7 +747,7 @@ public final class SpdyConnectionTest { @Test public void sendGoAway() throws Exception { // write the mocking script - peer.acceptFrame(); // SYN STREAM 1 + peer.acceptFrame(); // SYN_STREAM 1 peer.acceptFrame(); // GOAWAY peer.acceptFrame(); // PING peer.sendFrame().synStream(0, 2, 0, 0, 0, Arrays.asList("b", "b")); // Should be ignored! @@ -748,8 +759,8 @@ public final class SpdyConnectionTest { connection.newStream(Arrays.asList("a", "android"), true, true); Ping ping = connection.ping(); connection.shutdown(GOAWAY_PROTOCOL_ERROR); - ping.roundTripTime(); // Ensure that the SYN STREAM has been received. assertEquals(1, connection.openStreamCount()); + ping.roundTripTime(); // Prevent the peer from exiting prematurely. // verify the peer received what was expected MockSpdyPeer.InFrame synStream1 = peer.takeFrame(); @@ -785,9 +796,9 @@ public final class SpdyConnectionTest { @Test public void close() throws Exception { // write the mocking script - peer.acceptFrame(); // SYN STREAM + peer.acceptFrame(); // SYN_STREAM peer.acceptFrame(); // GOAWAY - peer.acceptFrame(); // RST STREAM + peer.acceptFrame(); // RST_STREAM peer.play(); // play it back @@ -840,8 +851,10 @@ public final class SpdyConnectionTest { @Test public void readTimeoutExpires() throws Exception { // write the mocking script - peer.acceptFrame(); // SYN STREAM + peer.acceptFrame(); // SYN_STREAM peer.sendFrame().synReply(0, 1, Arrays.asList("a", "android")); + peer.acceptFrame(); // PING + peer.sendFrame().ping(0, 1); peer.play(); // play it back @@ -858,6 +871,7 @@ public final class SpdyConnectionTest { long elapsedNanos = System.nanoTime() - startNanos; assertEquals(1000d, TimeUnit.NANOSECONDS.toMillis(elapsedNanos), 200d /* 200ms delta */); assertEquals(1, connection.openStreamCount()); + connection.ping().roundTripTime(); // Prevent the peer from exiting prematurely. // verify the peer received what was expected MockSpdyPeer.InFrame synStream = peer.takeFrame(); @@ -866,7 +880,7 @@ public final class SpdyConnectionTest { @Test public void headers() throws Exception { // write the mocking script - peer.acceptFrame(); // SYN STREAM + peer.acceptFrame(); // SYN_STREAM peer.acceptFrame(); // PING peer.sendFrame().synReply(0, 1, Arrays.asList("a", "android")); peer.sendFrame().headers(0, 1, Arrays.asList("c", "c3po")); @@ -888,10 +902,10 @@ public final class SpdyConnectionTest { @Test public void headersBeforeReply() throws Exception { // write the mocking script - peer.acceptFrame(); // SYN STREAM + peer.acceptFrame(); // SYN_STREAM peer.acceptFrame(); // PING peer.sendFrame().headers(0, 1, Arrays.asList("c", "c3po")); - peer.acceptFrame(); // RST STREAM + peer.acceptFrame(); // RST_STREAM peer.sendFrame().ping(0, 1); peer.play(); @@ -902,8 +916,8 @@ public final class SpdyConnectionTest { try { stream.getResponseHeaders(); fail(); - } catch (IOException e) { - assertEquals("stream was reset: PROTOCOL_ERROR", e.getMessage()); + } catch (IOException expected) { + assertEquals("stream was reset: PROTOCOL_ERROR", expected.getMessage()); } // verify the peer received what was expected @@ -918,7 +932,7 @@ public final class SpdyConnectionTest { @Test public void readSendsWindowUpdate() throws Exception { // Write the mocking script. - peer.acceptFrame(); // SYN STREAM + peer.acceptFrame(); // SYN_STREAM peer.sendFrame().synReply(0, 1, Arrays.asList("a", "android")); for (int i = 0; i < 3; i++) { peer.sendFrame().data(0, 1, new byte[WINDOW_UPDATE_THRESHOLD]); @@ -954,7 +968,7 @@ public final class SpdyConnectionTest { @Test public void writeAwaitsWindowUpdate() throws Exception { // Write the mocking script. This accepts more data frames than necessary! - peer.acceptFrame(); // SYN STREAM + peer.acceptFrame(); // SYN_STREAM for (int i = 0; i < Settings.DEFAULT_INITIAL_WINDOW_SIZE / 1024; i++) { peer.acceptFrame(); // DATA } @@ -980,6 +994,26 @@ public final class SpdyConnectionTest { assertEquals(TYPE_DATA, data.type); } + @Test public void testTruncatedDataFrame() throws Exception { + // write the mocking script + peer.acceptFrame(); // SYN_STREAM + peer.sendFrame().synReply(0, 1, Arrays.asList("a", "android")); + peer.sendTruncatedFrame(8 + 100).data(0, 1, new byte[1024]); + peer.play(); + + // play it back + SpdyConnection connection = new SpdyConnection.Builder(true, peer.openSocket()).build(); + SpdyStream stream = connection.newStream(Arrays.asList("b", "banana"), true, true); + assertEquals(Arrays.asList("a", "android"), stream.getResponseHeaders()); + InputStream in = stream.getInputStream(); + try { + Util.readFully(in, new byte[101]); + fail(); + } catch (IOException expected) { + assertEquals("stream was reset: PROTOCOL_ERROR", expected.getMessage()); + } + } + private void writeAndClose(SpdyStream stream, String data) throws IOException { OutputStream out = stream.getOutputStream(); out.write(data.getBytes("UTF-8"));