Skip to content

Don't cache HTTPS responses that have no handshake - #9395

Open
iVamsi wants to merge 13 commits into
lysine-dev:mainfrom
iVamsi:fix/cache-reject-https-null-handshake-8962
Open

Don't cache HTTPS responses that have no handshake#9395
iVamsi wants to merge 13 commits into
lysine-dev:mainfrom
iVamsi:fix/cache-reject-https-null-handshake-8962

Conversation

@iVamsi

@iVamsi iVamsi commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

HTTPS cache entries always include TLS session data. If the handshake is missing — a network interceptor stripped it, or cacheUrlOverride turned an HTTP URL into HTTPS — we cannot write a readable entry.

#9345 prevented the NPE crash in writeTo(), but still wrote unreadable/malformed entries to disk. This skips the write in put() and update().

Follow-up to #9345 / #8962

Reject HTTPS responses with null handshake in Cache.put() and Cache.update() so malformed entries are never written to disk.
@iVamsi iVamsi changed the title cache: guard null fields in Entry.writeTo() to fix NPE (#8962) cache: reject HTTPS responses with null handshake before cache write (#8962) Mar 30, 2026
Comment thread okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt
Comment thread okhttp/src/commonJvmAndroid/kotlin/okhttp3/Cache.kt
@yschimke

yschimke commented Apr 5, 2026

Copy link
Copy Markdown
Collaborator

@swankjesse do we agree that the result should be skipping caching when there isn't a valid handshake? I'm not certain that this would be expected. or if we should allow for this, maybe with some sentinel NoHandshake handshake?

@iVamsi

iVamsi commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

@swankjesse @yschimke, are any other changes needed to merge this PR?

@iVamsi iVamsi changed the title cache: reject HTTPS responses with null handshake before cache write (#8962) Don't cache HTTPS responses that have no handshake Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants