From 95c83c05c770fcf1e1d3c167d54972f870286935 Mon Sep 17 00:00:00 2001 From: Vamsi Date: Sun, 29 Mar 2026 21:21:18 -0500 Subject: [PATCH 1/2] cache: guard null fields in Entry.writeTo() to fix NPE (#8962) Reject HTTPS responses with null handshake in Cache.put() and Cache.update() so malformed entries are never written to disk. --- .../commonJvmAndroid/kotlin/okhttp3/Cache.kt | 10 ++++++++++ .../src/jvmTest/kotlin/okhttp3/CacheTest.kt | 20 +++++++++---------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt index c4076307e52a..1073fd6bb01a 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt @@ -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 { @@ -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 diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt index 238a02499fef..758a2bd7aa4f 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/CacheTest.kt @@ -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 @@ -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 */ @@ -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 */ @@ -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 From 0281a44c83133a0843ec7ebf23ab34672cebe3bd Mon Sep 17 00:00:00 2001 From: Vamsi <12653642+iVamsi@users.noreply.github.com> Date: Sun, 5 Apr 2026 12:34:31 -0500 Subject: [PATCH 2/2] cache: add platform log when skipping HTTPS cache without handshake --- okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt index 1073fd6bb01a..a6bbdab08f47 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt @@ -242,6 +242,10 @@ class Cache internal constructor( // 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. + Platform.get().log( + "HTTPS response has no handshake; skipping cache write for ${response.request.url.redact()}", + WARN, + ) return null } @@ -266,7 +270,12 @@ class Cache internal constructor( cached: Response, network: Response, ) { + // HTTPS responses without a handshake cannot be serialized (see put() for details). if (network.request.url.isHttps && network.handshake == null) { + Platform.get().log( + "HTTPS response has no handshake; skipping cache update for ${network.request.url.redact()}", + WARN, + ) return } val entry = Entry(network)