Don't let transports rewrite the request.

Now that we're preparing Content-Length and Transfer-Encoding
before passing a request to the HTTP engine, we have no strict
need to rewrite requests in the transport.

The nice upside of this change is that the transport becomes
even less obvious when it's in place.

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() {

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.

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();
 

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<String> writeNameValueBlock(Headers headers) {
+  public static List<String> writeNameValueBlock(Request request, String version) {
+    List<String> result = new ArrayList<String>(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<String> names = new LinkedHashSet<String>();
-    List<String> result = new ArrayList<String>(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);

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:

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<String> 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<String> nameValueBlock = SpdyTransport.writeNameValueBlock(request, "HTTP/1.1");
     List<String> 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.<String>asList(), SpdyTransport.writeNameValueBlock(builder.build()));
+    Request request = new Request.Builder()
+        .url("http://square.com/")
+        .header("Connection", "close")
+        .header("Transfer-Encoding", "chunked")
+        .build();
+    List<String> expected = Arrays.asList(
+        ":method", "GET",
+        ":path", "/",
+        ":version", "HTTP/1.1",
+        ":host", "square.com",
+        ":scheme", "http");
+    assertEquals(expected, SpdyTransport.writeNameValueBlock(request, "HTTP/1.1"));
   }
 }