am 0f102f51: Add further handling for when a CONNECT incorrectly returns a body.

* commit '0f102f51711ecc2ef9f25cbbad2148ee97bdb6cb':
  Add further handling for when a CONNECT incorrectly returns a body.

2 files changed, 48 insertions, 1 deletions

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 127807f..1d2e2bb 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
@@ -2946,6 +2946,39 @@ public final class URLConnectionTest {
   }
 
   /**
+   * Tolerate bad https proxy response when using HttpResponseCache. Android bug 6754912.
+   */
+  @Test
+  public void testConnectViaHttpProxyToHttpsUsingBadProxyAndHttpResponseCache() throws Exception {
+    initResponseCache();
+
+    server.useHttps(sslContext.getSocketFactory(), true);
+    // The inclusion of a body in the response to a CONNECT is key to reproducing b/6754912.
+    MockResponse
+        badProxyResponse = new MockResponse()
+        .setSocketPolicy(SocketPolicy.UPGRADE_TO_SSL_AT_END)
+        .clearHeaders()
+        .setBody("bogus proxy connect response content");
+
+    server.enqueue(badProxyResponse);
+    server.enqueue(new MockResponse().setBody("response"));
+
+    server.play();
+
+    URL url = new URL("https://android.com/foo");
+    client.setSslSocketFactory(sslContext.getSocketFactory());
+    client.setHostnameVerifier(new RecordingHostnameVerifier());
+
+    ProxyConfig proxyConfig = ProxyConfig.PROXY_SYSTEM_PROPERTY;
+    HttpsURLConnection connection = (HttpsURLConnection) proxyConfig.connect(server, client, url);
+    assertContent("response", connection);
+
+    RecordedRequest connect = server.takeRequest();
+    assertEquals("CONNECT android.com:443 HTTP/1.1", connect.getRequestLine());
+    assertContains(connect.getHeaders(), "Host: android.com");
+  }
+
+  /**
    * The RFC is unclear in this regard as it only specifies that this should
    * invalidate the cache entry (if any).
    */
diff --git a/okhttp/src/main/java/com/squareup/okhttp/Connection.java b/okhttp/src/main/java/com/squareup/okhttp/Connection.java
index 94527af..743c33b 100644
--- a/okhttp/src/main/java/com/squareup/okhttp/Connection.java
+++ b/okhttp/src/main/java/com/squareup/okhttp/Connection.java
@@ -17,10 +17,12 @@
 package com.squareup.okhttp;
 
 import com.squareup.okhttp.internal.Platform;
+import com.squareup.okhttp.internal.Util;
 import com.squareup.okhttp.internal.http.HttpAuthenticator;
 import com.squareup.okhttp.internal.http.HttpConnection;
 import com.squareup.okhttp.internal.http.HttpEngine;
 import com.squareup.okhttp.internal.http.HttpTransport;
+import com.squareup.okhttp.internal.http.OkHeaders;
 import com.squareup.okhttp.internal.http.SpdyTransport;
 import com.squareup.okhttp.internal.spdy.SpdyConnection;
 import java.io.Closeable;
@@ -29,6 +31,8 @@ import java.net.Proxy;
 import java.net.Socket;
 import javax.net.ssl.SSLSocket;
 import okio.ByteString;
+import okio.OkBuffer;
+import okio.Source;
 
 import static java.net.HttpURLConnection.HTTP_OK;
 import static java.net.HttpURLConnection.HTTP_PROXY_AUTH;
@@ -353,12 +357,22 @@ public final class Connection implements Closeable {
       tunnelConnection.writeRequest(request.headers(), requestLine);
       tunnelConnection.flush();
       Response response = tunnelConnection.readResponse().request(request).build();
-      tunnelConnection.emptyResponseBody();
+      // The response body from a CONNECT should be empty, but if it is not then we should consume
+      // it before proceeding.
+      long contentLength = OkHeaders.contentLength(response);
+      if (contentLength != -1) {
+        Source body = tunnelConnection.newFixedLengthSource(null, contentLength);
+        Util.skipAll(body, Integer.MAX_VALUE);
+      } else {
+        tunnelConnection.emptyResponseBody();
+      }
 
       switch (response.code()) {
         case HTTP_OK:
           // Assume the server won't send a TLS ServerHello until we send a TLS ClientHello. If that
           // happens, then we will have buffered bytes that are needed by the SSLSocket!
+          // This check is imperfect: it doesn't tell us whether a handshake will succeed, just that
+          // it will almost certainly fail because the proxy has sent unexpected data.
           if (tunnelConnection.bufferSize() > 0) {
             throw new IOException("TLS tunnel buffered too many bytes!");
           }