diff --git a/okhttp/src/main/java/com/squareup/okhttp/Request.java b/okhttp/src/main/java/com/squareup/okhttp/Request.java index 6eb8d8393..b392196ad 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Request.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Request.java @@ -444,8 +444,9 @@ public final class Request { return this; } - public void removeHeader(String name) { + public Builder removeHeader(String name) { headers.removeAll(name); + return this; } public Builder setChunked() { diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java index 7120b299a..841e98a23 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java @@ -182,7 +182,6 @@ public class HttpEngine { } transport = (Transport) connection.newTransport(this); - request = transport.prepareRequest(request); // Create a request body if we don't have one already. We'll already have // one if we're retrying a failed POST. diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpTransport.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpTransport.java index 1db10cc54..b7543102b 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpTransport.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpTransport.java @@ -61,10 +61,6 @@ public final class HttpTransport implements Transport { this.socketIn = inputStream; } - public Request prepareRequest(Request request) { - return request; - } - @Override public OutputStream createRequestBody(Request request) throws IOException { long contentLength = request.getContentLength(); 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 978764bd0..266512558 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 @@ -42,19 +42,6 @@ public final class SpdyTransport implements Transport { this.spdyConnection = spdyConnection; } - @Override public Request prepareRequest(Request request) { - Request.Builder builder = request.newBuilder() - .header(":method", request.method()) - .header(":scheme", request.url().getProtocol()) - .header(":path", RequestLine.requestPath(request.url())) - .header(":version", RequestLine.version(httpEngine.connection.getHttpMinorVersion())) - .header(":host", HttpEngine.hostHeader(request.url())); - - builder.removeHeader("Transfer-Encoding"); // SPDY doesn't use chunked encoding. - - return builder.build(); - } - @Override public OutputStream createRequestBody(Request request) throws IOException { // TODO: if bufferRequestBody is set, we must buffer the whole request writeRequestHeaders(request); @@ -67,8 +54,9 @@ public final class SpdyTransport implements Transport { httpEngine.writingRequestHeaders(); boolean hasRequestBody = httpEngine.hasRequestBody(); boolean hasResponseBody = true; + String version = RequestLine.version(httpEngine.connection.getHttpMinorVersion()); stream = spdyConnection.newStream( - writeNameValueBlock(request.getHeaders()), hasRequestBody, hasResponseBody); + writeNameValueBlock(request, version), hasRequestBody, hasResponseBody); stream.setReadTimeout(httpEngine.client.getReadTimeout()); } @@ -89,12 +77,23 @@ 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(Headers headers) { + public static List writeNameValueBlock(Request request, String version) { + List result = new ArrayList(request.headerCount() + 10); + result.add(":method"); + result.add(request.method()); + result.add(":path"); + result.add(RequestLine.requestPath(request.url())); + result.add(":version"); + result.add(version); + result.add(":host"); + result.add(HttpEngine.hostHeader(request.url())); + result.add(":scheme"); + result.add(request.url().getProtocol()); + Set names = new LinkedHashSet(); - List result = new ArrayList(headers.length() * 2); - for (int i = 0; i < headers.length(); i++) { - String name = headers.getFieldName(i).toLowerCase(Locale.US); - String value = headers.getValue(i); + for (int i = 0; i < request.headerCount(); i++) { + String name = request.headerName(i).toLowerCase(Locale.US); + String value = request.headerValue(i); // Drop headers that are forbidden when layering HTTP over SPDY. if (name.equals("connection") @@ -105,6 +104,15 @@ public final class SpdyTransport implements Transport { 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(":scheme")) { + continue; + } + // If we haven't seen this name before, add the pair to the end of the list... if (names.add(name)) { result.add(name); diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/Transport.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/Transport.java index cbe12fe35..59e986f1a 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/Transport.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/Transport.java @@ -24,13 +24,6 @@ import java.io.OutputStream; import java.net.CacheRequest; interface Transport { - /** - * Returns a request equivalent to {@code request} but with transport-specific - * changes. For example, this may set a Transfer-Encoding header if it is - * required but not present for the current transport. - */ - Request prepareRequest(Request request); - /** * Returns an output stream where the request body can be written. The * returned stream will of one of two types: 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 1dce00985..dc08b652a 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 @@ -53,13 +53,20 @@ public final class HeadersTest { } @Test public void toNameValueBlock() { - Headers.Builder builder = new Headers.Builder(); - builder.add("cache-control", "no-cache, no-store"); - builder.add("set-cookie", "Cookie1"); - builder.add("set-cookie", "Cookie2"); - builder.add(":status", "200 OK"); - List nameValueBlock = SpdyTransport.writeNameValueBlock(builder.build()); + Request request = new Request.Builder() + .url("http://square.com/") + .header("cache-control", "no-cache, no-store") + .addHeader("set-cookie", "Cookie1") + .addHeader("set-cookie", "Cookie2") + .header(":status", "200 OK") + .build(); + List nameValueBlock = SpdyTransport.writeNameValueBlock(request, "HTTP/1.1"); List expected = Arrays.asList( + ":method", "GET", + ":path", "/", + ":version", "HTTP/1.1", + ":host", "square.com", + ":scheme", "http", "cache-control", "no-cache, no-store", "set-cookie", "Cookie1\u0000Cookie2", ":status", "200 OK"); @@ -67,9 +74,17 @@ public final class HeadersTest { } @Test public void toNameValueBlockDropsForbiddenHeaders() { - Headers.Builder builder = new Headers.Builder(); - builder.add("Connection", "close"); - builder.add("Transfer-Encoding", "chunked"); - assertEquals(Arrays.asList(), SpdyTransport.writeNameValueBlock(builder.build())); + Request request = new Request.Builder() + .url("http://square.com/") + .header("Connection", "close") + .header("Transfer-Encoding", "chunked") + .build(); + List expected = Arrays.asList( + ":method", "GET", + ":path", "/", + ":version", "HTTP/1.1", + ":host", "square.com", + ":scheme", "http"); + assertEquals(expected, SpdyTransport.writeNameValueBlock(request, "HTTP/1.1")); } }