diff --git a/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java b/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java index 7622aa386..ce68e72e8 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java @@ -55,8 +55,8 @@ import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.List; import java.util.Map; -import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLSocket; import static com.squareup.okhttp.internal.Util.US_ASCII; import static com.squareup.okhttp.internal.Util.UTF_8; @@ -537,16 +537,16 @@ public final class HttpResponseCache extends ResponseCache { this.requestMethod = httpConnection.getRequestMethod(); this.responseHeaders = RawHeaders.fromMultimap(httpConnection.getHeaderFields(), true); - if (isHttps()) { - HttpsURLConnection httpsConnection = (HttpsURLConnection) httpConnection; - cipherSuite = httpsConnection.getCipherSuite(); + SSLSocket sslSocket = getSslSocket(httpConnection); + if (sslSocket != null) { + cipherSuite = sslSocket.getSession().getCipherSuite(); Certificate[] peerCertificatesNonFinal = null; try { - peerCertificatesNonFinal = httpsConnection.getServerCertificates(); + peerCertificatesNonFinal = sslSocket.getSession().getPeerCertificates(); } catch (SSLPeerUnverifiedException ignored) { } peerCertificates = peerCertificatesNonFinal; - localCertificates = httpsConnection.getLocalCertificates(); + localCertificates = sslSocket.getSession().getLocalCertificates(); } else { cipherSuite = null; peerCertificates = null; @@ -554,6 +554,22 @@ public final class HttpResponseCache extends ResponseCache { } } + /** + * Returns the SSL socket used by {@code httpConnection} for HTTPS, nor null + * if the connection isn't using HTTPS. Since we permit redirects across + * protocols (HTTP to HTTPS or vice versa), the implementation type of the + * connection doesn't necessarily match the implementation type of its HTTP + * engine. + */ + private SSLSocket getSslSocket(HttpURLConnection httpConnection) { + HttpEngine engine = httpConnection instanceof HttpsURLConnectionImpl + ? ((HttpsURLConnectionImpl) httpConnection).getHttpEngine() + : ((HttpURLConnectionImpl) httpConnection).getHttpEngine(); + return engine instanceof HttpsURLConnectionImpl.HttpsEngine + ? ((HttpsURLConnectionImpl.HttpsEngine) engine).getSslSocket() + : null; + } + public void writeTo(DiskLruCache.Editor editor) throws IOException { OutputStream out = editor.newOutputStream(ENTRY_METADATA); Writer writer = new BufferedWriter(new OutputStreamWriter(out, UTF_8)); diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java index 235f86295..249fb98da 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java @@ -447,6 +447,10 @@ public final class HttpsURLConnectionImpl extends HttpsURLConnection { return false; } + public SSLSocket getSslSocket() { + return sslSocket; + } + @Override protected TunnelRequest getTunnelConfig() { String userAgent = requestHeaders.getUserAgent(); if (userAgent == null) { diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java index 64a473848..dee87cf3e 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java @@ -89,6 +89,7 @@ public final class HttpResponseCacheTest { }; private final OkHttpClient client = new OkHttpClient(); private MockWebServer server = new MockWebServer(); + private MockWebServer server2 = new MockWebServer(); private HttpResponseCache cache; private final CookieManager cookieManager = new CookieManager(); @@ -113,6 +114,7 @@ public final class HttpResponseCacheTest { @After public void tearDown() throws Exception { server.shutdown(); + server2.shutdown(); ResponseCache.setDefault(null); cache.delete(); CookieHandler.setDefault(null); @@ -235,6 +237,7 @@ public final class HttpResponseCacheTest { Map> requestHeaders) throws IOException { return null; } + @Override public CacheRequest put(URI uri, URLConnection conn) throws IOException { HttpURLConnection httpConnection = (HttpURLConnection) conn; try { @@ -449,6 +452,42 @@ public final class HttpResponseCacheTest { assertEquals(2, cache.getHitCount()); } + /** + * We've had bugs where caching and cross-protocol redirects yield class + * cast exceptions internal to the cache because we incorrectly assumed that + * HttpsURLConnection was always HTTPS and HttpURLConnection was always HTTP; + * in practice redirects mean that each can do either. + * + * https://github.com/square/okhttp/issues/214 + */ + @Test public void secureResponseCachingAndProtocolRedirects() throws IOException { + server2.useHttps(sslContext.getSocketFactory(), false); + server2.enqueue(new MockResponse().addHeader("Last-Modified: " + formatDate(-1, TimeUnit.HOURS)) + .addHeader("Expires: " + formatDate(1, TimeUnit.HOURS)) + .setBody("ABC")); + server2.enqueue(new MockResponse().setBody("DEF")); + server2.play(); + + server.enqueue(new MockResponse().addHeader("Last-Modified: " + formatDate(-1, TimeUnit.HOURS)) + .addHeader("Expires: " + formatDate(1, TimeUnit.HOURS)) + .setResponseCode(HttpURLConnection.HTTP_MOVED_PERM) + .addHeader("Location: " + server2.getUrl("/"))); + server.play(); + + client.setSslSocketFactory(sslContext.getSocketFactory()); + client.setHostnameVerifier(NULL_HOSTNAME_VERIFIER); + + HttpURLConnection connection1 = client.open(server.getUrl("/")); + assertEquals("ABC", readAscii(connection1)); + + // Cached! + HttpURLConnection connection2 = client.open(server.getUrl("/")); + assertEquals("ABC", readAscii(connection2)); + + assertEquals(4, cache.getRequestCount()); // 2 direct + 2 redirect = 4 + assertEquals(2, cache.getHitCount()); + } + @Test public void responseCacheRequestHeaders() throws IOException, URISyntaxException { server.enqueue(new MockResponse().setBody("ABC")); server.play();