Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,13 @@ class Cache internal constructor(
return null
}

if (response.request.url.isHttps && response.handshake == null) {
// An HTTPS response without a handshake cannot be serialized to the cache format because the
// Entry reader expects TLS session data for HTTPS URLs. This can happen when a network
// interceptor strips the handshake or when cacheUrlOverride rewrites an HTTP URL to HTTPS.
return null
}

val entry = Entry(response)
var editor: DiskLruCache.Editor? = null
try {
Expand All @@ -259,6 +266,9 @@ class Cache internal constructor(
cached: Response,
network: Response,
) {
if (network.request.url.isHttps && network.handshake == null) {
return
}
val entry = Entry(network)
val snapshot = (cached.body as CacheResponseBody).snapshot
var editor: DiskLruCache.Editor? = null
Expand Down
20 changes: 10 additions & 10 deletions okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -423,14 +423,13 @@ class CacheTest {
}

/**
* A network interceptor strips the handshake from a real HTTPS response before the
* CacheInterceptor writes it to disk. This creates the bug condition: url.isHttps=true
* but handshake=null. Before the fix, `handshake!!` in writeTo() threw NPE.
* An HTTPS response with a null handshake is rejected by Cache.put() rather than writing a
* malformed cache entry. The response still succeeds but nothing is written to disk.
*
* https://github.com/square/okhttp/issues/8962
*/
@Test
fun httpsResponseWithNullHandshakeDoesNotCrashWriteTo() {
fun httpsResponseWithNullHandshakeIsNotCached() {
server.useHttps(handshakeCertificates.sslSocketFactory())
server.enqueue(
MockResponse
Expand Down Expand Up @@ -458,11 +457,11 @@ class CacheTest {
val response = client.newCall(Request(server.url("/"))).execute()
assertThat(response.code).isEqualTo(200)
assertThat(response.body.string()).isEqualTo("secure content")
assertThat(cache.writeSuccessCount()).isEqualTo(0)
}

/**
* Verifies the null-handshake fix holds across multiple sequential cache writes, confirming
* it is not a one-time race condition.
* Multiple HTTPS requests with null handshake all succeed without writing to cache.
*
* https://github.com/square/okhttp/issues/8962
*/
Expand Down Expand Up @@ -499,12 +498,12 @@ class CacheTest {
assertThat(response.code).isEqualTo(200)
assertThat(response.body.string()).isEqualTo("response $i")
}
assertThat(cache.writeSuccessCount()).isEqualTo(0)
}

/**
* When handshake is null for an HTTPS URL, the TLS block is skipped making the entry
* unreadable on re-read. The response should still succeed but won't be served from cache
* on subsequent requests.
* When handshake is null for an HTTPS URL, Cache.put() rejects the entry so nothing is written
* to disk. Subsequent requests hit the network.
*
* https://github.com/square/okhttp/issues/8962
*/
Expand Down Expand Up @@ -544,10 +543,11 @@ class CacheTest {
val response1 = client.newCall(Request(server.url("/"))).execute()
assertThat(response1.body.string()).isEqualTo("first")

// Second request hits the network again because the first entry was not cacheable
// Second request hits the network because the first was not written to cache
val response2 = client.newCall(Request(server.url("/"))).execute()
assertThat(response2.body.string()).isEqualTo("second")
assertThat(response2.cacheResponse).isNull()
assertThat(cache.writeSuccessCount()).isEqualTo(0)
}

@Test
Expand Down