From b7155fd57239e986bbaba254a91aeb9600d60305 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Thu, 2 Sep 2010 18:31:25 -0700 Subject: [PATCH] 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 --- .../net/www/protocol/http/HttpConnection.java | 14 +++++++++---- .../www/protocol/https/HttpsURLConnectionImpl.java | 10 ++++++++- .../java/libcore/java/net/URLConnectionTest.java | 24 ++++++++++++++++++++-- 3 files changed, 41 insertions(+), 7 deletions(-) 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 33135107..dfda31f1 100644 --- 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 ddbc54ee..67a1a1a8 100644 --- 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 0467be48..5566a7f0 100644 --- 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) { } } -- 2.11.0