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 8670f5ba7..ab90db69d 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 @@ -52,7 +52,17 @@ public interface FrameWriter extends Closeable { throws IOException; void headers(int streamId, List
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. */ 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 a416d10b2..4589788c1 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 @@ -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); } 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 236b30ecf..4d0bf8455 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 @@ -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); 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 dbfb2dc3a..ba14dbe45 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 @@ -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. */ 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 c54bd165c..31d08421a 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 @@ -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(); + } } 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 new file mode 100644 index 000000000..83d3fd2a4 --- /dev/null +++ b/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(); + } +} 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 3e140c31e..313bcbc74 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 @@ -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