diff options
| author | Neil Fuller <nfuller@google.com> | 2014-07-30 17:35:46 +0100 |
|---|---|---|
| committer | Neil Fuller <nfuller@google.com> | 2014-07-31 10:31:18 +0100 |
| commit | f6af62d5c9bb5e15649a80ebae973463e8e2dc46 (patch) | |
| tree | 3fc14991b693505f926e3a6da18e88d16f3eeac2 | |
| parent | 4909663c795d974d0d4b0e2d1ebd6e179486c897 (diff) | |
| download | android_external_okhttp-f6af62d5c9bb5e15649a80ebae973463e8e2dc46.tar.gz android_external_okhttp-f6af62d5c9bb5e15649a80ebae973463e8e2dc46.tar.bz2 android_external_okhttp-f6af62d5c9bb5e15649a80ebae973463e8e2dc46.zip | |
Fix transparent gzip for basic auth.
Externally reported Android bug:
https://code.google.com/p/android/issues/detail?id=74026
Thanks to mattpan91 for the report.
The issue was fixed in OkHttp 2.0. It has now been back-ported to 1.6.
This is a cherry pick from the okhttp_16 branch:
https://github.com/square/okhttp/commit/e8fee51087a062384f52e11400ff4a104a00a2b2
The merge was not entirely straightforward: Android is currently not quite
on 1.6; it is close and it was only the package for the files that differed
for existing files. There is an additional class (Job) that had to be patched
that the version Android has and 1.6 does not have. The class is probably not
used on Android.
The okhttp CTS tests pass, modulo some tests known to be flaky.
Bug: 16628050
Bug: https://code.google.com/p/android/issues/detail?id=74026
Change-Id: Ic179947f9f3664a4f2a7fcde435ec9fb7f1ae340
8 files changed, 280 insertions, 134 deletions
diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/RecordedResponse.java b/okhttp-tests/src/test/java/com/squareup/okhttp/RecordedResponse.java index 6628331..5b57baa 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/RecordedResponse.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/RecordedResponse.java @@ -77,7 +77,7 @@ public class RecordedResponse { * response for the original request. */ public RecordedResponse redirectedBy() { - Response redirectedBy = response.redirectedBy(); + Response redirectedBy = response.priorResponse(); assertNotNull(redirectedBy); assertNull(redirectedBy.body()); return new RecordedResponse(redirectedBy.request(), redirectedBy, null, null); diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/RecordingOkAuthenticator.java b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/RecordingOkAuthenticator.java index 636acbd..5d3020f 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/RecordingOkAuthenticator.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/RecordingOkAuthenticator.java @@ -23,28 +23,43 @@ import java.util.ArrayList; import java.util.List; public final class RecordingOkAuthenticator implements OkAuthenticator { - public final List<String> calls = new ArrayList<String>(); + public final List<URL> urls = new ArrayList<URL>(); + public final List<List<Challenge>> challengesList = new ArrayList<List<Challenge>>(); + public final List<Proxy> proxies = new ArrayList<Proxy>(); public final Credential credential; public RecordingOkAuthenticator(Credential credential) { this.credential = credential; } + public URL onlyUrl() { + if (urls.size() != 1) throw new IllegalStateException(); + return urls.get(0); + } + + public List<Challenge> onlyChallenge() { + if (challengesList.size() != 1) throw new IllegalStateException(); + return challengesList.get(0); + } + + public Proxy onlyProxy() { + if (proxies.size() != 1) throw new IllegalStateException(); + return proxies.get(0); + } + @Override public Credential authenticate(Proxy proxy, URL url, List<Challenge> challenges) throws IOException { - calls.add("authenticate" - + " proxy=" + proxy.type() - + " url=" + url - + " challenges=" + challenges); + urls.add(url); + challengesList.add(challenges); + proxies.add(proxy); return credential; } @Override public Credential authenticateProxy(Proxy proxy, URL url, List<Challenge> challenges) throws IOException { - calls.add("authenticateProxy" - + " proxy=" + proxy.type() - + " url=" + url - + " challenges=" + challenges); + urls.add(url); + challengesList.add(challenges); + proxies.add(proxy); return credential; } } diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java index 2280e83..f1cf9c8 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java @@ -18,6 +18,7 @@ package com.squareup.okhttp.internal.http; import com.squareup.okhttp.ConnectionPool; import com.squareup.okhttp.HttpResponseCache; +import com.squareup.okhttp.OkAuthenticator.Challenge; import com.squareup.okhttp.OkAuthenticator.Credential; import com.squareup.okhttp.OkHttpClient; import com.squareup.okhttp.Protocol; @@ -1674,6 +1675,39 @@ public final class URLConnectionTest { } } + /** https://code.google.com/p/android/issues/detail?id=74026 */ + @Test public void authenticateWithGetAndTransparentGzip() throws Exception { + MockResponse pleaseAuthenticate = new MockResponse().setResponseCode(401) + .addHeader("WWW-Authenticate: Basic realm=\"protected area\"") + .setBody("Please authenticate."); + // fail auth three times... + server.enqueue(pleaseAuthenticate); + server.enqueue(pleaseAuthenticate); + server.enqueue(pleaseAuthenticate); + // ...then succeed the fourth time + MockResponse successfulResponse = new MockResponse() + .addHeader("Content-Encoding", "gzip") + .setBody(gzip("Successful auth!".getBytes("UTF-8"))); + server.enqueue(successfulResponse); + server.play(); + + Authenticator.setDefault(new RecordingAuthenticator()); + connection = client.open(server.getUrl("/")); + assertEquals("Successful auth!", readAscii(connection.getInputStream(), Integer.MAX_VALUE)); + + // no authorization header for the first request... + RecordedRequest request = server.takeRequest(); + assertContainsNoneMatching(request.getHeaders(), "Authorization: Basic .*"); + + // ...but the three requests that follow requests include an authorization header + for (int i = 0; i < 3; i++) { + request = server.takeRequest(); + assertEquals("GET / HTTP/1.1", request.getRequestLine()); + assertContains(request.getHeaders(), + "Authorization: Basic " + RecordingAuthenticator.BASE_64_CREDENTIALS); + } + } + /** https://github.com/square/okhttp/issues/342 */ @Test public void authenticateRealmUppercase() throws Exception { server.enqueue(new MockResponse().setResponseCode(401) @@ -2691,11 +2725,10 @@ public final class URLConnectionTest { assertContains(server.takeRequest().getHeaders(), "Authorization: " + credential.getHeaderValue()); - assertEquals(1, authenticator.calls.size()); - String call = authenticator.calls.get(0); - assertTrue(call, call.contains("proxy=DIRECT")); - assertTrue(call, call.contains("url=" + server.getUrl("/private"))); - assertTrue(call, call.contains("challenges=[Basic realm=\"protected area\"]")); + assertEquals(Proxy.NO_PROXY, authenticator.onlyProxy()); + URL url = authenticator.onlyUrl(); + assertEquals("/private", url.getPath()); + assertEquals(Arrays.asList(new Challenge("Basic", "protected area")), authenticator.onlyChallenge()); } @Test public void npnSetsProtocolHeader_SPDY_3() throws Exception { diff --git a/okhttp/src/main/java/com/squareup/okhttp/Job.java b/okhttp/src/main/java/com/squareup/okhttp/Job.java index 64ce188..721acc8 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Job.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Job.java @@ -117,7 +117,7 @@ final class Job extends NamedRunnable { } // Create the initial HTTP engine. Retries and redirects need new engine for each attempt. - engine = new HttpEngine(client, request, false, null, null, null); + engine = new HttpEngine(client, request, false, null, null, null, null); while (true) { if (canceled) return null; @@ -150,7 +150,7 @@ final class Job extends NamedRunnable { engine.releaseConnection(); return response.newBuilder() .body(new RealResponseBody(response, engine.getResponseBody())) - .redirectedBy(redirectedBy) + .priorResponse(redirectedBy) .build(); } @@ -159,9 +159,9 @@ final class Job extends NamedRunnable { } Connection connection = engine.close(); - redirectedBy = response.newBuilder().redirectedBy(redirectedBy).build(); // Chained. + redirectedBy = response.newBuilder().priorResponse(redirectedBy).build(); // Chained. request = redirect; - engine = new HttpEngine(client, request, false, connection, null, null); + engine = new HttpEngine(client, request, false, connection, null, null, null); } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/Response.java b/okhttp/src/main/java/com/squareup/okhttp/Response.java index 13f9124..03c2d1f 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Response.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Response.java @@ -49,7 +49,9 @@ public final class Response { private final Handshake handshake; private final Headers headers; private final Body body; - private final Response redirectedBy; + private Response networkResponse; + private Response cacheResponse; + private final Response priorResponse; private volatile ParsedHeaders parsedHeaders; // Lazily initialized. private volatile CacheControl cacheControl; // Lazily initialized. @@ -60,7 +62,9 @@ public final class Response { this.handshake = builder.handshake; this.headers = builder.headers.build(); this.body = builder.body; - this.redirectedBy = builder.redirectedBy; + this.networkResponse = builder.networkResponse; + this.cacheResponse = builder.cacheResponse; + this.priorResponse = builder.priorResponse; } /** @@ -134,8 +138,27 @@ public final class Response { * of the returned response should not be read because it has already been * consumed by the redirecting client. */ - public Response redirectedBy() { - return redirectedBy; + public Response priorResponse() { + return priorResponse; + } + + /** + * Returns the raw response received from the network. Will be null if this + * response didn't use the network, such as when the response is fully cached. + * The body of the returned response should not be read. + */ + public Response networkResponse() { + return networkResponse; + } + + /** + * Returns the raw response received from the cache. Will be null if this + * response didn't use the cache. For conditional get requests the cache + * response and network response may both be non-null. The body of the + * returned response should not be read. + */ + public Response cacheResponse() { + return cacheResponse; } // TODO: move out of public API @@ -361,7 +384,9 @@ public final class Response { private Handshake handshake; private Headers.Builder headers; private Body body; - private Response redirectedBy; + private Response networkResponse; + private Response cacheResponse; + private Response priorResponse; public Builder() { headers = new Headers.Builder(); @@ -373,7 +398,9 @@ public final class Response { this.handshake = response.handshake; this.headers = response.headers.newBuilder(); this.body = response.body; - this.redirectedBy = response.redirectedBy; + this.networkResponse = response.networkResponse; + this.cacheResponse = response.cacheResponse; + this.priorResponse = response.priorResponse; } public Builder request(Request request) { @@ -439,8 +466,32 @@ public final class Response { return header(OkHeaders.RESPONSE_SOURCE, responseSource + " " + statusLine.code()); } - public Builder redirectedBy(Response redirectedBy) { - this.redirectedBy = redirectedBy; + public Builder networkResponse(Response networkResponse) { + if (networkResponse != null) checkSupportResponse("networkResponse", networkResponse); + this.networkResponse = networkResponse; + return this; + } + + public Builder cacheResponse(Response cacheResponse) { + if (cacheResponse != null) checkSupportResponse("cacheResponse", cacheResponse); + this.cacheResponse = cacheResponse; + return this; + } + + private void checkSupportResponse(String name, Response response) { + if (response.body != null) { + throw new IllegalArgumentException(name + ".body != null"); + } else if (response.networkResponse != null) { + throw new IllegalArgumentException(name + ".networkResponse != null"); + } else if (response.cacheResponse != null) { + throw new IllegalArgumentException(name + ".cacheResponse != null"); + } else if (response.priorResponse != null) { + throw new IllegalArgumentException(name + ".priorResponse != null"); + } + } + + public Builder priorResponse(Response priorResponse) { + this.priorResponse = priorResponse; return this; } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/CacheStrategy.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/CacheStrategy.java index 7cc7e21..75e13d9 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/CacheStrategy.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/CacheStrategy.java @@ -46,14 +46,18 @@ public final class CacheStrategy { } } - public final Request request; - public final Response response; + /** The request to send on the network, or null if this call doesn't use the network. */ + public final Request networkRequest; + + /** The cached response to return or validate; or null if this call doesn't use a cache. */ + public final Response cacheResponse; + public final ResponseSource source; private CacheStrategy( - Request request, Response response, ResponseSource source) { - this.request = request; - this.response = response; + Request networkRequest, Response cacheResponse, ResponseSource source) { + this.networkRequest = networkRequest; + this.cacheResponse = cacheResponse; this.source = source; } @@ -167,12 +171,12 @@ public final class CacheStrategy { if (candidate.source != ResponseSource.CACHE && request.cacheControl().onlyIfCached()) { // We're forbidden from using the network, but the cache is insufficient. Response noneResponse = new Response.Builder() - .request(candidate.request) + .request(candidate.networkRequest) .statusLine(GATEWAY_TIMEOUT_STATUS_LINE) .setResponseSource(ResponseSource.NONE) .body(EMPTY_BODY) .build(); - return new CacheStrategy(candidate.request, noneResponse, ResponseSource.NONE); + return new CacheStrategy(null, noneResponse, ResponseSource.NONE); } return candidate; @@ -182,19 +186,19 @@ public final class CacheStrategy { private CacheStrategy getCandidate() { // No cached response. if (cacheResponse == null) { - return new CacheStrategy(request, cacheResponse, ResponseSource.NETWORK); + return new CacheStrategy(request, null, ResponseSource.NETWORK); } // Drop the cached response if it's missing a required handshake. if (request.isHttps() && cacheResponse.handshake() == null) { - return new CacheStrategy(request, cacheResponse, ResponseSource.NETWORK); + return new CacheStrategy(request, null, ResponseSource.NETWORK); } // If this response shouldn't have been stored, it should never be used // as a response source. This check should be redundant as long as the // persistence store is well-behaved and the rules are constant. if (!isCacheable(cacheResponse, request)) { - return new CacheStrategy(request, cacheResponse, ResponseSource.NETWORK); + return new CacheStrategy(request, null, ResponseSource.NETWORK); } CacheControl requestCaching = request.cacheControl(); @@ -230,7 +234,7 @@ public final class CacheStrategy { if (ageMillis > oneDayMillis && isFreshnessLifetimeHeuristic()) { builder.addHeader("Warning", "113 HttpURLConnection \"Heuristic expiration\""); } - return new CacheStrategy(request, builder.build(), ResponseSource.CACHE); + return new CacheStrategy(null, builder.build(), ResponseSource.CACHE); } Request.Builder conditionalRequestBuilder = request.newBuilder(); @@ -246,10 +250,9 @@ public final class CacheStrategy { } Request conditionalRequest = conditionalRequestBuilder.build(); - ResponseSource responseSource = hasConditions(conditionalRequest) - ? ResponseSource.CONDITIONAL_CACHE - : ResponseSource.NETWORK; - return new CacheStrategy(conditionalRequest, cacheResponse, responseSource); + return hasConditions(conditionalRequest) + ? new CacheStrategy(conditionalRequest, cacheResponse, ResponseSource.CONDITIONAL_CACHE) + : new CacheStrategy(conditionalRequest, null, ResponseSource.NETWORK); } /** 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 38e26c3..f00fbe7 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 @@ -78,6 +78,7 @@ public class HttpEngine { private Connection connection; private RouteSelector routeSelector; private Route route; + private final Response priorResponse; private Transport transport; @@ -98,29 +99,51 @@ public class HttpEngine { */ public final boolean bufferRequestBody; - private Request originalRequest; - private Request request; + /** + * The original application-provided request. Never modified by OkHttp. When + * follow-up requests are necessary, they are derived from this request. + */ + private final Request userRequest; + + /** + * The request to send on the network, or null for no network request. This is + * derived from the user request, and customized to support OkHttp features + * like compression and caching. + */ + private Request networkRequest; + + /** + * The cached response, or null if the cache doesn't exist or cannot be used + * for this request. Conditional caching means this may be non-null even when + * the network request is non-null. Never modified by OkHttp. + */ + private Response cacheResponse; + + /** + * The response read from the network. Null if the network response hasn't + * been read yet, or if the network is not used. Never modified by OkHttp. + */ + private Response networkResponse; + + /** + * The user-visible response. This is derived from either the network + * response, cache response, or both. It is customized to support OkHttp + * features like compression and caching. + */ + private Response userResponse; + private Sink requestBodyOut; private BufferedSink bufferedRequestBody; private ResponseSource responseSource; /** Null until a response is received from the network or the cache. */ - private Response response; private Source responseTransferSource; private Source responseBody; private InputStream responseBodyBytes; - /** - * The cache response currently being validated on a conditional get. Null - * if the cached response doesn't exist or doesn't need validation. If the - * conditional get succeeds, these will be used for the response. If it fails, - * it will be set to null. - */ - private Response validatingResponse; - /** The cache request currently being populated from a network response. */ - private CacheRequest cacheRequest; + private CacheRequest storeRequest; /** * @param request the HTTP request without a body. The body must be @@ -134,14 +157,15 @@ public class HttpEngine { * recover from a failure. */ public HttpEngine(OkHttpClient client, Request request, boolean bufferRequestBody, - Connection connection, RouteSelector routeSelector, RetryableSink requestBodyOut) { + Connection connection, RouteSelector routeSelector, RetryableSink requestBodyOut, + Response priorResponse) { this.client = client; - this.originalRequest = request; - this.request = request; + this.userRequest = request; this.bufferRequestBody = bufferRequestBody; this.connection = connection; this.routeSelector = routeSelector; this.requestBodyOut = requestBodyOut; + this.priorResponse = priorResponse; if (connection != null) { connection.setOwner(this); @@ -160,33 +184,31 @@ public class HttpEngine { if (responseSource != null) return; // Already sent. if (transport != null) throw new IllegalStateException(); - prepareRawRequestHeaders(); - OkResponseCache responseCache = client.getOkResponseCache(); + Request request = networkRequest(userRequest); - Response cacheResponse = responseCache != null + OkResponseCache responseCache = client.getOkResponseCache(); + Response cacheCandidate = responseCache != null ? responseCache.get(request) : null; long now = System.currentTimeMillis(); - CacheStrategy cacheStrategy = new CacheStrategy.Factory(now, request, cacheResponse).get(); + CacheStrategy cacheStrategy = new CacheStrategy.Factory(now, request, cacheCandidate).get(); responseSource = cacheStrategy.source; - request = cacheStrategy.request; + networkRequest = cacheStrategy.networkRequest; + cacheResponse = cacheStrategy.cacheResponse; if (responseCache != null) { responseCache.trackResponse(responseSource); } - if (responseSource != ResponseSource.NETWORK) { - validatingResponse = cacheStrategy.response; - } - - if (cacheResponse != null && !responseSource.usesCache()) { - closeQuietly(cacheResponse.body()); // We don't need this cached response. Close it. + if (cacheCandidate != null + && (responseSource == ResponseSource.NONE || cacheResponse == null)) { + closeQuietly(cacheCandidate.body()); // The cache candidate wasn't applicable. Close it. } - if (responseSource.requiresConnection()) { + if (networkRequest != null) { // Open a connection unless we inherited one from a redirect. if (connection == null) { - connect(); + connect(networkRequest); } // Blow up if we aren't the current owner of the connection. @@ -208,21 +230,25 @@ public class HttpEngine { } // No need for the network! Promote the cached response immediately. - this.response = validatingResponse; - if (validatingResponse.body() != null) { - initContentStream(validatingResponse.body().source()); + this.userResponse = cacheResponse.newBuilder() + .request(userRequest) + .priorResponse(stripBody(priorResponse)) + .cacheResponse(stripBody(cacheResponse)) + .build(); + if (userResponse.body() != null) { + initContentStream(userResponse.body().source()); } } } - private Response cacheableResponse() { - // Use an unreadable response body when offering the response to the cache. - // The cache isn't allowed to consume the response body bytes! - return response.newBuilder().body(null).build(); + private static Response stripBody(Response response) { + return response != null && response.body() != null + ? response.newBuilder().body(null).build() + : response; } /** Connect to the origin server either directly or via a proxy. */ - private void connect() throws IOException { + private void connect(Request request) throws IOException { if (connection != null) throw new IllegalStateException(); if (routeSelector == null) { @@ -267,7 +293,7 @@ public class HttpEngine { } boolean hasRequestBody() { - return HttpMethod.hasRequestBody(request.method()); + return HttpMethod.hasRequestBody(userRequest.method()); } /** Returns the request body or null if this request doesn't have a body. */ @@ -286,26 +312,22 @@ public class HttpEngine { } public final boolean hasResponse() { - return response != null; - } - - public final ResponseSource responseSource() { - return responseSource; + return userResponse != null; } public final Request getRequest() { - return request; + return userRequest; } /** Returns the engine's response. */ // TODO: the returned body will always be null. public final Response getResponse() { - if (response == null) throw new IllegalStateException(); - return response; + if (userResponse == null) throw new IllegalStateException(); + return userResponse; } public final Source getResponseBody() { - if (response == null) throw new IllegalStateException(); + if (userResponse == null) throw new IllegalStateException(); return responseBody; } @@ -341,8 +363,8 @@ public class HttpEngine { Connection connection = close(); // For failure recovery, use the same route selector with a new connection. - return new HttpEngine(client, originalRequest, bufferRequestBody, connection, routeSelector, - (RetryableSink) requestBodyOut); + return new HttpEngine(client, userRequest, bufferRequestBody, connection, routeSelector, + (RetryableSink) requestBodyOut, priorResponse); } private boolean isRecoverable(IOException e) { @@ -367,13 +389,13 @@ public class HttpEngine { if (responseCache == null) return; // Should we cache this response for this request? - if (!CacheStrategy.isCacheable(response, request)) { - responseCache.maybeRemove(request); + if (!CacheStrategy.isCacheable(userResponse, networkRequest)) { + responseCache.maybeRemove(networkRequest); return; } // Offer this request to the cache. - cacheRequest = responseCache.put(cacheableResponse()); + storeRequest = responseCache.put(stripBody(userResponse)); } /** @@ -448,8 +470,8 @@ public class HttpEngine { */ private void initContentStream(Source transferSource) throws IOException { responseTransferSource = transferSource; - if (transparentGzip && "gzip".equalsIgnoreCase(response.header("Content-Encoding"))) { - response = response.newBuilder() + if (transparentGzip && "gzip".equalsIgnoreCase(userResponse.header("Content-Encoding"))) { + userResponse = userResponse.newBuilder() .removeHeader("Content-Encoding") .removeHeader("Content-Length") .build(); @@ -465,11 +487,11 @@ public class HttpEngine { */ public final boolean hasResponseBody() { // HEAD requests never yield a body regardless of the response headers. - if (request.method().equals("HEAD")) { + if (userRequest.method().equals("HEAD")) { return false; } - int responseCode = response.code(); + int responseCode = userResponse.code(); if ((responseCode < HTTP_CONTINUE || responseCode >= 200) && responseCode != HTTP_NO_CONTENT && responseCode != HTTP_NOT_MODIFIED) { @@ -479,8 +501,8 @@ public class HttpEngine { // If the Content-Length or Transfer-Encoding headers disagree with the // response code, the response is malformed. For best compatibility, we // honor the headers. - if (OkHeaders.contentLength(response) != -1 - || "chunked".equalsIgnoreCase(response.header("Transfer-Encoding"))) { + if (OkHeaders.contentLength(networkResponse) != -1 + || "chunked".equalsIgnoreCase(networkResponse.header("Transfer-Encoding"))) { return true; } @@ -493,7 +515,7 @@ public class HttpEngine { * <p>This client doesn't specify a default {@code Accept} header because it * doesn't know what content types the application is interested in. */ - private void prepareRawRequestHeaders() throws IOException { + private Request networkRequest(Request request) throws IOException { Request.Builder result = request.newBuilder(); if (request.getUserAgent() == null) { @@ -531,7 +553,7 @@ public class HttpEngine { OkHeaders.addCookies(result, cookies); } - request = result.build(); + return result.build(); } public static String getDefaultUserAgent() { @@ -550,9 +572,15 @@ public class HttpEngine { * headers and starts reading the HTTP response body if it exists. */ public final void readResponse() throws IOException { - if (response != null) return; - if (responseSource == null) throw new IllegalStateException("call sendRequest() first!"); - if (!responseSource.requiresConnection()) return; + if (userResponse != null) { + return; // Already ready. + } + if (networkRequest == null && cacheResponse == null) { + throw new IllegalStateException("call sendRequest() first!"); + } + if (networkRequest == null) { + return; // No network response to read. + } // Flush the request body if there's data outstanding. if (bufferedRequestBody != null && bufferedRequestBody.buffer().size() > 0) { @@ -560,14 +588,15 @@ public class HttpEngine { } if (sentRequestMillis == -1) { - if (OkHeaders.contentLength(request) == -1 && requestBodyOut instanceof RetryableSink) { + if (OkHeaders.contentLength(networkRequest) == -1 + && requestBodyOut instanceof RetryableSink) { // We might not learn the Content-Length until the request body has been buffered. long contentLength = ((RetryableSink) requestBodyOut).contentLength(); - request = request.newBuilder() + networkRequest = networkRequest.newBuilder() .header("Content-Length", Long.toString(contentLength)) .build(); } - transport.writeRequestHeaders(request); + transport.writeRequestHeaders(networkRequest); } if (requestBodyOut != null) { @@ -584,68 +613,79 @@ public class HttpEngine { transport.flushRequest(); - response = transport.readResponseHeaders() - .request(request) + networkResponse = transport.readResponseHeaders() + .request(networkRequest) .handshake(connection.getHandshake()) .header(OkHeaders.SENT_MILLIS, Long.toString(sentRequestMillis)) .header(OkHeaders.RECEIVED_MILLIS, Long.toString(System.currentTimeMillis())) .setResponseSource(responseSource) .build(); - connection.setHttpMinorVersion(response.httpMinorVersion()); - receiveHeaders(response.headers()); + connection.setHttpMinorVersion(networkResponse.httpMinorVersion()); + receiveHeaders(networkResponse.headers()); if (responseSource == ResponseSource.CONDITIONAL_CACHE) { - if (validatingResponse.validate(response)) { + if (cacheResponse.validate(networkResponse)) { + userResponse = cacheResponse.newBuilder() + .request(userRequest) + .priorResponse(stripBody(priorResponse)) + .headers(combine(cacheResponse.headers(), networkResponse.headers())) + .cacheResponse(stripBody(cacheResponse)) + .networkResponse(stripBody(networkResponse)) + .build(); transport.emptyTransferStream(); releaseConnection(); - response = combine(validatingResponse, response); // Update the cache after combining headers but before stripping the // Content-Encoding header (as performed by initContentStream()). OkResponseCache responseCache = client.getOkResponseCache(); responseCache.trackConditionalCacheHit(); - responseCache.update(validatingResponse, cacheableResponse()); - - if (validatingResponse.body() != null) { - initContentStream(validatingResponse.body().source()); + responseCache.update(cacheResponse, stripBody(userResponse)); + if (cacheResponse.body() != null) { + initContentStream(cacheResponse.body().source()); } + return; } else { - closeQuietly(validatingResponse.body()); + closeQuietly(cacheResponse.body()); } } + userResponse = networkResponse.newBuilder() + .request(userRequest) + .priorResponse(stripBody(priorResponse)) + .cacheResponse(stripBody(cacheResponse)) + .networkResponse(stripBody(networkResponse)) + .build(); + if (!hasResponseBody()) { // Don't call initContentStream() when the response doesn't have any content. - responseTransferSource = transport.getTransferStream(cacheRequest); + responseTransferSource = transport.getTransferStream(storeRequest); responseBody = responseTransferSource; return; } maybeCache(); - initContentStream(transport.getTransferStream(cacheRequest)); + initContentStream(transport.getTransferStream(storeRequest)); } /** * Combines cached headers with a network headers as defined by RFC 2616, * 13.5.3. */ - private static Response combine(Response cached, Response network) throws IOException { + private static Headers combine(Headers cachedHeaders, Headers networkHeaders) throws IOException { Headers.Builder result = new Headers.Builder(); - Headers cachedHeaders = cached.headers(); for (int i = 0; i < cachedHeaders.size(); i++) { String fieldName = cachedHeaders.name(i); String value = cachedHeaders.value(i); if ("Warning".equals(fieldName) && value.startsWith("1")) { continue; // drop 100-level freshness warnings } - if (!isEndToEnd(fieldName) || network.header(fieldName) == null) { + if (!isEndToEnd(fieldName) || networkHeaders.get(fieldName) == null) { result.add(fieldName, value); } } - Headers networkHeaders = network.headers(); for (int i = 0; i < networkHeaders.size(); i++) { String fieldName = networkHeaders.name(i); if (isEndToEnd(fieldName)) { @@ -653,7 +693,7 @@ public class HttpEngine { } } - return cached.newBuilder().headers(result.build()).build(); + return result.build(); } /** @@ -672,20 +712,20 @@ public class HttpEngine { } private TunnelRequest getTunnelConfig() { - if (!request.isHttps()) return null; + if (!userRequest.isHttps()) return null; - String userAgent = request.getUserAgent(); + String userAgent = userRequest.getUserAgent(); if (userAgent == null) userAgent = getDefaultUserAgent(); - URL url = request.url(); + URL url = userRequest.url(); return new TunnelRequest(url.getHost(), getEffectivePort(url), userAgent, - request.getProxyAuthorization()); + userRequest.getProxyAuthorization()); } public void receiveHeaders(Headers headers) throws IOException { CookieHandler cookieHandler = client.getCookieHandler(); if (cookieHandler != null) { - cookieHandler.put(request.uri(), OkHeaders.toMultimap(headers, null)); + cookieHandler.put(userRequest.uri(), OkHeaders.toMultimap(headers, null)); } } } 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 d43af99..899d914 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 @@ -267,7 +267,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection { throw new ProtocolException(method + " does not support writing"); } } - httpEngine = newHttpEngine(method, null, null); + httpEngine = newHttpEngine(method, null, null, null); } catch (IOException e) { httpEngineFailure = e; throw e; @@ -275,7 +275,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection { } private HttpEngine newHttpEngine(String method, Connection connection, - RetryableSink requestBody) { + RetryableSink requestBody, Response priorResponse) { Request.Builder builder = new Request.Builder() .url(getURL()) .method(method, null /* No body; that's passed separately. */); @@ -303,7 +303,8 @@ public class HttpURLConnectionImpl extends HttpURLConnection { engineClient = client.clone().setOkResponseCache(null); } - return new HttpEngine(engineClient, request, bufferRequestBody, connection, null, requestBody); + return new HttpEngine(engineClient, request, bufferRequestBody, connection, null, requestBody, + priorResponse); } /** @@ -323,6 +324,8 @@ public class HttpURLConnectionImpl extends HttpURLConnection { continue; } + Response response = httpEngine.getResponse(); + Retry retry = processResponseHeaders(); if (retry == Retry.NONE) { httpEngine.releaseConnection(); @@ -355,7 +358,8 @@ public class HttpURLConnectionImpl extends HttpURLConnection { } Connection connection = httpEngine.close(); - httpEngine = newHttpEngine(retryMethod, connection, (RetryableSink) requestBody); + httpEngine = newHttpEngine(retryMethod, connection, (RetryableSink) requestBody, + response); } } |
