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 6d1cd8318..e3e1a4f93 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,35 @@ import java.util.ArrayList; import java.util.List; public final class RecordingOkAuthenticator implements OkAuthenticator { - public final List calls = new ArrayList(); + public final List responses = new ArrayList(); + public final List proxies = new ArrayList(); public final String credential; public RecordingOkAuthenticator(String credential) { this.credential = credential; } + public Response onlyResponse() { + if (responses.size() != 1) throw new IllegalStateException(); + return responses.get(0); + } + + public Proxy onlyProxy() { + if (proxies.size() != 1) throw new IllegalStateException(); + return proxies.get(0); + } + @Override public Request authenticate(Proxy proxy, Response response) { - calls.add("authenticate" - + " proxy=" + proxy.type() - + " url=" + response.request().url() - + " challenges=" + response.challenges()); + responses.add(response); + proxies.add(proxy); return response.request().newBuilder() .addHeader("Authorization", credential) .build(); } @Override public Request authenticateProxy(Proxy proxy, Response response) { - calls.add("authenticateProxy" - + " proxy=" + proxy.type() - + " url=" + response.request().url() - + " challenges=" + response.challenges()); + responses.add(response); + proxies.add(proxy); return response.request().newBuilder() .addHeader("Proxy-Authorization", credential) .build(); 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 9769b68f8..ccb654267 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,10 +18,12 @@ package com.squareup.okhttp.internal.http; import com.squareup.okhttp.AbstractResponseCache; import com.squareup.okhttp.Cache; +import com.squareup.okhttp.Challenge; import com.squareup.okhttp.ConnectionPool; import com.squareup.okhttp.Credentials; import com.squareup.okhttp.OkHttpClient; import com.squareup.okhttp.Protocol; +import com.squareup.okhttp.Response; import com.squareup.okhttp.internal.Internal; import com.squareup.okhttp.internal.RecordingAuthenticator; import com.squareup.okhttp.internal.RecordingHostnameVerifier; @@ -2711,11 +2713,10 @@ public final class URLConnectionTest { assertContains(server.takeRequest().getHeaders(), "Authorization: " + credential); - 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()); + Response response = authenticator.onlyResponse(); + assertEquals("/private", response.request().url().getPath()); + assertEquals(Arrays.asList(new Challenge("Basic", "protected area")), response.challenges()); } @Test public void customTokenAuthenticator() throws Exception { @@ -2733,11 +2734,31 @@ public final class URLConnectionTest { assertContainsNoneMatching(server.takeRequest().getHeaders(), "Authorization: .*"); assertContains(server.takeRequest().getHeaders(), "Authorization: oauthed abc123"); - 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=[Bearer realm=\"oauthed\"]")); + Response response = authenticator.onlyResponse(); + assertEquals("/private", response.request().url().getPath()); + assertEquals(Arrays.asList(new Challenge("Bearer", "oauthed")), response.challenges()); + } + + @Test public void authenticateCallsTrackedAsRedirects() throws Exception { + server.enqueue(new MockResponse() + .setResponseCode(302) + .addHeader("Location: /b")); + server.enqueue(new MockResponse() + .setResponseCode(401) + .addHeader("WWW-Authenticate: Basic realm=\"protected area\"")); + server.enqueue(new MockResponse().setBody("c")); + server.play(); + + RecordingOkAuthenticator authenticator = new RecordingOkAuthenticator( + Credentials.basic("jesse", "peanutbutter")); + client.setAuthenticator(authenticator); + assertContent("c", client.open(server.getUrl("/a"))); + + Response challengeResponse = authenticator.responses.get(0); + assertEquals("/b", challengeResponse.request().url().getPath()); + + Response redirectedBy = challengeResponse.redirectedBy(); + assertEquals("/a", redirectedBy.request().url().getPath()); } @Test public void setsNegotiatedProtocolHeader_SPDY_3() throws Exception { diff --git a/okhttp/src/main/java/com/squareup/okhttp/Call.java b/okhttp/src/main/java/com/squareup/okhttp/Call.java index de14eaa43..f45636b0c 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Call.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Call.java @@ -169,8 +169,6 @@ public final class Call { * call was canceled. */ private Response getResponse() throws IOException { - Response redirectedBy = null; - // Copy body metadata to the appropriate request headers. Request.Body body = request.body(); RetryableSink requestBodyOut = null; @@ -196,7 +194,7 @@ public final class Call { } // Create the initial HTTP engine. Retries and redirects need new engine for each attempt. - engine = new HttpEngine(client, request, false, null, null, requestBodyOut); + engine = new HttpEngine(client, request, false, null, null, requestBodyOut, null); while (true) { if (canceled) return null; @@ -228,7 +226,6 @@ public final class Call { engine.releaseConnection(); return response.newBuilder() .body(new RealResponseBody(response, engine.getResponseBody())) - .redirectedBy(redirectedBy) .build(); } @@ -241,9 +238,8 @@ public final class Call { } Connection connection = engine.close(); - redirectedBy = response.newBuilder().redirectedBy(redirectedBy).build(); // Chained. request = followUp; - engine = new HttpEngine(client, request, false, connection, null, null); + engine = new HttpEngine(client, request, false, connection, null, null, response); } } 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 a3f4db744..f90e8d780 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 @@ -96,6 +96,7 @@ public final class HttpEngine { private Connection connection; private RouteSelector routeSelector; private Route route; + private final Response redirectedBy; private Transport transport; @@ -152,7 +153,8 @@ public final 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 redirectedBy) { this.client = client; this.originalRequest = request; this.request = request; @@ -160,6 +162,7 @@ public final class HttpEngine { this.connection = connection; this.routeSelector = routeSelector; this.requestBodyOut = requestBodyOut; + this.redirectedBy = redirectedBy; if (connection != null) { Internal.instance.setOwner(connection, this); @@ -364,7 +367,7 @@ public final class HttpEngine { // For failure recovery, use the same route selector with a new connection. return new HttpEngine(client, originalRequest, bufferRequestBody, connection, routeSelector, - (RetryableSink) requestBodyOut); + (RetryableSink) requestBodyOut, redirectedBy); } public HttpEngine recover(IOException e) { @@ -632,6 +635,7 @@ public final class HttpEngine { transport.flushRequest(); response = transport.readResponseHeaders() + .redirectedBy(redirectedBy) .request(request) .handshake(connection.getHandshake()) .header(OkHeaders.SENT_MILLIS, Long.toString(sentRequestMillis)) diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/huc/HttpURLConnectionImpl.java b/okhttp/src/main/java/com/squareup/okhttp/internal/huc/HttpURLConnectionImpl.java index 78cf9d052..197189595 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/huc/HttpURLConnectionImpl.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/huc/HttpURLConnectionImpl.java @@ -272,7 +272,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; @@ -280,7 +280,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection { } private HttpEngine newHttpEngine(String method, Connection connection, - RetryableSink requestBody) { + RetryableSink requestBody, Response redirectedBy) { Request.Builder builder = new Request.Builder() .url(getURL()) .method(method, null /* No body; that's passed separately. */); @@ -308,7 +308,8 @@ public class HttpURLConnectionImpl extends HttpURLConnection { engineClient = client.clone().setCache(null); } - return new HttpEngine(engineClient, request, bufferRequestBody, connection, null, requestBody); + return new HttpEngine(engineClient, request, bufferRequestBody, connection, null, requestBody, + redirectedBy); } /** @@ -361,7 +362,8 @@ public class HttpURLConnectionImpl extends HttpURLConnection { } Connection connection = httpEngine.close(); - httpEngine = newHttpEngine(followUp.method(), connection, (RetryableSink) requestBody); + httpEngine = newHttpEngine(followUp.method(), connection, (RetryableSink) requestBody, + response); } }