Skip to content

Redirection cookie handling tests for issue #2671#2672

Closed
Zerkath wants to merge 2 commits into
softwaremill:masterfrom
Zerkath:master
Closed

Redirection cookie handling tests for issue #2671#2672
Zerkath wants to merge 2 commits into
softwaremill:masterfrom
Zerkath:master

Conversation

@Zerkath
Copy link
Copy Markdown

@Zerkath Zerkath commented Sep 23, 2025

Tests for #2671 which are now failing with the following output:

- should pass cookies during redirections *** FAILED *** (13 milliseconds)
  Vector() did not contain element "session" (in Checkpoint) at FollowRedirectsBackendTest.scala:90
  Vector() did not contain element "session" (in Checkpoint) at FollowRedirectsBackendTest.scala:90
  Vector() did not contain element "session" (in Checkpoint) at FollowRedirectsBackendTest.scala:90
  Vector() did not contain element "session" (in Checkpoint) at FollowRedirectsBackendTest.scala:90
  Vector() did not contain element "session" (in Checkpoint) at FollowRedirectsBackendTest.scala:90
  Vector(session=final) did not equal List(0-fuzz=, 1-fuzz=, 2-fuzz=, 3-fuzz=, 4-fuzz=, session=final) (in Checkpoint) 

at FollowRedirectsBackendTest.scala:118 (FollowRedirectsBackendTest.scala:119)

Before submitting pull request:

  • Check if the project compiles by running sbt compile
  • Verify docs compilation by running sbt compileDocs
  • Check if tests pass by running sbt test
  • Format code by running sbt scalafmt

Redirects do not apply set-cookie in the request chain meaning
multistep request chains that track state will be missing cookies in the
middle of redirection, only real alternative is manual control

After redirection chain completes any set-cookies that started or were
in the middle of the request chain get ignored. Only the last responses set-cookie's
will be applied. It is possible to collect the cookies / headers from
response.history, but adds complexity.
@adamw
Copy link
Copy Markdown
Member

adamw commented May 29, 2026

Thanks for the report and the failing test case @Zerkath. I've opened #2897, which implements the opt-in CookieStorage approach @adamw described above: cookies set via Set-Cookie during a redirect chain are collected and sent to subsequent matching requests, when a CookieStorage is attached to the request via .cookieStorage(...). Your scenario from this PR is covered as an integration test there. This PR can be closed in favour of #2897 once that's reviewed.

adamw added a commit that referenced this pull request May 29, 2026
…2897)

Closes #2671. Supersedes #2672 (which added a failing test demonstrating
the problem, but no fix).

## Problem

The `Cookie` header is a sensitive header, so it is stripped when
following a redirect. As a result, cookies set via `Set-Cookie` during 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.

- New `sttp.client4.wrappers.CookieStorage` — an immutable cookie jar.
`set(setBy, cookies)` collects `Set-Cookie` cookies (rejecting ones
whose `Domain` doesn't match the setting host); `forUri(uri)` returns
the cookies to send to a URI. Matching follows a subset of [RFC
6265](https://www.rfc-editor.org/rfc/rfc6265): domain-matching,
path-matching and the `Secure` attribute. Time-based expiry isn't
tracked, but `Max-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 `CookieStorage` is
explicitly attached.

## Tests

- `CookieStorageTest` — domain/host-only isolation, subdomain matching,
cross-domain rejection, path, `Secure`, overwrite and `Max-Age`
deletion.
- `FollowRedirectsBackendTest` — cookies set across a redirect chain
reach subsequent requests when a storage is attached, and are not
carried when it isn't.

Full `core` suite 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.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@adamw adamw closed this May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants