Merge pull request #472 from adriancole/backfill-data-test

Backfilled http/2 data frame test and clarified comments.

okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/FrameWriter.java

@@ -52,7 +52,17 @@ public interface FrameWriter extends Closeable {
       throws IOException;
   void headers(int streamId, List<Header> headerBlock) throws IOException;
   void rstStream(int streamId, ErrorCode errorCode) throws IOException;
+
+  /**
+   * {@code data.length} may be longer than the max length of the variant's data frame.
+   * Implementations must send multiple frames as necessary.
+   */
   void data(boolean outFinished, int streamId, byte[] data) throws IOException;
+
+  /**
+   * {@code byteCount} may be longer than the max length of the variant's data frame.
+   * Implementations must send multiple frames as necessary.
+   */
   void data(boolean outFinished, int streamId, byte[] data, int offset, int byteCount)
       throws IOException;
   /** Write okhttp's settings to the peer. */

okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Http20Draft09.java

@@ -391,12 +391,16 @@ public final class Http20Draft09 implements Variant {
 
     @Override public synchronized void data(boolean outFinished, int streamId, byte[] data,
         int offset, int byteCount) throws IOException {
-      int type = TYPE_DATA;
       int flags = 0;
       if (outFinished) flags |= FLAG_END_STREAM;
       // TODO: Implement looping strategy.
+      sendDataFrame(streamId, flags, data, offset, byteCount);
+    }
+
+    void sendDataFrame(int streamId, int flags, byte[] data, int offset, int byteCount)
+        throws IOException {
       checkFrameSize(byteCount);
-      out.writeInt((byteCount & 0x3fff) << 16 | (type & 0xff) << 8 | (flags & 0xff));
+      out.writeInt((byteCount & 0x3fff) << 16 | (TYPE_DATA & 0xff) << 8 | (flags & 0xff));
       out.writeInt(streamId & 0x7fffffff);
       out.write(data, offset, byteCount);
     }

okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Spdy3.java

@@ -403,7 +403,16 @@ final class Spdy3 implements Variant {
 
     @Override public synchronized void data(boolean outFinished, int streamId, byte[] data,
         int offset, int byteCount) throws IOException {
+      // TODO: Implement looping strategy.
       int flags = (outFinished ? FLAG_FIN : 0);
+      sendDataFrame(streamId, flags, data, offset, byteCount);
+    }
+
+    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);
+      }
       out.writeInt(streamId & 0x7fffffff);
       out.writeInt((flags & 0xff) << 24 | byteCount & 0xffffff);
       out.write(data, offset, byteCount);

okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/SpdyStream.java

@@ -29,6 +29,7 @@ import static com.squareup.okhttp.internal.Util.checkOffsetAndCount;
 
 /** A logical bidirectional stream. */
 public final class SpdyStream {
+  static final int OUTPUT_BUFFER_SIZE = 8192;
 
   // Internal state is guarded by this. No long-running or potentially
   // blocking operations are performed while the lock is held.
@@ -576,7 +577,7 @@ public final class SpdyStream {
    * is not thread safe.
    */
   private final class SpdyDataOutputStream extends OutputStream {
-    private final byte[] buffer = SpdyStream.this.connection.bufferPool.getBuf(8192);
+    private final byte[] buffer = SpdyStream.this.connection.bufferPool.getBuf(OUTPUT_BUFFER_SIZE);
     private int pos = 0;
 
     /** True if the caller has closed this stream. */

okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Http20Draft09Test.java

@@ -15,10 +15,12 @@
  */
 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 java.io.InputStream;
 import java.util.Arrays;
 import java.util.List;
 import org.junit.Test;
@@ -28,6 +30,7 @@ import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public class Http20Draft09Test {
   static final int expectedStreamId = 15;
@@ -307,6 +310,49 @@ public class Http20Draft09Test {
     });
   }
 
+  @Test public void maxLengthDataFrame() throws IOException {
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+    DataOutputStream dataOut = new DataOutputStream(out);
+
+    final byte[] expectedData = new byte[16383];
+    Arrays.fill(expectedData, (byte) 2);
+
+    // Write the data frame.
+    dataOut.writeShort(expectedData.length);
+    dataOut.write(Http20Draft09.TYPE_DATA);
+    dataOut.write(0); // no flags
+    dataOut.writeInt(expectedStreamId & 0x7fffffff);
+    dataOut.write(expectedData);
+
+    // Check writer sends the same bytes.
+    assertArrayEquals(out.toByteArray(), sendDataFrame(expectedData));
+
+    FrameReader fr = newReader(out);
+
+    fr.nextFrame(new BaseTestHandler() {
+      @Override public void data(boolean inFinished, int streamId, InputStream in, int length)
+          throws IOException {
+        assertFalse(inFinished);
+        assertEquals(expectedStreamId, streamId);
+        assertEquals(16383, length);
+        byte[] data = new byte[length];
+        Util.readFully(in, data);
+        for (byte b : data){
+          assertEquals(2, b);
+        }
+      }
+    });
+  }
+
+  @Test public void tooLargeDataFrame() throws IOException {
+    try {
+      sendDataFrame(new byte[0x1000000]);
+      fail();
+    } catch (IOException e) {
+      assertEquals("FRAME_SIZE_ERROR max size is 16383: 16777216", 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);
@@ -323,4 +369,14 @@ public class Http20Draft09Test {
     new Http20Draft09.Writer(out, true).ping(ack, payload1, payload2);
     return out.toByteArray();
   }
+
+  private byte[] sendDataFrame(byte[] data) throws IOException {
+    return sendDataFrame(data, 0, data.length);
+  }
+
+  private byte[] sendDataFrame(byte[] data, int offset, int byteCount) throws IOException {
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+    new Http20Draft09.Writer(out, true).sendDataFrame(expectedStreamId, 0, data, offset, byteCount);
+    return out.toByteArray();
+  }
 }

okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/Spdy3Test.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2014 Square, Inc.
+ *
+ * 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.io.ByteArrayOutputStream;
+import java.io.IOException;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+public class Spdy3Test {
+  static final int expectedStreamId = 15;
+
+  @Test public void tooLargeDataFrame() throws IOException {
+    try {
+      sendDataFrame(new byte[0x1000000]);
+      fail();
+    } catch (IOException e) {
+      assertEquals("FRAME_TOO_LARGE max size is 16Mib: 16777216", e.getMessage());
+    }
+  }
+
+  private byte[] sendDataFrame(byte[] data) throws IOException {
+    return sendDataFrame(data, 0, data.length);
+  }
+
+  private byte[] sendDataFrame(byte[] data, int offset, int byteCount) throws IOException {
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+    new Spdy3.Writer(out, true).sendDataFrame(expectedStreamId, 0, data, offset, byteCount);
+    return out.toByteArray();
+  }
+}

okhttp-protocols/src/test/java/com/squareup/okhttp/internal/spdy/SpdyConnectionTest.java

@@ -24,7 +24,6 @@ import java.io.InputStream;
 import java.io.InterruptedIOException;
 import java.io.OutputStream;
 import java.util.Arrays;
-import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.junit.After;
@@ -1094,6 +1093,40 @@ public final class SpdyConnectionTest {
     }
   }
 
+  /**
+   * This tests that data frames are written in chunks limited by the
+   * SpdyDataOutputStream buffer size.  A side-effect is that this size
+   * prevents us from overrunning the max frame size of SPDY/3 or HTTP/2.
+   */
+  @Test public void spdyStreamOutputBufferSizeLimitsDataFrameLength() throws Exception {
+    MockSpdyPeer peer = new MockSpdyPeer(Variant.HTTP_20_DRAFT_09, false);
+
+    byte[] buff = new byte[SpdyStream.OUTPUT_BUFFER_SIZE * 2];
+    Arrays.fill(buff, (byte) '*');
+
+    // write the mocking script
+    peer.acceptFrame(); // SYN_STREAM
+    peer.sendFrame().synReply(false, 1, headerEntries("a", "android"));
+    peer.acceptFrame(); // DATA 1
+    peer.acceptFrame(); // DATA 2
+    peer.play();
+
+    // play it back
+    SpdyConnection connection = http2Connection(peer);
+    SpdyStream stream = connection.newStream(headerEntries("b", "banana"), true, true);
+    OutputStream out = stream.getOutputStream();
+    out.write(buff);
+    out.flush();
+    out.close();
+
+    MockSpdyPeer.InFrame synStream = peer.takeFrame();
+    assertEquals(TYPE_HEADERS, synStream.type);
+    MockSpdyPeer.InFrame data = peer.takeFrame();
+    assertEquals(SpdyStream.OUTPUT_BUFFER_SIZE, data.data.length);
+    data = peer.takeFrame();
+    assertEquals(SpdyStream.OUTPUT_BUFFER_SIZE, data.data.length);
+  }
+
   /** https://github.com/square/okhttp/issues/333 */
   @Test public void headerBlockHasTrailingCompressedBytes() throws Exception {
     // write the mocking script