diff --git a/okhttp/src/main/java/com/squareup/okhttp/Connection.java b/okhttp/src/main/java/com/squareup/okhttp/Connection.java index 26625b15e..a9d41822a 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Connection.java +++ b/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 diff --git a/okhttp/src/main/java/com/squareup/okhttp/Job.java b/okhttp/src/main/java/com/squareup/okhttp/Job.java index 1f1e6fd05..8af097356 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Job.java +++ b/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(); diff --git a/okhttp/src/main/java/com/squareup/okhttp/Request.java b/okhttp/src/main/java/com/squareup/okhttp/Request.java index 780b889ac..6eb8d8393 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Request.java +++ b/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; 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 db3f4e440..7120b299a 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 @@ -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; 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 81e1216c1..1db10cc54 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 @@ -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 { diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java index cb13100a2..e089d91a7 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java +++ b/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: + * + * + *

Warning: 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; - } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/Policy.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/Policy.java deleted file mode 100644 index eabf484e4..000000000 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/Policy.java +++ /dev/null @@ -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); -} 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 61810e118..978764bd0 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 @@ -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(); } diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java index 2873d463f..125c34cd0 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java +++ b/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 {