Merge pull request #804 from square/jwilson_0505_redirected_in_auth

Track redirectedBy in HttpEngine.

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<String> calls = new ArrayList<String>();
+  public final List<Response> responses = new ArrayList<Response>();
+  public final List<Proxy> proxies = new ArrayList<Proxy>();
   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();

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 {

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

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

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