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 69bc2f2ba..a3884cb32 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 @@ -31,6 +31,11 @@ import java.util.List; * http://tools.ietf.org/html/draft-ietf-httpbis-http2-09 */ public final class Http20Draft09 implements Variant { + + @Override public String getProtocol() { + return "HTTP-draft-09/2.0"; + } + private static final byte[] CONNECTION_HEADER; static { try { @@ -158,7 +163,6 @@ public final class Http20Draft09 implements Variant { if ((flags & FLAG_END_HEADERS) != 0) { hpackReader.emitReferenceSet(); - // not filtering out illegal headers on read. List nameValueBlock = hpackReader.getAndReset(); // TODO: Concat multi-value headers with 0x0, except COOKIE, which uses 0x3B, 0x20. // http://tools.ietf.org/html/draft-ietf-httpbis-http2-09#section-8.1.3 @@ -322,20 +326,6 @@ public final class Http20Draft09 implements Variant { private void headers(boolean outFinished, int streamId, int priority, List nameValueBlock) throws IOException { hpackBuffer.reset(); - for (int i = 0, size = nameValueBlock.size(); i < size; i += 2) { - String name = nameValueBlock.get(i); - // our SpdyTransport.writeNameValueBlock hard-codes :host - // TODO: is :authority literally the same value as :host? - // https://github.com/http2/http2-spec/issues/334 - if (":host".equals(name)) { - nameValueBlock.set(i, ":authority"); - } else if (shouldDropHeader(name)) { - //TODO: Avoid creating headers like these. - nameValueBlock.remove(i); - nameValueBlock.remove(i); - i -= 2; - } - } hpackWriter.writeHeaders(nameValueBlock); int type = TYPE_HEADERS; // TODO: implement CONTINUATION @@ -423,19 +413,4 @@ public final class Http20Draft09 implements Variant { private static IOException ioException(String message, Object... args) throws IOException { throw new IOException(String.format(message, args)); } - - /** - * Leniently drop as opposed to throwing malformed. - * http://tools.ietf.org/html/draft-ietf-httpbis-http2-09#section-8.1.3 - */ - private static boolean shouldDropHeader(String name) { - return name.equals("connection") - || name.equals("host") // host is not supported in http/2 - || name.equals("keep-alive") - || name.equals("proxy-connection") - || name.equals("te") - || name.equals("transfer-encoding") - || name.equals("encoding") - || name.equals("upgrade"); - } } 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 5d9a49b3a..1f9391f44 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 @@ -29,6 +29,11 @@ import java.util.List; import java.util.zip.Deflater; final class Spdy3 implements Variant { + + @Override public String getProtocol() { + return "spdy/3"; + } + static final int TYPE_DATA = 0x0; static final int TYPE_SYN_STREAM = 0x1; static final int TYPE_SYN_REPLY = 0x2; 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 ca5656b62..15986a35d 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 @@ -103,6 +103,15 @@ public final class SpdyConnection implements Closeable { new Thread(new Reader(), "Spdy Reader " + hostName).start(); } + /** + * The protocol name, like {@code spdy/3} or {@code HTTP-draft-09/2.0}. + * + * @see com.squareup.okhttp.internal.spdy.Variant#getProtocol() + */ + public String getProtocol() { + return variant.getProtocol(); + } + /** * Returns the number of {@link SpdyStream#isOpen() open streams} on this * connection. diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Variant.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Variant.java index 72f8c91d2..3ff3e0b2f 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Variant.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/spdy/Variant.java @@ -23,6 +23,9 @@ interface Variant { Variant SPDY3 = new Spdy3(); Variant HTTP_20_DRAFT_09 = new Http20Draft09(); + /** The protocol name, like {@code spdy/3} or {@code HTTP-draft-09/2.0}. */ + String getProtocol(); + /** * @param client true if this is the HTTP client's reader, reading frames from * a peer SPDY or HTTP/2 server. diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/SpdyTransport.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/SpdyTransport.java index 3a372e3dc..19dba82b3 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/SpdyTransport.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/SpdyTransport.java @@ -57,7 +57,8 @@ public final class SpdyTransport implements Transport { boolean hasResponseBody = true; String version = RequestLine.version(httpEngine.connection.getHttpMinorVersion()); stream = spdyConnection.newStream( - writeNameValueBlock(request, version), hasRequestBody, hasResponseBody); + writeNameValueBlock(request, spdyConnection.getProtocol(), version), hasRequestBody, + hasResponseBody); stream.setReadTimeout(httpEngine.client.getReadTimeout()); } @@ -70,7 +71,7 @@ public final class SpdyTransport implements Transport { } @Override public Response.Builder readResponseHeaders() throws IOException { - return readNameValueBlock(stream.getResponseHeaders()); + return readNameValueBlock(stream.getResponseHeaders(), spdyConnection.getProtocol()); } /** @@ -78,7 +79,7 @@ public final class SpdyTransport implements Transport { * Names are all lower case. No names are repeated. If any name has multiple * values, they are concatenated using "\0" as a delimiter. */ - public static List writeNameValueBlock(Request request, String version) { + public static List writeNameValueBlock(Request request, String protocol, String version) { Headers headers = request.headers(); List result = new ArrayList(headers.size() + 10); result.add(":method"); @@ -87,7 +88,13 @@ public final class SpdyTransport implements Transport { result.add(RequestLine.requestPath(request.url())); result.add(":version"); result.add(version); - result.add(":host"); + if (protocol.equals("spdy/3")) { + result.add(":host"); + } else if (protocol.equals("HTTP-draft-09/2.0")) { + result.add(":authority"); + } else { + throw new AssertionError(); + } result.add(HttpEngine.hostHeader(request.url())); result.add(":scheme"); result.add(request.url().getProtocol()); @@ -98,19 +105,14 @@ public final class SpdyTransport implements Transport { String value = headers.value(i); // Drop headers that are forbidden when layering HTTP over SPDY. - if (name.equals("connection") - || name.equals("host") - || name.equals("keep-alive") - || name.equals("proxy-connection") - || name.equals("transfer-encoding")) { - continue; - } + if (isProhibitedHeader(protocol, name)) continue; // They shouldn't be set, but if they are, drop them. We've already written them! if (name.equals(":method") || name.equals(":path") || name.equals(":version") || name.equals(":host") + || name.equals(":authority") || name.equals(":scheme")) { continue; } @@ -134,7 +136,7 @@ public final class SpdyTransport implements Transport { } /** Returns headers for a name value block containing a SPDY response. */ - public static Response.Builder readNameValueBlock(List nameValueBlock) + public static Response.Builder readNameValueBlock(List nameValueBlock, String protocol) throws IOException { if (nameValueBlock.size() % 2 != 0) { throw new IllegalArgumentException("Unexpected name value block: " + nameValueBlock); @@ -143,7 +145,7 @@ public final class SpdyTransport implements Transport { String version = null; Headers.Builder headersBuilder = new Headers.Builder(); - headersBuilder.set(OkHeaders.SELECTED_TRANSPORT, "spdy/3"); + headersBuilder.set(OkHeaders.SELECTED_TRANSPORT, protocol); for (int i = 0; i < nameValueBlock.size(); i += 2) { String name = nameValueBlock.get(i); String values = nameValueBlock.get(i + 1); @@ -157,7 +159,7 @@ public final class SpdyTransport implements Transport { status = value; } else if (":version".equals(name)) { version = value; - } else { + } else if (!isProhibitedHeader(protocol, name)) { // Don't write forbidden headers! headersBuilder.add(name, value); } start = end + 1; @@ -190,4 +192,34 @@ public final class SpdyTransport implements Transport { } return true; } + + /** When true, this header should not be emitted or consumed. */ + private static boolean isProhibitedHeader(String protocol, String name) { + boolean prohibited = false; + if (protocol.equals("spdy/3")) { + // http://www.chromium.org/spdy/spdy-protocol/spdy-protocol-draft3#TOC-3.2.1-Request + if (name.equals("connection") + || name.equals("host") + || name.equals("keep-alive") + || name.equals("proxy-connection") + || name.equals("transfer-encoding")) { + prohibited = true; + } + } else if (protocol.equals("HTTP-draft-09/2.0")) { + // http://tools.ietf.org/html/draft-ietf-httpbis-http2-09#section-8.1.3 + if (name.equals("connection") + || name.equals("host") + || name.equals("keep-alive") + || name.equals("proxy-connection") + || name.equals("te") + || name.equals("transfer-encoding") + || name.equals("encoding") + || name.equals("upgrade")) { + prohibited = true; + } + } else { + throw new AssertionError(); + } + return prohibited; + } } diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java index ab001e3e8..172d9369d 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java @@ -34,7 +34,8 @@ public final class HeadersTest { ":status", "200 OK", ":version", "HTTP/1.1"); Request request = new Request.Builder().url("http://square.com/").build(); - Response response = SpdyTransport.readNameValueBlock(nameValueBlock).request(request).build(); + Response response = + SpdyTransport.readNameValueBlock(nameValueBlock, "spdy/3").request(request).build(); Headers headers = response.headers(); assertEquals(4, headers.size()); assertEquals("HTTP/1.1 200 OK", response.statusLine()); @@ -53,6 +54,34 @@ public final class HeadersTest { assertNull(headers.get(":version")); } + @Test public void readNameValueBlockDropsForbiddenHeadersSpdy3() throws IOException { + List nameValueBlock = Arrays.asList( + ":status", "200 OK", + ":version", "HTTP/1.1", + "connection", "close"); + Request request = new Request.Builder().url("http://square.com/").build(); + Response response = + SpdyTransport.readNameValueBlock(nameValueBlock, "spdy/3").request(request).build(); + Headers headers = response.headers(); + assertEquals(1, headers.size()); + assertEquals(OkHeaders.SELECTED_TRANSPORT, headers.name(0)); + assertEquals("spdy/3", headers.value(0));; + } + + @Test public void readNameValueBlockDropsForbiddenHeadersHttp2() throws IOException { + List nameValueBlock = Arrays.asList( + ":status", "200 OK", + ":version", "HTTP/1.1", + "connection", "close"); + Request request = new Request.Builder().url("http://square.com/").build(); + Response response = + SpdyTransport.readNameValueBlock(nameValueBlock, "HTTP-draft-09/2.0").request(request).build(); + Headers headers = response.headers(); + assertEquals(1, headers.size()); + assertEquals(OkHeaders.SELECTED_TRANSPORT, headers.name(0)); + assertEquals("HTTP-draft-09/2.0", headers.value(0));; + } + @Test public void toNameValueBlock() { Request request = new Request.Builder() .url("http://square.com/") @@ -61,7 +90,7 @@ public final class HeadersTest { .addHeader("set-cookie", "Cookie2") .header(":status", "200 OK") .build(); - List nameValueBlock = SpdyTransport.writeNameValueBlock(request, "HTTP/1.1"); + List nameValueBlock = SpdyTransport.writeNameValueBlock(request, "spdy/3", "HTTP/1.1"); List expected = Arrays.asList( ":method", "GET", ":path", "/", @@ -74,7 +103,7 @@ public final class HeadersTest { assertEquals(expected, nameValueBlock); } - @Test public void toNameValueBlockDropsForbiddenHeaders() { + @Test public void toNameValueBlockDropsForbiddenHeadersSpdy3() { Request request = new Request.Builder() .url("http://square.com/") .header("Connection", "close") @@ -86,6 +115,22 @@ public final class HeadersTest { ":version", "HTTP/1.1", ":host", "square.com", ":scheme", "http"); - assertEquals(expected, SpdyTransport.writeNameValueBlock(request, "HTTP/1.1")); + assertEquals(expected, SpdyTransport.writeNameValueBlock(request, "spdy/3", "HTTP/1.1")); + } + + @Test public void toNameValueBlockDropsForbiddenHeadersHttp2() { + Request request = new Request.Builder() + .url("http://square.com/") + .header("Connection", "upgrade") + .header("Upgrade", "websocket") + .build(); + List expected = Arrays.asList( + ":method", "GET", + ":path", "/", + ":version", "HTTP/1.1", + ":authority", "square.com", + ":scheme", "http"); + assertEquals(expected, + SpdyTransport.writeNameValueBlock(request, "HTTP-draft-09/2.0", "HTTP/1.1")); } }