Scope WebView cookies to the request host - #211
Open
mbarta wants to merge 3 commits into
Open
Conversation
The offline/subresource HTTP path (and the navigation HttpRepository) attached the WebView cookie for the original URL as a manual Cookie request header, then let OkHttp follow redirects with a default client (no CookieJar). OkHttp strips only Authorization on a cross-host redirect, so the manually-set Cookie header — including the session cookie — followed a cross-host redirect to the redirect target. The final response's Set-Cookie was also written back to the original URL, letting a redirect target plant cookies under the original host. Replace the manual cookie plumbing with a CookieJar backed by the WebView CookieManager and stop hand-setting the Cookie header. The CookieJar loads cookies per-hop against each request's actual host and saves Set-Cookie against the emitting host, so cookies no longer leak to or from a cross-host redirect target. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover host-scoped cookie loading and saving, the OkHttp integration (request cookies, response cookies, redirect hops), and the shared client wiring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BaseRepositoryTest replaces HotwireHttpClient.instance with a test client and never restored it, leaking a client without a cookie jar into later tests in the same JVM. WebViewCookieJarTest asserted on the shared client's cookie jar and failed depending on test order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
|
🤖 Heads up @mbarta — I've rebased #213 onto this branch, so it's now stacked on top of your per-request-host With #211 handling the cookie scoping, #213 narrows to hardening the native redirect-verification fetch on top of it:
Net effect: the cookie-scoping fix converges here on #211, and #213 layers the verification-path hardening on top. #213's base is now |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Replace the manual
Cookie-header plumbing in the navigation and offline/subresource HTTP paths with an OkHttpCookieJarbacked by the WebViewCookieManager. Same change as hotwired/turbo-android#356.Why
Today
HttpRepositoryandOfflineHttpRepositoryattach cookies by readingCookieManager.getCookie(url)for the original request URL and setting a manualCookieheader, then hand redirect-following to OkHttp using a client with noCookieJar. Because the header is set by hand rather than managed by aCookieJar, OkHttp does not re-evaluate it per hop, so it no longer tracks the actual host once a request is redirected.Set-Cookieon the final response is likewise written back against the original URL rather than the host that emitted it. The net effect is that OkHttp's cookie scoping can diverge from what the WebView itself would apply.How
A small
WebViewCookieJar : CookieJardelegates toCookieManager:loadForRequest(url)returns the cookiesCookieManagerholds for that request's host — so OkHttp attaches the right cookies for each hop, including after a redirect.saveFromResponse(url, cookies)writesSet-Cookieback against the host that actually emitted it.HotwireHttpClientinstalls the jar; the repositories stop hand-setting theCookieheader, andOfflineHttpRepositorydrops any incomingCookieheader from the interceptedWebResourceRequestso the jar is the single source of cookies.Testing
WebViewCookieJarTestcovers host-scoped loading and saving, the OkHttp integration (request cookies, response cookies, redirect hops viaMockWebServer), and the shared client wiring.:core:testDebugUnitTestpasses.