Fixing a broken pipe with HttpsURLConnection connection reuse.

We had a bug where we were peeking a byte from the raw socket
stream rather than the encrypted stream when we were attempting
to reuse an HTTPS connection. The raw stream field shouldn't
have been initialized, but setupTransportIO() did it while the
connection was being established.

This fixes setupTransportIO() to not initialize the fields
and isStale() to use the SSL fields if they exist.

See http://code.google.com/p/android/issues/detail?id=8625

Change-Id: Idd5b6f20e098adf436997829940707548896bedb

diff --git a/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/http/HttpConnection.java b/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/http/HttpConnection.java
index 3313510..dfda31f 100644 (file)
--- a/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/http/HttpConnection.java
+++ b/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/http/HttpConnection.java
@@ -148,6 +148,10 @@ public final class HttpConnection {
         return inputStream;
     }
 
+    private Socket getSocket() {
+        return sslSocket != null ? sslSocket : socket;
+    }
+
     public Address getAddress() {
         return address;
     }
@@ -198,17 +202,19 @@ public final class HttpConnection {
             return true;
         }
 
-        if (inputStream.available() > 0) {
+        InputStream in = getInputStream();
+        if (in.available() > 0) {
             return false;
         }
 
+        Socket socket = getSocket();
         int soTimeout = socket.getSoTimeout();
         try {
             socket.setSoTimeout(1);
-            inputStream.mark(1);
-            int byteRead = inputStream.read();
+            in.mark(1);
+            int byteRead = in.read();
             if (byteRead != -1) {
-                inputStream.reset();
+                in.reset();
                 return false;
             }
             return true; // the socket is reporting all data read; it's stale

diff --git a/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/https/HttpsURLConnectionImpl.java b/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/https/HttpsURLConnectionImpl.java
index ddbc54e..67a1a1a 100644 (file)
--- a/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/https/HttpsURLConnectionImpl.java
+++ b/luni/src/main/java/org/apache/harmony/luni/internal/net/www/protocol/https/HttpsURLConnectionImpl.java
@@ -30,6 +30,7 @@ import java.util.Map;
 import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLPeerUnverifiedException;
 import javax.net.ssl.SSLSocket;
+import org.apache.harmony.luni.internal.net.www.protocol.http.HttpConnection;
 import org.apache.harmony.luni.internal.net.www.protocol.http.HttpURLConnectionImpl;
 
 /**
@@ -373,7 +374,7 @@ public class HttpsURLConnectionImpl extends HttpsURLConnection {
             super.connect();
 
             // make SSL Tunnel
-            if (usingProxy()) {
+            if (requiresTunnel()) {
                 String originalMethod = method;
                 method = CONNECT;
                 intermediateResponse = true;
@@ -392,6 +393,13 @@ public class HttpsURLConnectionImpl extends HttpsURLConnection {
             setUpTransportIO(connection);
         }
 
+        @Override protected void setUpTransportIO(HttpConnection connection) throws IOException {
+            if (!requiresTunnel() && sslSocket == null) {
+                return; // don't initialize streams that won't be used
+            }
+            super.setUpTransportIO(connection);
+        }
+
         @Override protected boolean requiresTunnel() {
             return usingProxy();
         }

diff --git a/luni/src/test/java/libcore/java/net/URLConnectionTest.java b/luni/src/test/java/libcore/java/net/URLConnectionTest.java
index 0467be4..5566a7f 100644 (file)
--- a/luni/src/test/java/libcore/java/net/URLConnectionTest.java
+++ b/luni/src/test/java/libcore/java/net/URLConnectionTest.java
@@ -382,16 +382,36 @@ public class URLConnectionTest extends junit.framework.TestCase {
         server.enqueue(new MockResponse().setBody("another response via HTTPS"));
         server.play();
 
+        HttpsURLConnection connection = (HttpsURLConnection) server.getUrl("/").openConnection();
+        connection.setSSLSocketFactory(testSSLContext.clientContext.getSocketFactory());
+        assertContent("this response comes via HTTPS", connection);
+
+        connection = (HttpsURLConnection) server.getUrl("/").openConnection();
+        connection.setSSLSocketFactory(testSSLContext.clientContext.getSocketFactory());
+        assertContent("another response via HTTPS", connection);
+
+        assertEquals(0, server.takeRequest().getSequenceNumber());
+        assertEquals(1, server.takeRequest().getSequenceNumber());
+    }
+
+    public void testConnectViaHttpsReusingConnectionsDiffeerentFactories()
+            throws IOException, InterruptedException {
+        TestSSLContext testSSLContext = TestSSLContext.create();
+
+        server.useHttps(testSSLContext.serverContext.getSocketFactory(), false);
+        server.enqueue(new MockResponse().setBody("this response comes via HTTPS"));
+        server.enqueue(new MockResponse().setBody("another response via HTTPS"));
+        server.play();
+
         // install a custom SSL socket factory so the server can be authorized
         HttpsURLConnection connection = (HttpsURLConnection) server.getUrl("/").openConnection();
         connection.setSSLSocketFactory(testSSLContext.clientContext.getSocketFactory());
         assertContent("this response comes via HTTPS", connection);
 
-        // without an SSL socket factory, the connection should fail
         connection = (HttpsURLConnection) server.getUrl("/").openConnection();
         try {
             readAscii(connection.getInputStream(), Integer.MAX_VALUE);
-            fail();
+            fail("without an SSL socket factory, the connection should fail");
         } catch (SSLException expected) {
         }
     }