1
0
mirror of https://github.com/square/okhttp.git synced 2026-01-25 16:01:38 +03:00

Fix a bug in caching cross-protocol redirects.

This commit is contained in:
jwilson
2013-06-23 18:53:15 -04:00
parent 5516526004
commit 08ac8245dc
3 changed files with 65 additions and 6 deletions

View File

@@ -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));

View File

@@ -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) {

View File

@@ -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<String, List<String>> 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();