Handle 408 responses by retrying the request

okhttp-android-support/src/test/java/okhttp3/internal/huc/ResponseCacheTest.java

@@ -178,6 +178,8 @@ private HttpURLConnection openConnection(URL url) {
   }
 
   private void assertCached(boolean shouldPut, int responseCode) throws Exception {
+    int expectedResponseCode = responseCode;
+
     server = new MockWebServer();
     MockResponse mockResponse = new MockResponse()
         .addHeader("Last-Modified: " + formatDate(-1, TimeUnit.HOURS))
@@ -191,11 +193,22 @@ private void assertCached(boolean shouldPut, int responseCode) throws Exception
       mockResponse.addHeader("WWW-Authenticate: Basic realm=\"protected area\"");
     }
     server.enqueue(mockResponse);
+
+    if (responseCode == HttpURLConnection.HTTP_CLIENT_TIMEOUT) {
+      // 408's are a bit of an outlier because we may repeat the request if we encounter this
+      // response code. In this scenario, there are 2 responses: the initial 408 and then the 200
+      // because of the retry. We just want to ensure the initial 408 isn't cached.
+      expectedResponseCode = 200;
+      server.enqueue(new MockResponse()
+          .setHeader("Cache-Control", "no-store")
+          .setBody("FGHIJ"));
+    }
+
     server.start();
 
     URL url = server.url("/").url();
     HttpURLConnection connection = openConnection(url);
-    assertEquals(responseCode, connection.getResponseCode());
+    assertEquals(expectedResponseCode, connection.getResponseCode());
 
     // Exhaust the content stream.
     readAscii(connection);

okhttp-tests/src/test/java/okhttp3/CacheTest.java

@@ -153,6 +153,8 @@ public final class CacheTest {
   }
 
   private void assertCached(boolean shouldPut, int responseCode) throws Exception {
+    int expectedResponseCode = responseCode;
+
     server = new MockWebServer();
     MockResponse mockResponse = new MockResponse()
         .addHeader("Last-Modified: " + formatDate(-1, TimeUnit.HOURS))
@@ -169,13 +171,24 @@ private void assertCached(boolean shouldPut, int responseCode) throws Exception
       mockResponse.setBody(""); // We forbid bodies for 204 and 205.
     }
     server.enqueue(mockResponse);
+
+    if (responseCode == HttpURLConnection.HTTP_CLIENT_TIMEOUT) {
+      // 408's are a bit of an outlier because we may repeat the request if we encounter this
+      // response code. In this scenario, there are 2 responses: the initial 408 and then the 200
+      // because of the retry. We just want to ensure the initial 408 isn't cached.
+      expectedResponseCode = 200;
+      server.enqueue(new MockResponse()
+          .setHeader("Cache-Control", "no-store")
+          .setBody("FGHIJ"));
+    }
+
     server.start();
 
     Request request = new Request.Builder()
         .url(server.url("/"))
         .build();
     Response response = client.newCall(request).execute();
-    assertEquals(responseCode, response.code());
+    assertEquals(expectedResponseCode, response.code());
 
     // Exhaust the content stream.
     response.body().string();

okhttp-tests/src/test/java/okhttp3/CallTest.java

@@ -1400,6 +1400,41 @@ private void postBodyRetransmittedAfterAuthorizationFail(String body) throws Exc
     assertEquals("GET /page2 HTTP/1.1", page2.getRequestLine());
   }
 
+  @Test public void getClientRequestTimeout() throws Exception {
+    enqueueRequestTimeoutResponses();
+
+    Response response = client.newCall(new Request.Builder()
+        .url(server.url("/")).build()).execute();
+
+    assertEquals("Body", response.body().string());
+  }
+
+  private void enqueueRequestTimeoutResponses() {
+    server.enqueue(new MockResponse()
+        .setSocketPolicy(SocketPolicy.DISCONNECT_AT_END)
+        .setResponseCode(HttpURLConnection.HTTP_CLIENT_TIMEOUT)
+        .setHeader("Connection", "Close")
+        .setBody("You took too long!"));
+    server.enqueue(new MockResponse().setBody("Body"));
+  }
+
+  @Test public void requestBodyRetransmittedOnClientRequestTimeout() throws Exception {
+    enqueueRequestTimeoutResponses();
+
+    Response response = client.newCall(new Request.Builder()
+        .url(server.url("/"))
+        .post(RequestBody.create(MediaType.parse("text/plain"), "Hello"))
+        .build()).execute();
+
+    assertEquals("Body", response.body().string());
+
+    RecordedRequest request1 = server.takeRequest();
+    assertEquals("Hello", request1.getBody().readUtf8());
+
+    RecordedRequest request2 = server.takeRequest();
+    assertEquals("Hello", request2.getBody().readUtf8());
+  }
+
   @Test public void propfindRedirectsToPropfind() throws Exception {
     server.enqueue(new MockResponse()
         .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP)

okhttp-tests/src/test/java/okhttp3/URLConnectionTest.java

@@ -2327,6 +2327,53 @@ private void testRedirect(boolean temporary, String method) throws Exception {
         trustManager.calls);
   }
 
+  @Test public void getClientRequestTimeout() throws Exception {
+    enqueueClientRequestTimeoutResponses();
+
+    HttpURLConnection connection = urlFactory.open(server.url("/").url());
+
+    assertEquals(200, connection.getResponseCode());
+    assertEquals("Body", readAscii(connection.getInputStream(), Integer.MAX_VALUE));
+  }
+
+  private void enqueueClientRequestTimeoutResponses() {
+    server.enqueue(new MockResponse()
+        .setSocketPolicy(SocketPolicy.DISCONNECT_AT_END)
+        .setResponseCode(HttpURLConnection.HTTP_CLIENT_TIMEOUT)
+        .setHeader("Connection", "Close")
+        .setBody("You took too long!"));
+    server.enqueue(new MockResponse().setBody("Body"));
+  }
+
+  @Test public void bufferedBodyWithClientRequestTimeout() throws Exception {
+    enqueueClientRequestTimeoutResponses();
+
+    HttpURLConnection connection = urlFactory.open(server.url("/").url());
+    connection.setRequestMethod("POST");
+    connection.getOutputStream().write("Hello".getBytes("UTF-8"));
+
+    assertEquals(200, connection.getResponseCode());
+    assertEquals("Body", readAscii(connection.getInputStream(), Integer.MAX_VALUE));
+
+    RecordedRequest request1 = server.takeRequest();
+    assertEquals("Hello", request1.getBody().readUtf8());
+
+    RecordedRequest request2 = server.takeRequest();
+    assertEquals("Hello", request2.getBody().readUtf8());
+  }
+
+  @Test public void streamedBodyWithClientRequestTimeout() throws Exception {
+    enqueueClientRequestTimeoutResponses();
+
+    HttpURLConnection connection = urlFactory.open(server.url("/").url());
+    connection.setRequestMethod("POST");
+    connection.setChunkedStreamingMode(0);
+    connection.getOutputStream().write("Hello".getBytes("UTF-8"));
+
+    assertEquals(408, connection.getResponseCode());
+    assertEquals(1, server.getRequestCount());
+  }
+
   @Test public void readTimeouts() throws IOException {
     // This relies on the fact that MockWebServer doesn't close the
     // connection after a response has been sent. This causes the client to

okhttp-urlconnection/src/test/java/okhttp3/UrlConnectionCacheTest.java

@@ -161,6 +161,8 @@ public final class UrlConnectionCacheTest {
   }
 
   private void assertCached(boolean shouldPut, int responseCode) throws Exception {
+    int expectedResponseCode = responseCode;
+
     server = new MockWebServer();
     MockResponse response = new MockResponse()
         .addHeader("Last-Modified: " + formatDate(-1, TimeUnit.HOURS))
@@ -174,11 +176,22 @@ private void assertCached(boolean shouldPut, int responseCode) throws Exception
       response.addHeader("WWW-Authenticate: Basic realm=\"protected area\"");
     }
     server.enqueue(response);
+
+    if (responseCode == HttpURLConnection.HTTP_CLIENT_TIMEOUT) {
+      // 408's are a bit of an outlier because we may repeat the request if we encounter this
+      // response code. In this scenario, there are 2 responses: the initial 408 and then the 200
+      // because of the retry. We just want to ensure the initial 408 isn't cached.
+      expectedResponseCode = 200;
+      server.enqueue(new MockResponse()
+          .addHeader("Cache-Control", "no-store")
+          .setBody("FGHIJ"));
+    }
+
     server.start();
 
     URL url = server.url("/").url();
     HttpURLConnection conn = urlFactory.open(url);
-    assertEquals(responseCode, conn.getResponseCode());
+    assertEquals(expectedResponseCode, conn.getResponseCode());
 
     // exhaust the content stream
     readAscii(conn);

okhttp/src/main/java/okhttp3/internal/http/HttpEngine.java

@@ -51,6 +51,7 @@
 import okio.Source;
 import okio.Timeout;
 
+import static java.net.HttpURLConnection.HTTP_CLIENT_TIMEOUT;
 import static java.net.HttpURLConnection.HTTP_MOVED_PERM;
 import static java.net.HttpURLConnection.HTTP_MOVED_TEMP;
 import static java.net.HttpURLConnection.HTTP_MULT_CHOICE;
@@ -868,8 +869,8 @@ public void receiveHeaders(Headers headers) throws IOException {
 
   /**
    * Figures out the HTTP request to make in response to receiving this engine's response. This will
-   * either add authentication headers or follow redirects. If a follow-up is either unnecessary or
-   * not applicable, this returns null.
+   * either add authentication headers, follow redirects or handle a client request timeout. If a
+   * follow-up is either unnecessary or not applicable, this returns null.
    */
   public Request followUpRequest() throws IOException {
     if (userResponse == null) throw new IllegalStateException();
@@ -940,6 +941,17 @@ public Request followUpRequest() throws IOException {
 
         return requestBuilder.url(url).build();
 
+      case HTTP_CLIENT_TIMEOUT:
+        // 408's are rare in practice, but some servers like HAProxy use this response code. The
+        // spec says that we may repeat the request without modifications. Modern browsers also
+        // repeat the request (even non-idempotent ones.)
+        boolean retryableBody = requestBodyOut == null || requestBodyOut instanceof RetryableSink;
+        if (callerWritesRequestBody && !retryableBody) {
+          return null;
+        }
+
+        return userRequest;
+
       default:
         return null;
     }