Carry cookies across redirects via an opt-in CookieStorage (#2671)#2897
Merged
Conversation
The Cookie header is sensitive, so it's stripped when following redirects, and cookies set via Set-Cookie within a redirect chain were lost. This adds an opt-in CookieStorage cookie jar, attached to a request via .cookieStorage(...). FollowRedirectsBackend then, for each request in a redirect chain, sends the stored cookies matching the request and threads an updated storage to the next request. Matching follows a subset of RFC 6265 (domain, path, Secure); Max-Age<=0 removes a cookie. Default behaviour is unchanged unless a storage is attached. Implements the approach outlined in #2671; supersedes the test-only PR #2672. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 tasks
response.unsafeCookies pulls in CookieWithMeta.parse, which uses java.time.DateTimeFormatterBuilder to parse the Expires attribute - a symbol not available on Scala Native, which broke linking of the (shared) core FollowRedirectsBackend. Parse the Set-Cookie headers directly, reading only the attributes used for storage (Domain, Path, Secure, Max-Age) and ignoring Expires, so cookie handling works on all platforms. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CookieWithMeta.toString formats the Expires date via java.time, which isn't available on Scala Native, and instantiating it in core (shared) code made that symbol reachable, breaking the Native link. Represent stored cookies as plain name/value pairs: parse Set-Cookie headers directly and exchange (name, value) tuples with the request. The feature now works on JVM, JS and Native. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Compute a path-less cookie's default-path from the setting request's directory per RFC 6265 5.1.4, instead of always "/" (a path-less cookie set at /admin/x is no longer sent to unrelated paths). - Fix scaladoc links (PartialRequestBuilder.cookieStorage) and reword the CookieWithMeta/Scala Native comment to be accurate. - Extract the send-matching predicate into a named `matches` helper. - Add tests: redirect signalled by a thrown exception, domain normalization, path-segment-boundary matching, and the RFC default-path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CookieWithMeta is available on all platforms; the actual issue is that its Set-Cookie rendering/parsing reaches java.time date formatting, which Scala Native's javalib doesn't provide, breaking the Native link when reached. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Closes #2671. Supersedes #2672 (which added a failing test demonstrating the problem, but no fix).
Problem
The
Cookieheader is a sensitive header, so it is stripped when following a redirect. As a result, cookies set viaSet-Cookieduring a redirect chain were never sent to subsequent requests in that chain (see #2671).Approach
Implements the design @adamw outlined in #2671: an opt-in cookie jar.
sttp.client4.wrappers.CookieStorage— an immutable cookie jar.set(setBy, cookies)collectsSet-Cookiecookies (rejecting ones whoseDomaindoesn't match the setting host);forUri(uri)returns the cookies to send to a URI. Matching follows a subset of RFC 6265: domain-matching, path-matching and theSecureattribute. Time-based expiry isn't tracked, butMax-Age<= 0 removes a cookie.RequestBuilder.cookieStorage(storage)attaches a storage to a request (via a request attribute).FollowRedirectsBackend(applied to all backends by default), when a storage is attached, sends the matching stored cookies with each request in a redirect chain and threads an updated storage through to the next request.Default behaviour is unchanged unless a
CookieStorageis explicitly attached.Tests
CookieStorageTest— domain/host-only isolation, subdomain matching, cross-domain rejection, path,Secure, overwrite andMax-Agedeletion.FollowRedirectsBackendTest— cookies set across a redirect chain reach subsequent requests when a storage is attached, and are not carried when it isn't.Full
coresuite passes (631 tests), cross-compiled for Scala 2.12 / 2.13 / 3.Credit to @Zerkath for the original report and test case in #2671 / #2672.