Kill the Policy interface.

This interface was necessary while cleaning up the delicate
intertwined relationship between HttpURLConnection and HttpEngine.
With this change HttpEngine no longer has a reverse dependency
on HttpURLConnection.

okhttp/src/main/java/com/squareup/okhttp/Connection.java

@@ -92,9 +92,8 @@ public final class Connection implements Closeable {
 
   public void connect(int connectTimeout, int readTimeout, TunnelRequest tunnelRequest)
       throws IOException {
-    if (connected) {
-      throw new IllegalStateException("already connected");
-    }
+    if (connected) throw new IllegalStateException("already connected");
+
     connected = true;
     socket = (route.proxy.type() != Proxy.Type.HTTP) ? new Socket(route.proxy) : new Socket();
     Platform.get().connectSocket(socket, route.inetSocketAddress, connectTimeout);
@@ -270,10 +269,6 @@ public final class Connection implements Closeable {
     return spdyConnection != null;
   }
 
-  public SpdyConnection getSpdyConnection() {
-    return spdyConnection;
-  }
-
   /**
    * Returns the minor HTTP version that should be used for future requests on
    * this connection. Either 0 for HTTP/1.0, or 1 for HTTP/1.1. The default

okhttp/src/main/java/com/squareup/okhttp/Job.java

@@ -17,8 +17,6 @@ package com.squareup.okhttp;
 
 import com.squareup.okhttp.internal.http.HttpAuthenticator;
 import com.squareup.okhttp.internal.http.HttpEngine;
-import com.squareup.okhttp.internal.http.HttpTransport;
-import com.squareup.okhttp.internal.http.Policy;
 import java.io.IOException;
 import java.net.ProtocolException;
 import java.net.Proxy;
@@ -30,10 +28,10 @@ import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_MOVED
 import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_MULT_CHOICE;
 import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_PROXY_AUTH;
 import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_SEE_OTHER;
-import static com.squareup.okhttp.internal.http.StatusLine.HTTP_TEMP_REDIRECT;
 import static com.squareup.okhttp.internal.http.HttpURLConnectionImpl.HTTP_UNAUTHORIZED;
+import static com.squareup.okhttp.internal.http.StatusLine.HTTP_TEMP_REDIRECT;
 
-final class Job implements Runnable, Policy {
+final class Job implements Runnable {
   private final Dispatcher dispatcher;
   private final OkHttpClient client;
   private final Response.Receiver responseReceiver;
@@ -49,18 +47,6 @@ final class Job implements Runnable, Policy {
     this.responseReceiver = responseReceiver;
   }
 
-  @Override public int getChunkLength() {
-    return request.body().contentLength() == -1 ? HttpTransport.DEFAULT_CHUNK_LENGTH : -1;
-  }
-
-  @Override public long getFixedContentLength() {
-    return request.body().contentLength();
-  }
-
-  @Override public void setSelectedProxy(Proxy proxy) {
-    // Do nothing.
-  }
-
   Object tag() {
     return request.tag();
   }
@@ -90,9 +76,20 @@ final class Job implements Runnable, Policy {
       if (body != null) {
         MediaType contentType = body.contentType();
         if (contentType == null) throw new IllegalStateException("contentType == null");
-        if (request.header("Content-Type") == null) {
-          request = request.newBuilder().header("Content-Type", contentType.toString()).build();
+
+        Request.Builder requestBuilder = request.newBuilder();
+        requestBuilder.header("Content-Type", contentType.toString());
+
+        long contentLength = body.contentLength();
+        if (contentLength != -1) {
+          requestBuilder.setContentLength(contentLength);
+          requestBuilder.removeHeader("Transfer-Encoding");
+        } else {
+          requestBuilder.setChunked();
+          requestBuilder.removeHeader("Content-Length");
         }
+
+        request = requestBuilder.build();
       }
 
       HttpEngine engine = newEngine(connection);
@@ -128,7 +125,7 @@ final class Job implements Runnable, Policy {
   }
 
   HttpEngine newEngine(Connection connection) throws IOException {
-    return new HttpEngine(client, this, request, connection, null);
+    return new HttpEngine(client, request, false, connection, null);
   }
 
   /**
@@ -139,8 +136,8 @@ final class Job implements Runnable, Policy {
    */
   private Request processResponse(HttpEngine engine, Response response) throws IOException {
     Request request = response.request();
-    Proxy selectedProxy = engine.getConnection() != null
-        ? engine.getConnection().getRoute().getProxy()
+    Proxy selectedProxy = engine.getRoute() != null
+        ? engine.getRoute().getProxy()
         : client.getProxy();
     int responseCode = response.code();
 

okhttp/src/main/java/com/squareup/okhttp/Request.java

@@ -286,7 +286,7 @@ public final class Request {
           hasAuthorization = true;
         } else if ("Content-Length".equalsIgnoreCase(fieldName)) {
           try {
-            contentLength = Integer.parseInt(value);
+            contentLength = Long.parseLong(value);
           } catch (NumberFormatException ignored) {
           }
         } else if ("Transfer-Encoding".equalsIgnoreCase(fieldName)) {
@@ -444,6 +444,10 @@ public final class Request {
       return this;
     }
 
+    public void removeHeader(String name) {
+      headers.removeAll(name);
+    }
+
     public Builder setChunked() {
       headers.set("Transfer-Encoding", "chunked");
       return this;

okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java

@@ -25,6 +25,7 @@ import com.squareup.okhttp.OkResponseCache;
 import com.squareup.okhttp.Request;
 import com.squareup.okhttp.Response;
 import com.squareup.okhttp.ResponseSource;
+import com.squareup.okhttp.Route;
 import com.squareup.okhttp.TunnelRequest;
 import com.squareup.okhttp.internal.Dns;
 import java.io.IOException;
@@ -69,11 +70,11 @@ import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
  * required, use {@link #automaticallyReleaseConnectionToPool()}.
  */
 public class HttpEngine {
-  final Policy policy;
   final OkHttpClient client;
 
   Connection connection;
   RouteSelector routeSelector;
+  private Route route;
 
   private Transport transport;
 
@@ -86,6 +87,14 @@ public class HttpEngine {
    */
   private boolean transparentGzip;
 
+  /**
+   * True if the request body must be completely buffered before transmission;
+   * false if it can be streamed. Buffering has two advantages: we don't need
+   * the content-length in advance and we can retransmit if necessary. The
+   * upside of streaming is that we can save memory.
+   */
+  public final boolean bufferRequestBody;
+
   private Request request;
   private OutputStream requestBodyOut;
 
@@ -124,12 +133,13 @@ public class HttpEngine {
    *     redirect. This engine assumes ownership of the connection and must
    *     release it when it is unneeded.
    */
-  public HttpEngine(OkHttpClient client, Policy policy, Request request,
+  public HttpEngine(OkHttpClient client, Request request, boolean bufferRequestBody,
       Connection connection, RetryableOutputStream requestBodyOut) throws IOException {
     this.client = client;
-    this.policy = policy;
     this.request = request;
+    this.bufferRequestBody = bufferRequestBody;
     this.connection = connection;
+    this.route = connection != null ? connection.getRoute() : null;
     this.requestBodyOut = requestBodyOut;
   }
 
@@ -234,8 +244,7 @@ public class HttpEngine {
       connection.updateReadTimeout(client.getReadTimeout());
     }
 
-    // Update the policy to tell 'em which proxy we ended up going with.
-    policy.setSelectedProxy(connection.getRoute().getProxy());
+    route = connection.getRoute();
   }
 
   /**
@@ -291,6 +300,14 @@ public class HttpEngine {
     return connection;
   }
 
+  /**
+   * Returns the route used to retrieve the response. Null if we haven't
+   * connected yet, or if no connection was necessary.
+   */
+  public Route getRoute() {
+    return route;
+  }
+
   private void maybeCache() throws IOException {
     OkResponseCache responseCache = client.getOkResponseCache();
     if (responseCache == null) return;

okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpTransport.java

@@ -62,54 +62,44 @@ public final class HttpTransport implements Transport {
   }
 
   public Request prepareRequest(Request request) {
-    if (!httpEngine.hasRequestBody()) return request;
-
-    if (!request.isChunked()
-        && httpEngine.policy.getChunkLength() > 0
-        && httpEngine.connection.getHttpMinorVersion() != 0) {
-      return request.newBuilder().setChunked().build();
-    }
-
-    long fixedContentLength = httpEngine.policy.getFixedContentLength();
-    if (fixedContentLength != -1) {
-      return request.newBuilder().setContentLength(fixedContentLength).build();
-    }
-
     return request;
   }
 
   @Override public OutputStream createRequestBody(Request request) throws IOException {
-    // Stream a request body of unknown length.
-    if (request.isChunked()) {
-      int chunkLength = httpEngine.policy.getChunkLength();
-      if (chunkLength == -1) chunkLength = DEFAULT_CHUNK_LENGTH;
-      writeRequestHeaders(request);
-      return new ChunkedOutputStream(requestOut, chunkLength);
-    }
-
-    // Stream a request body of a known length.
-    long fixedContentLength = httpEngine.policy.getFixedContentLength();
-    if (fixedContentLength != -1) {
-      writeRequestHeaders(request);
-      return new FixedLengthOutputStream(requestOut, fixedContentLength);
-    }
-
     long contentLength = request.getContentLength();
-    if (contentLength > Integer.MAX_VALUE) {
-      throw new IllegalArgumentException("Use setFixedLengthStreamingMode() or "
-          + "setChunkedStreamingMode() for requests larger than 2 GiB.");
+
+    if (httpEngine.bufferRequestBody) {
+      if (contentLength > Integer.MAX_VALUE) {
+        throw new IllegalStateException("Use setFixedLengthStreamingMode() or "
+            + "setChunkedStreamingMode() for requests larger than 2 GiB.");
+      }
+
+      if (contentLength != -1) {
+        // Buffer a request body of a known length.
+        writeRequestHeaders(request);
+        return new RetryableOutputStream((int) contentLength);
+      } else {
+        // Buffer a request body of an unknown length. Don't write request
+        // headers until the entire body is ready; otherwise we can't set the
+        // Content-Length header correctly.
+        return new RetryableOutputStream();
+      }
     }
 
-    // Buffer a request body of a known length.
-    if (contentLength != -1) {
+    if (request.isChunked()) {
+      // Stream a request body of unknown length.
       writeRequestHeaders(request);
-      return new RetryableOutputStream((int) contentLength);
+      return new ChunkedOutputStream(requestOut, DEFAULT_CHUNK_LENGTH);
     }
 
-    // Buffer a request body of an unknown length. Don't write request
-    // headers until the entire body is ready; otherwise we can't set the
-    // Content-Length header correctly.
-    return new RetryableOutputStream();
+    if (contentLength != -1) {
+      // Stream a request body of a known length.
+      writeRequestHeaders(request);
+      return new FixedLengthOutputStream(requestOut, contentLength);
+    }
+
+    throw new IllegalStateException(
+        "Cannot stream a request body without chunked encoding or a known content length!");
   }
 
   @Override public void flushRequest() throws IOException {

okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java

@@ -21,6 +21,7 @@ import com.squareup.okhttp.Connection;
 import com.squareup.okhttp.OkHttpClient;
 import com.squareup.okhttp.Request;
 import com.squareup.okhttp.Response;
+import com.squareup.okhttp.Route;
 import com.squareup.okhttp.internal.Platform;
 import com.squareup.okhttp.internal.Util;
 import java.io.FileNotFoundException;
@@ -59,7 +60,7 @@ import static com.squareup.okhttp.internal.http.StatusLine.HTTP_TEMP_REDIRECT;
  * attempted. Once a connection has been attempted, certain properties (request
  * header fields, request method, etc.) are immutable.
  */
-public class HttpURLConnectionImpl extends HttpURLConnection implements Policy {
+public class HttpURLConnectionImpl extends HttpURLConnection {
 
   /**
    * How many redirects should we follow? Chrome follows 21; Firefox, curl,
@@ -76,7 +77,12 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy {
   private int redirectionCount;
   protected IOException httpEngineFailure;
   protected HttpEngine httpEngine;
-  private Proxy selectedProxy;
+
+  /**
+   * The most recently attempted route. This will be null if we haven't sent a
+   * request yet, or if the response comes from a cache.
+   */
+  private Route route;
 
   public HttpURLConnectionImpl(URL url, OkHttpClient client) {
     super(url);
@@ -278,6 +284,18 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy {
     for (int i = 0; i < headers.length(); i++) {
       builder.addHeader(headers.getFieldName(i), headers.getValue(i));
     }
+
+    boolean bufferRequestBody;
+    if (fixedContentLength != -1) {
+      bufferRequestBody = false;
+      builder.header("Content-Length", Long.toString(fixedContentLength));
+    } else if (chunkLength > 0) {
+      bufferRequestBody = false;
+      builder.header("Transfer-Encoding", "chunked");
+    } else {
+      bufferRequestBody = true;
+    }
+
     Request request = builder.build();
 
     // If we're currently not using caches, make sure the engine's client doesn't have one.
@@ -286,7 +304,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy {
       engineClient = client.clone().setOkResponseCache(null);
     }
 
-    return new HttpEngine(engineClient, this, request, connection, requestBody);
+    return new HttpEngine(engineClient, request, bufferRequestBody, connection, requestBody);
   }
 
   /**
@@ -351,6 +369,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy {
   private boolean execute(boolean readResponse) throws IOException {
     try {
       httpEngine.sendRequest();
+      route = httpEngine.getRoute();
       if (readResponse) {
         httpEngine.readResponse();
       }
@@ -474,29 +493,21 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy {
     }
   }
 
-  /** @see java.net.HttpURLConnection#setFixedLengthStreamingMode(int) */
-  @Override public final long getFixedContentLength() {
-    return fixedContentLength;
-  }
-
-  @Override public final int getChunkLength() {
-    return chunkLength;
-  }
-
+  /**
+   * Returns true if either:
+   * <ul>
+   *   <li>A specific proxy was explicitly configured for this connection.
+   *   <li>The response has already been retrieved, and a proxy was {@link
+   *       java.net.ProxySelector selected} in order to get it.
+   * </ul>
+   *
+   * <p><strong>Warning:</strong> This method may return false before attempting
+   * to connect and true afterwards.
+   */
   @Override public final boolean usingProxy() {
-    if (selectedProxy != null) {
-      return isValidNonDirectProxy(selectedProxy);
-    }
-
-    // This behavior is a bit odd (but is probably justified by the
-    // oddness of the APIs involved). Before a connection is established,
-    // this method will return true only if this connection was explicitly
-    // opened with a Proxy. We don't attempt to query the ProxySelector
-    // at all.
-    return isValidNonDirectProxy(client.getProxy());
-  }
-
-  private static boolean isValidNonDirectProxy(Proxy proxy) {
+    Proxy proxy = route != null
+        ? route.getProxy()
+        : client.getProxy();
     return proxy != null && proxy.type() != Proxy.Type.DIRECT;
   }
 
@@ -592,8 +603,4 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy {
     this.fixedContentLength = contentLength;
     super.fixedContentLength = (int) Math.min(contentLength, Integer.MAX_VALUE);
   }
-
-  @Override public final void setSelectedProxy(Proxy proxy) {
-    this.selectedProxy = proxy;
-  }
 }

okhttp/src/main/java/com/squareup/okhttp/internal/http/Policy.java

@@ -1,32 +0,0 @@
-/*
- * Copyright (C) 2013 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.http;
-
-import java.net.Proxy;
-
-public interface Policy {
-  /** @see java.net.HttpURLConnection#setChunkedStreamingMode(int) */
-  int getChunkLength();
-
-  /** @see java.net.HttpURLConnection#setFixedLengthStreamingMode(int) */
-  long getFixedContentLength();
-
-  /**
-   * Sets the current proxy that this connection is using.
-   * @see java.net.HttpURLConnection#usingProxy
-   */
-  void setSelectedProxy(Proxy proxy);
-}

okhttp/src/main/java/com/squareup/okhttp/internal/http/SpdyTransport.java

@@ -50,18 +50,13 @@ public final class SpdyTransport implements Transport {
         .header(":version", RequestLine.version(httpEngine.connection.getHttpMinorVersion()))
         .header(":host", HttpEngine.hostHeader(request.url()));
 
-    if (httpEngine.hasRequestBody()) {
-      long fixedContentLength = httpEngine.policy.getFixedContentLength();
-      if (fixedContentLength != -1) {
-        builder.setContentLength(fixedContentLength);
-      }
-    }
+    builder.removeHeader("Transfer-Encoding"); // SPDY doesn't use chunked encoding.
 
     return builder.build();
   }
 
   @Override public OutputStream createRequestBody(Request request) throws IOException {
-    // TODO: if we aren't streaming up to the server, we should buffer the whole request
+    // TODO: if bufferRequestBody is set, we must buffer the whole request
     writeRequestHeaders(request);
     return stream.getOutputStream();
   }

okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java

@@ -613,6 +613,7 @@ public final class URLConnectionTest {
     URL url = new URL("http://android.com/foo");
     HttpURLConnection connection = proxyConfig.connect(server, client, url);
     assertContent("this response comes via a proxy", connection);
+    assertTrue(connection.usingProxy());
 
     RecordedRequest request = server.takeRequest();
     assertEquals("GET http://android.com/foo HTTP/1.1", request.getRequestLine());
@@ -1129,26 +1130,21 @@ public final class URLConnectionTest {
     assertEquals(1, server.takeRequest().getSequenceNumber()); // Connection is pooled!
   }
 
-  /**
-   * Obnoxiously test that the chunk sizes transmitted exactly equal the
-   * requested data+chunk header size. Although setChunkedStreamingMode()
-   * isn't specific about whether the size applies to the data or the
-   * complete chunk, the RI interprets it as a complete chunk.
-   */
   @Test public void setChunkedStreamingMode() throws IOException, InterruptedException {
     server.enqueue(new MockResponse());
     server.play();
 
+    String body = "ABCDEFGHIJKLMNOPQ";
     HttpURLConnection urlConnection = client.open(server.getUrl("/"));
-    urlConnection.setChunkedStreamingMode(8);
+    urlConnection.setChunkedStreamingMode(0); // OkHttp does not honor specific chunk sizes.
     urlConnection.setDoOutput(true);
     OutputStream outputStream = urlConnection.getOutputStream();
-    outputStream.write("ABCDEFGHIJKLMNOPQ".getBytes("US-ASCII"));
+    outputStream.write(body.getBytes("US-ASCII"));
     assertEquals(200, urlConnection.getResponseCode());
 
     RecordedRequest request = server.takeRequest();
-    assertEquals("ABCDEFGHIJKLMNOPQ", new String(request.getBody(), "US-ASCII"));
-    assertEquals(Arrays.asList(3, 3, 3, 3, 3, 2), request.getChunkSizes());
+    assertEquals(body, new String(request.getBody(), "US-ASCII"));
+    assertEquals(Arrays.asList(body.length()), request.getChunkSizes());
   }
 
   @Test public void authenticateWithFixedLengthStreaming() throws Exception {