From 934a8bf726c7f399500c3cda50195f4e853d6420 Mon Sep 17 00:00:00 2001 From: adamw Date: Fri, 29 May 2026 07:44:20 +0000 Subject: [PATCH 1/6] Add CookieStorage to carry cookies across redirects (#2671) 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) --- .../scala/sttp/client4/requestBuilder.scala | 9 ++ .../sttp/client4/wrappers/CookieStorage.scala | 93 +++++++++++++++++++ .../wrappers/FollowRedirectsBackend.scala | 30 +++++- .../client4/FollowRedirectsBackendTest.scala | 46 ++++++++- .../client4/wrappers/CookieStorageTest.scala | 61 ++++++++++++ docs/requests/cookies.md | 17 ++++ 6 files changed, 251 insertions(+), 5 deletions(-) create mode 100644 core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala create mode 100644 core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala diff --git a/core/src/main/scala/sttp/client4/requestBuilder.scala b/core/src/main/scala/sttp/client4/requestBuilder.scala index 9fa1fac912..824ed558dc 100644 --- a/core/src/main/scala/sttp/client4/requestBuilder.scala +++ b/core/src/main/scala/sttp/client4/requestBuilder.scala @@ -4,6 +4,7 @@ import sttp.client4.internal.SttpFile import sttp.client4.internal.Utf8 import sttp.client4.internal.contentTypeWithCharset import sttp.client4.logging.LoggingOptions +import sttp.client4.wrappers.CookieStorage import sttp.client4.wrappers.DigestAuthenticationBackend import sttp.model.HasHeaders import sttp.model.Header @@ -161,6 +162,14 @@ trait PartialRequestBuilder[+PR <: PartialRequestBuilder[PR, R], +R] onDuplicate = DuplicateHeaderBehavior.Combine ) + /** Attaches a [[sttp.client4.wrappers.CookieStorage CookieStorage]] to this request. When following redirects (see + * [[sttp.client4.wrappers.FollowRedirectsBackend]], applied to all backends by default), cookies set via + * `Set-Cookie` in a redirect chain are then collected into the storage and sent with subsequent requests that they + * domain/path-match. Without a storage, cookies are not carried across redirects (the `Cookie` header is sensitive + * and stripped). Start with [[sttp.client4.wrappers.CookieStorage.empty]]. + */ + def cookieStorage(storage: CookieStorage): PR = attribute(CookieStorage.attributeKey, storage) + private[client4] def hasContentType: Boolean = headers.exists(_.is(HeaderNames.ContentType)) private[client4] def setContentTypeIfMissing(mt: MediaType): PR = if (hasContentType) this else contentType(mt) diff --git a/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala b/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala new file mode 100644 index 0000000000..d1fbc59f82 --- /dev/null +++ b/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala @@ -0,0 +1,93 @@ +package sttp.client4.wrappers + +import sttp.attributes.AttributeKey +import sttp.model.Uri +import sttp.model.headers.CookieWithMeta + +/** An immutable cookie jar. + * + * Collects cookies received in `Set-Cookie` response headers and determines which of them should be sent with a + * request to a given URI, applying a subset of the [[https://www.rfc-editor.org/rfc/rfc6265 RFC 6265]] rules: + * domain-matching, path-matching and the `Secure` attribute. + * + * Intended use: attach a storage to a request using [[sttp.client4.RequestBuilder.cookieStorage]]. The + * [[FollowRedirectsBackend]] (applied to all backends by default) then, for each request in a redirect chain, sends + * the matching stored cookies and threads an updated storage through to the next request. This makes cookies set via + * `Set-Cookie` during a redirect chain visible to subsequent requests in that chain - which otherwise doesn't happen, + * as the `Cookie` header is a sensitive header, stripped when following redirects. + * + * Time-based expiry (`Max-Age` > 0, `Expires`) is not tracked, as the storage has no notion of the current time; a + * `Set-Cookie` with `Max-Age` <= 0 removes a matching cookie, so a server can still clear cookies within a chain. + */ +final class CookieStorage private (private val entries: Map[CookieStorage.Key, CookieStorage.Stored]) { + import CookieStorage._ + + /** A new storage updated with the `cookies` received in a response from `setBy`. Following RFC 6265, a cookie whose + * `Domain` attribute does not domain-match `setBy` is rejected (to prevent a host setting cookies for unrelated + * domains). A cookie with `Max-Age` <= 0 removes a matching stored cookie. + */ + def set(setBy: Uri, cookies: Iterable[CookieWithMeta]): CookieStorage = { + val host = hostOf(setBy) + val updated = cookies.foldLeft(entries) { (acc, c) => + val hostOnly = c.domain.isEmpty + val domain = c.domain.map(normalizeDomain).getOrElse(host) + if (domain.isEmpty || !domainMatches(host, domain)) acc + else { + val key = Key(c.name, domain, c.path.getOrElse(DefaultPath)) + if (c.maxAge.exists(_ <= 0)) acc - key + else acc.updated(key, Stored(c, hostOnly)) + } + } + new CookieStorage(updated) + } + + /** The cookies that should be sent with a request to `uri`, according to domain-matching, path-matching and the + * `Secure` attribute (secure cookies are only sent over `https`). + */ + def forUri(uri: Uri): Seq[CookieWithMeta] = { + val host = hostOf(uri) + val path = pathOf(uri) + val secure = uri.scheme.exists(_.equalsIgnoreCase("https")) + entries.collect { + case (key, Stored(cookie, hostOnly)) + if (if (hostOnly) host == key.domain else domainMatches(host, key.domain)) && + pathMatches(path, key.path) && + (!cookie.secure || secure) => + cookie + }.toSeq + } + + def isEmpty: Boolean = entries.isEmpty +} + +object CookieStorage { + + /** An empty storage. */ + val empty: CookieStorage = new CookieStorage(Map.empty) + + /** The attribute key under which a [[CookieStorage]] is attached to a request; see + * [[sttp.client4.RequestBuilder.cookieStorage]]. + */ + val attributeKey: AttributeKey[CookieStorage] = + new AttributeKey[CookieStorage]("sttp.client4.wrappers.CookieStorage") + + private val DefaultPath = "/" + + // a cookie is identified by its name, the domain it's scoped to and its path + private case class Key(name: String, domain: String, path: String) + private case class Stored(cookie: CookieWithMeta, hostOnly: Boolean) + + private def hostOf(uri: Uri): String = uri.host.getOrElse("").toLowerCase + private def pathOf(uri: Uri): String = "/" + uri.path.mkString("/") + private def normalizeDomain(d: String): String = d.stripPrefix(".").toLowerCase + + // RFC 6265, 5.1.3: equal, or `host` is a subdomain of `domain` + private def domainMatches(host: String, domain: String): Boolean = + host == domain || host.endsWith("." + domain) + + // RFC 6265, 5.1.4 + private def pathMatches(requestPath: String, cookiePath: String): Boolean = + requestPath == cookiePath || + (requestPath.startsWith(cookiePath) && + (cookiePath.endsWith("/") || requestPath.charAt(cookiePath.length) == '/')) +} diff --git a/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala b/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala index 0f4af2bfd3..4c4e13d7e2 100644 --- a/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala +++ b/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala @@ -16,15 +16,19 @@ abstract class FollowRedirectsBackend[F[_], P] private ( override def send[T](request: GenericRequest[T, R]): F[Response[T]] = sendWithCounter(request, 0) protected def sendWithCounter[T](request: GenericRequest[T, R], redirects: Int): F[Response[T]] = { + // if a cookie storage is attached, send the cookies matching this request's URI (cookies are otherwise lost + // across redirects, as the `Cookie` header is sensitive and stripped) + val requestWithCookies = applyStoredCookies(request) + // if there are nested follow redirect backends, disabling them and handling redirects here // using a def instead of a val so that errors are properly caught - def resp = delegate.send(request.followRedirects(false)) + def resp = delegate.send(requestWithCookies.followRedirects(false)) if (request.options.followRedirects) { resp .flatMap { (response: Response[T]) => if (response.isRedirect) { - followRedirect(request, response, redirects) + followRedirect(updateStoredCookies(request, response), response, redirects) } else { monad.unit(response) } @@ -34,7 +38,7 @@ abstract class FollowRedirectsBackend[F[_], P] private ( case Some(re) if re.response.isRedirect => re.response.header(HeaderNames.Location) match { case None => monad.error(e) // no location header, propagating the exception - case Some(loc) => followRedirect(request, re.response, redirects, loc) + case Some(loc) => followRedirect(updateStoredCookies(request, re.response), re.response, redirects, loc) } case _ => monad.error(e) } @@ -44,6 +48,26 @@ abstract class FollowRedirectsBackend[F[_], P] private ( } } + /** If a [[CookieStorage]] is attached, adds the cookies matching the request's URI as a `Cookie` header. No-op + * otherwise, so the default behaviour is unchanged unless a storage is explicitly attached. + */ + private def applyStoredCookies[T](request: GenericRequest[T, R]): GenericRequest[T, R] = + request.attribute(CookieStorage.attributeKey) match { + case Some(storage) => + val cookies = storage.forUri(request.uri) + if (cookies.isEmpty) request else request.cookies(cookies) + case None => request + } + + /** If a [[CookieStorage]] is attached, returns the request with the storage updated with the response's `Set-Cookie` + * cookies, so that the next request in the redirect chain can send them. No-op otherwise. + */ + private def updateStoredCookies[T](request: GenericRequest[T, R], response: ResponseMetadata): GenericRequest[T, R] = + request.attribute(CookieStorage.attributeKey) match { + case Some(storage) => request.attribute(CookieStorage.attributeKey, storage.set(request.uri, response.unsafeCookies)) + case None => request + } + private def followRedirect[T]( request: GenericRequest[T, R], response: Response[T], diff --git a/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala b/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala index 5315e266bb..a8983d1717 100644 --- a/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala +++ b/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala @@ -4,9 +4,10 @@ import org.scalatest.EitherValues import org.scalatest.funsuite.AnyFunSuite import org.scalatest.matchers.should.Matchers import sttp.client4.testing.{BackendStub, ResponseStub} -import sttp.client4.wrappers.{FollowRedirectsBackend, FollowRedirectsConfig} +import sttp.client4.wrappers.{CookieStorage, FollowRedirectsBackend, FollowRedirectsConfig} +import sttp.model.headers.CookieWithMeta import sttp.model.internal.Rfc3986 -import sttp.model.{Header, StatusCode, Uri} +import sttp.model.{Header, HeaderNames, StatusCode, Uri} class FollowRedirectsBackendTest extends AnyFunSuite with Matchers with EitherValues { val testData = List( @@ -53,4 +54,45 @@ class FollowRedirectsBackendTest extends AnyFunSuite with Matchers with EitherVa result.body.value shouldBe "All good!" } + // a redirect chain example.com/0 -> /1 -> ... -> /n, where each hop sets a cookie `c`; records the cookies + // (name=value) seen in the `Cookie` header of each request, by target id + private def redirectChainSettingCookies(n: Int): (SyncBackend, collection.Map[Int, Set[String]]) = { + def url(id: Int) = uri"https://example.com/$id" + val seen = scala.collection.mutable.Map[Int, Set[String]]() + val stub = BackendStub.synchronous.whenRequestMatchesPartial { + case r if r.uri.host.contains("example.com") => + val id = r.uri.path.last.toInt + seen(id) = r.header(HeaderNames.Cookie).map(_.split("; ").toSet).getOrElse(Set.empty) + if (id < n) + ResponseStub.adjust( + "", + StatusCode.Found, + Vector(Header.location(url(id + 1)), Header.setCookie(CookieWithMeta(s"c$id", id.toString))) + ) + else ResponseStub.adjust("done", StatusCode.Ok) + } + (FollowRedirectsBackend(stub), seen) + } + + test("should send cookies set during a redirect chain to subsequent requests when a cookie storage is attached") { + val (backend, seen) = redirectChainSettingCookies(3) + + val result = basicRequest.get(uri"https://example.com/0").cookieStorage(CookieStorage.empty).send(backend) + + result.code shouldBe StatusCode.Ok + seen(0) shouldBe empty // nothing set yet + seen(1) shouldBe Set("c0=0") + seen(2) shouldBe Set("c0=0", "c1=1") + seen(3) shouldBe Set("c0=0", "c1=1", "c2=2") + } + + test("should not carry cookies across redirects when no cookie storage is attached") { + val (backend, seen) = redirectChainSettingCookies(3) + + val result = basicRequest.get(uri"https://example.com/0").send(backend) + + result.code shouldBe StatusCode.Ok + seen.values.foreach(_ shouldBe empty) + } + } diff --git a/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala b/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala new file mode 100644 index 0000000000..4cb00c240f --- /dev/null +++ b/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala @@ -0,0 +1,61 @@ +package sttp.client4.wrappers + +import org.scalatest.funsuite.AnyFunSuite +import org.scalatest.matchers.should.Matchers +import sttp.model.headers.CookieWithMeta +import sttp.client4._ + +class CookieStorageTest extends AnyFunSuite with Matchers { + private def names(cs: Seq[CookieWithMeta]) = cs.map(_.name).toSet + + test("stores a host-only cookie and sends it back to the same host") { + val storage = CookieStorage.empty.set(uri"https://example.com/a", List(CookieWithMeta("s", "1"))) + names(storage.forUri(uri"https://example.com/b")) shouldBe Set("s") + } + + test("does not send a host-only cookie to a subdomain") { + val storage = CookieStorage.empty.set(uri"https://example.com/", List(CookieWithMeta("s", "1"))) + storage.forUri(uri"https://sub.example.com/") shouldBe empty + } + + test("sends a domain cookie to matching subdomains, but not to unrelated domains") { + val cookie = CookieWithMeta("s", "1", domain = Some("example.com")) + val storage = CookieStorage.empty.set(uri"https://example.com/", List(cookie)) + names(storage.forUri(uri"https://sub.example.com/")) shouldBe Set("s") + storage.forUri(uri"https://other.com/") shouldBe empty + } + + test("rejects a cookie whose domain does not match the setting host") { + val cookie = CookieWithMeta("s", "1", domain = Some("evil.com")) + val storage = CookieStorage.empty.set(uri"https://example.com/", List(cookie)) + storage.isEmpty shouldBe true + } + + test("respects the cookie path") { + val cookie = CookieWithMeta("s", "1", path = Some("/admin")) + val storage = CookieStorage.empty.set(uri"https://example.com/admin", List(cookie)) + names(storage.forUri(uri"https://example.com/admin/x")) shouldBe Set("s") + storage.forUri(uri"https://example.com/public") shouldBe empty + } + + test("does not send a secure cookie over http") { + val cookie = CookieWithMeta("s", "1", secure = true) + val storage = CookieStorage.empty.set(uri"https://example.com/", List(cookie)) + storage.forUri(uri"http://example.com/") shouldBe empty + names(storage.forUri(uri"https://example.com/")) shouldBe Set("s") + } + + test("a later cookie with the same name/domain/path overwrites the earlier one") { + val storage = CookieStorage.empty + .set(uri"https://example.com/", List(CookieWithMeta("s", "1"))) + .set(uri"https://example.com/", List(CookieWithMeta("s", "2"))) + storage.forUri(uri"https://example.com/").map(_.value) shouldBe Seq("2") + } + + test("a cookie with Max-Age <= 0 removes a matching stored cookie") { + val storage = CookieStorage.empty + .set(uri"https://example.com/", List(CookieWithMeta("s", "1"))) + .set(uri"https://example.com/", List(CookieWithMeta("s", "", maxAge = Some(0)))) + storage.isEmpty shouldBe true + } +} diff --git a/docs/requests/cookies.md b/docs/requests/cookies.md index 2e00870f09..89c14c77da 100644 --- a/docs/requests/cookies.md +++ b/docs/requests/cookies.md @@ -51,3 +51,20 @@ val cookiesFromResponse = response.unsafeCookies basicRequest.cookies(cookiesFromResponse) ``` + +## Cookies across redirects + +The `Cookie` header is a sensitive header, so by default it is stripped when following a redirect; cookies set via `Set-Cookie` during a redirect chain are not carried over to subsequent requests either. To opt into a cookie jar that does this, attach a `CookieStorage` to the request. The `FollowRedirectsBackend` (applied to all backends by default) then, for each request in a redirect chain, sends the stored cookies that domain/path-match the request and collects the response's `Set-Cookie` cookies into the storage: + +```scala mdoc:compile-only +import sttp.client4.* +import sttp.client4.wrappers.CookieStorage + +val backend = DefaultSyncBackend() +basicRequest + .cookieStorage(CookieStorage.empty) + .get(uri"https://endpoint.com") + .send(backend) +``` + +Matching follows a subset of [RFC 6265](https://www.rfc-editor.org/rfc/rfc6265): domain, path and the `Secure` attribute. Time-based expiry is not tracked, but a `Set-Cookie` with `Max-Age` <= 0 removes a matching cookie. From 5c023a3af614ce156d2128e77562b5464491117e Mon Sep 17 00:00:00 2001 From: adamw Date: Fri, 29 May 2026 08:14:25 +0000 Subject: [PATCH 2/6] Harvest redirect cookies without java.time parsing (Scala Native) 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) --- .../sttp/client4/wrappers/CookieStorage.scala | 36 +++++++++++++++++++ .../wrappers/FollowRedirectsBackend.scala | 6 ++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala b/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala index d1fbc59f82..22c577a1f1 100644 --- a/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala +++ b/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala @@ -41,6 +41,14 @@ final class CookieStorage private (private val entries: Map[CookieStorage.Key, C new CookieStorage(updated) } + /** As [[set]], but parses the raw `Set-Cookie` header values received from `setBy`. Used instead of + * [[sttp.model.headers.CookieWithMeta.parse]] to remain usable on Scala Native, where the date parsing that + * full cookie parsing depends on is unavailable; only the attributes relevant for storage (`Domain`, `Path`, + * `Secure`, `Max-Age`) are read, `Expires` is ignored. + */ + def setFromSetCookieHeaders(setBy: Uri, setCookieHeaders: Iterable[String]): CookieStorage = + set(setBy, setCookieHeaders.flatMap(CookieStorage.parseSetCookie)) + /** The cookies that should be sent with a request to `uri`, according to domain-matching, path-matching and the * `Secure` attribute (secure cookies are only sent over `https`). */ @@ -90,4 +98,32 @@ object CookieStorage { requestPath == cookiePath || (requestPath.startsWith(cookiePath) && (cookiePath.endsWith("/") || requestPath.charAt(cookiePath.length) == '/')) + + // A minimal `Set-Cookie` parser reading only the attributes used for storage. Unlike CookieWithMeta.parse it does + // not parse `Expires` dates, so it doesn't pull in java.time formatting (unavailable on Scala Native). + private def parseSetCookie(headerValue: String): Option[CookieWithMeta] = + headerValue.split(";").iterator.map(_.trim).filter(_.nonEmpty).toList match { + case nameValue :: directives => + val eq = nameValue.indexOf('=') + val name = if (eq < 0) nameValue else nameValue.substring(0, eq).trim + if (name.isEmpty) None + else { + val value = if (eq < 0) "" else nameValue.substring(eq + 1).trim + val attrs = directives.map { d => + val i = d.indexOf('=') + if (i < 0) (d.toLowerCase, "") else (d.substring(0, i).trim.toLowerCase, d.substring(i + 1).trim) + }.toMap + Some( + CookieWithMeta( + name = name, + value = value, + domain = attrs.get("domain").filter(_.nonEmpty), + path = attrs.get("path").filter(_.nonEmpty), + maxAge = attrs.get("max-age").flatMap(s => scala.util.Try(s.toLong).toOption), + secure = attrs.contains("secure") + ) + ) + } + case Nil => None + } } diff --git a/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala b/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala index 4c4e13d7e2..369e523c32 100644 --- a/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala +++ b/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala @@ -64,8 +64,10 @@ abstract class FollowRedirectsBackend[F[_], P] private ( */ private def updateStoredCookies[T](request: GenericRequest[T, R], response: ResponseMetadata): GenericRequest[T, R] = request.attribute(CookieStorage.attributeKey) match { - case Some(storage) => request.attribute(CookieStorage.attributeKey, storage.set(request.uri, response.unsafeCookies)) - case None => request + case Some(storage) => + val updated = storage.setFromSetCookieHeaders(request.uri, response.headers(HeaderNames.SetCookie)) + request.attribute(CookieStorage.attributeKey, updated) + case None => request } private def followRedirect[T]( From c484b42e6152c25543aa45e1b6bc996a0e50adb1 Mon Sep 17 00:00:00 2001 From: adamw Date: Fri, 29 May 2026 08:31:11 +0000 Subject: [PATCH 3/6] Avoid CookieWithMeta entirely so cookie storage links on Scala Native 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) --- .../sttp/client4/wrappers/CookieStorage.scala | 70 ++++++++++--------- .../wrappers/FollowRedirectsBackend.scala | 4 +- .../client4/FollowRedirectsBackendTest.scala | 3 +- .../client4/wrappers/CookieStorageTest.scala | 45 ++++++------ 4 files changed, 61 insertions(+), 61 deletions(-) diff --git a/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala b/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala index 22c577a1f1..625ccd074e 100644 --- a/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala +++ b/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala @@ -2,7 +2,6 @@ package sttp.client4.wrappers import sttp.attributes.AttributeKey import sttp.model.Uri -import sttp.model.headers.CookieWithMeta /** An immutable cookie jar. * @@ -16,52 +15,46 @@ import sttp.model.headers.CookieWithMeta * `Set-Cookie` during a redirect chain visible to subsequent requests in that chain - which otherwise doesn't happen, * as the `Cookie` header is a sensitive header, stripped when following redirects. * - * Time-based expiry (`Max-Age` > 0, `Expires`) is not tracked, as the storage has no notion of the current time; a - * `Set-Cookie` with `Max-Age` <= 0 removes a matching cookie, so a server can still clear cookies within a chain. + * Cookies are represented as plain name/value pairs (rather than [[sttp.model.headers.CookieWithMeta]]) so that the + * storage is usable on all platforms, including Scala Native. Time-based expiry (`Max-Age` > 0, `Expires`) is not + * tracked, as the storage has no notion of the current time; a `Set-Cookie` with `Max-Age` <= 0 removes a matching + * cookie, so a server can still clear cookies within a chain. */ final class CookieStorage private (private val entries: Map[CookieStorage.Key, CookieStorage.Stored]) { import CookieStorage._ - /** A new storage updated with the `cookies` received in a response from `setBy`. Following RFC 6265, a cookie whose - * `Domain` attribute does not domain-match `setBy` is rejected (to prevent a host setting cookies for unrelated - * domains). A cookie with `Max-Age` <= 0 removes a matching stored cookie. + /** A new storage updated with the cookies parsed from the `Set-Cookie` header values received from `setBy`. + * Following RFC 6265, a cookie whose `Domain` attribute does not domain-match `setBy` is rejected (to prevent a + * host setting cookies for unrelated domains). A cookie with `Max-Age` <= 0 removes a matching stored cookie. */ - def set(setBy: Uri, cookies: Iterable[CookieWithMeta]): CookieStorage = { + def setFromSetCookieHeaders(setBy: Uri, setCookieHeaders: Iterable[String]): CookieStorage = { val host = hostOf(setBy) - val updated = cookies.foldLeft(entries) { (acc, c) => + val updated = setCookieHeaders.flatMap(parseSetCookie).foldLeft(entries) { (acc, c) => val hostOnly = c.domain.isEmpty val domain = c.domain.map(normalizeDomain).getOrElse(host) if (domain.isEmpty || !domainMatches(host, domain)) acc else { val key = Key(c.name, domain, c.path.getOrElse(DefaultPath)) - if (c.maxAge.exists(_ <= 0)) acc - key - else acc.updated(key, Stored(c, hostOnly)) + if (c.removed) acc - key + else acc.updated(key, Stored(c.value, c.secure, hostOnly)) } } new CookieStorage(updated) } - /** As [[set]], but parses the raw `Set-Cookie` header values received from `setBy`. Used instead of - * [[sttp.model.headers.CookieWithMeta.parse]] to remain usable on Scala Native, where the date parsing that - * full cookie parsing depends on is unavailable; only the attributes relevant for storage (`Domain`, `Path`, - * `Secure`, `Max-Age`) are read, `Expires` is ignored. + /** The cookies, as `name -> value` pairs, that should be sent with a request to `uri`, according to domain-matching, + * path-matching and the `Secure` attribute (secure cookies are only sent over `https`). */ - def setFromSetCookieHeaders(setBy: Uri, setCookieHeaders: Iterable[String]): CookieStorage = - set(setBy, setCookieHeaders.flatMap(CookieStorage.parseSetCookie)) - - /** The cookies that should be sent with a request to `uri`, according to domain-matching, path-matching and the - * `Secure` attribute (secure cookies are only sent over `https`). - */ - def forUri(uri: Uri): Seq[CookieWithMeta] = { + def cookiesFor(uri: Uri): Seq[(String, String)] = { val host = hostOf(uri) val path = pathOf(uri) val secure = uri.scheme.exists(_.equalsIgnoreCase("https")) entries.collect { - case (key, Stored(cookie, hostOnly)) - if (if (hostOnly) host == key.domain else domainMatches(host, key.domain)) && + case (key, stored) + if (if (stored.hostOnly) host == key.domain else domainMatches(host, key.domain)) && pathMatches(path, key.path) && - (!cookie.secure || secure) => - cookie + (!stored.secure || secure) => + key.name -> stored.value }.toSeq } @@ -83,7 +76,18 @@ object CookieStorage { // a cookie is identified by its name, the domain it's scoped to and its path private case class Key(name: String, domain: String, path: String) - private case class Stored(cookie: CookieWithMeta, hostOnly: Boolean) + private case class Stored(value: String, secure: Boolean, hostOnly: Boolean) + + // a `Set-Cookie` cookie before its domain is resolved against the setting host; `removed` marks a Max-Age <= 0 + // deletion + private case class Parsed( + name: String, + value: String, + domain: Option[String], + path: Option[String], + secure: Boolean, + removed: Boolean + ) private def hostOf(uri: Uri): String = uri.host.getOrElse("").toLowerCase private def pathOf(uri: Uri): String = "/" + uri.path.mkString("/") @@ -99,9 +103,10 @@ object CookieStorage { (requestPath.startsWith(cookiePath) && (cookiePath.endsWith("/") || requestPath.charAt(cookiePath.length) == '/')) - // A minimal `Set-Cookie` parser reading only the attributes used for storage. Unlike CookieWithMeta.parse it does - // not parse `Expires` dates, so it doesn't pull in java.time formatting (unavailable on Scala Native). - private def parseSetCookie(headerValue: String): Option[CookieWithMeta] = + // A minimal `Set-Cookie` parser reading only the attributes used for storage. CookieWithMeta isn't used, as it isn't + // available on Scala Native (its date handling depends on java.time formatting); `Expires` is ignored for the same + // reason - storage doesn't track time-based expiry anyway. + private def parseSetCookie(headerValue: String): Option[Parsed] = headerValue.split(";").iterator.map(_.trim).filter(_.nonEmpty).toList match { case nameValue :: directives => val eq = nameValue.indexOf('=') @@ -113,14 +118,15 @@ object CookieStorage { val i = d.indexOf('=') if (i < 0) (d.toLowerCase, "") else (d.substring(0, i).trim.toLowerCase, d.substring(i + 1).trim) }.toMap + val maxAge = attrs.get("max-age").flatMap(s => scala.util.Try(s.toLong).toOption) Some( - CookieWithMeta( + Parsed( name = name, value = value, domain = attrs.get("domain").filter(_.nonEmpty), path = attrs.get("path").filter(_.nonEmpty), - maxAge = attrs.get("max-age").flatMap(s => scala.util.Try(s.toLong).toOption), - secure = attrs.contains("secure") + secure = attrs.contains("secure"), + removed = maxAge.exists(_ <= 0) ) ) } diff --git a/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala b/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala index 369e523c32..7b8ddcb95f 100644 --- a/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala +++ b/core/src/main/scala/sttp/client4/wrappers/FollowRedirectsBackend.scala @@ -54,8 +54,8 @@ abstract class FollowRedirectsBackend[F[_], P] private ( private def applyStoredCookies[T](request: GenericRequest[T, R]): GenericRequest[T, R] = request.attribute(CookieStorage.attributeKey) match { case Some(storage) => - val cookies = storage.forUri(request.uri) - if (cookies.isEmpty) request else request.cookies(cookies) + val cookies = storage.cookiesFor(request.uri) + if (cookies.isEmpty) request else request.cookies(cookies: _*) case None => request } diff --git a/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala b/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala index a8983d1717..8ea09c9d5a 100644 --- a/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala +++ b/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala @@ -5,7 +5,6 @@ import org.scalatest.funsuite.AnyFunSuite import org.scalatest.matchers.should.Matchers import sttp.client4.testing.{BackendStub, ResponseStub} import sttp.client4.wrappers.{CookieStorage, FollowRedirectsBackend, FollowRedirectsConfig} -import sttp.model.headers.CookieWithMeta import sttp.model.internal.Rfc3986 import sttp.model.{Header, HeaderNames, StatusCode, Uri} @@ -67,7 +66,7 @@ class FollowRedirectsBackendTest extends AnyFunSuite with Matchers with EitherVa ResponseStub.adjust( "", StatusCode.Found, - Vector(Header.location(url(id + 1)), Header.setCookie(CookieWithMeta(s"c$id", id.toString))) + Vector(Header.location(url(id + 1)), Header(HeaderNames.SetCookie, s"c$id=$id")) ) else ResponseStub.adjust("done", StatusCode.Ok) } diff --git a/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala b/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala index 4cb00c240f..fb67fab0cb 100644 --- a/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala +++ b/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala @@ -2,60 +2,55 @@ package sttp.client4.wrappers import org.scalatest.funsuite.AnyFunSuite import org.scalatest.matchers.should.Matchers -import sttp.model.headers.CookieWithMeta import sttp.client4._ class CookieStorageTest extends AnyFunSuite with Matchers { - private def names(cs: Seq[CookieWithMeta]) = cs.map(_.name).toSet + private def names(cs: Seq[(String, String)]) = cs.map(_._1).toSet test("stores a host-only cookie and sends it back to the same host") { - val storage = CookieStorage.empty.set(uri"https://example.com/a", List(CookieWithMeta("s", "1"))) - names(storage.forUri(uri"https://example.com/b")) shouldBe Set("s") + val storage = CookieStorage.empty.setFromSetCookieHeaders(uri"https://example.com/a", List("s=1")) + names(storage.cookiesFor(uri"https://example.com/b")) shouldBe Set("s") } test("does not send a host-only cookie to a subdomain") { - val storage = CookieStorage.empty.set(uri"https://example.com/", List(CookieWithMeta("s", "1"))) - storage.forUri(uri"https://sub.example.com/") shouldBe empty + val storage = CookieStorage.empty.setFromSetCookieHeaders(uri"https://example.com/", List("s=1")) + storage.cookiesFor(uri"https://sub.example.com/") shouldBe empty } test("sends a domain cookie to matching subdomains, but not to unrelated domains") { - val cookie = CookieWithMeta("s", "1", domain = Some("example.com")) - val storage = CookieStorage.empty.set(uri"https://example.com/", List(cookie)) - names(storage.forUri(uri"https://sub.example.com/")) shouldBe Set("s") - storage.forUri(uri"https://other.com/") shouldBe empty + val storage = CookieStorage.empty.setFromSetCookieHeaders(uri"https://example.com/", List("s=1; Domain=example.com")) + names(storage.cookiesFor(uri"https://sub.example.com/")) shouldBe Set("s") + storage.cookiesFor(uri"https://other.com/") shouldBe empty } test("rejects a cookie whose domain does not match the setting host") { - val cookie = CookieWithMeta("s", "1", domain = Some("evil.com")) - val storage = CookieStorage.empty.set(uri"https://example.com/", List(cookie)) + val storage = CookieStorage.empty.setFromSetCookieHeaders(uri"https://example.com/", List("s=1; Domain=evil.com")) storage.isEmpty shouldBe true } test("respects the cookie path") { - val cookie = CookieWithMeta("s", "1", path = Some("/admin")) - val storage = CookieStorage.empty.set(uri"https://example.com/admin", List(cookie)) - names(storage.forUri(uri"https://example.com/admin/x")) shouldBe Set("s") - storage.forUri(uri"https://example.com/public") shouldBe empty + val storage = CookieStorage.empty.setFromSetCookieHeaders(uri"https://example.com/admin", List("s=1; Path=/admin")) + names(storage.cookiesFor(uri"https://example.com/admin/x")) shouldBe Set("s") + storage.cookiesFor(uri"https://example.com/public") shouldBe empty } test("does not send a secure cookie over http") { - val cookie = CookieWithMeta("s", "1", secure = true) - val storage = CookieStorage.empty.set(uri"https://example.com/", List(cookie)) - storage.forUri(uri"http://example.com/") shouldBe empty - names(storage.forUri(uri"https://example.com/")) shouldBe Set("s") + val storage = CookieStorage.empty.setFromSetCookieHeaders(uri"https://example.com/", List("s=1; Secure")) + storage.cookiesFor(uri"http://example.com/") shouldBe empty + names(storage.cookiesFor(uri"https://example.com/")) shouldBe Set("s") } test("a later cookie with the same name/domain/path overwrites the earlier one") { val storage = CookieStorage.empty - .set(uri"https://example.com/", List(CookieWithMeta("s", "1"))) - .set(uri"https://example.com/", List(CookieWithMeta("s", "2"))) - storage.forUri(uri"https://example.com/").map(_.value) shouldBe Seq("2") + .setFromSetCookieHeaders(uri"https://example.com/", List("s=1")) + .setFromSetCookieHeaders(uri"https://example.com/", List("s=2")) + storage.cookiesFor(uri"https://example.com/") shouldBe Seq("s" -> "2") } test("a cookie with Max-Age <= 0 removes a matching stored cookie") { val storage = CookieStorage.empty - .set(uri"https://example.com/", List(CookieWithMeta("s", "1"))) - .set(uri"https://example.com/", List(CookieWithMeta("s", "", maxAge = Some(0)))) + .setFromSetCookieHeaders(uri"https://example.com/", List("s=1")) + .setFromSetCookieHeaders(uri"https://example.com/", List("s=; Max-Age=0")) storage.isEmpty shouldBe true } } From 1d45576f93d13bc30480dcc3ff87f3f419226c6e Mon Sep 17 00:00:00 2001 From: adamw Date: Fri, 29 May 2026 09:02:03 +0000 Subject: [PATCH 4/6] Address review: RFC default-path, doc fixes, more tests - 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) --- .claude/perf-notes.md | 8 +++++ .../sttp/client4/wrappers/CookieStorage.scala | 33 +++++++++++-------- .../client4/FollowRedirectsBackendTest.scala | 32 ++++++++++++++++-- .../client4/wrappers/CookieStorageTest.scala | 15 ++++++++- 4 files changed, 72 insertions(+), 16 deletions(-) create mode 100644 .claude/perf-notes.md diff --git a/.claude/perf-notes.md b/.claude/perf-notes.md new file mode 100644 index 0000000000..93b3bee012 --- /dev/null +++ b/.claude/perf-notes.md @@ -0,0 +1,8 @@ + +## CookieStorage / FollowRedirectsBackend (feature/cookie-storage-redirects) +- Hot path: FollowRedirectsBackend wraps all backends; sendWithCounter runs per request + per redirect hop. +- No-storage common case: applyStoredCookies + updateStoredCookies each do request.attribute(key) = AttributeMap.get = Map.get(typeName String) -> Option. AttributeMap.get returns Some/None then matches None. Near-zero cost (one immutable Map.get + boxing). Acceptable. +- updateStoredCookies only called on redirect responses (not common path) - fine. +- CookieStorage.cookiesFor: iterates ALL entries (no domain index) - O(n) per request; n = total cookies stored. Fine for jar use. +- Uri.path is `def` returning .toList (allocates) every call; pathOf builds "/"+mkString - allocs per cookiesFor. +- parseSetCookie: split(";") + .toList + attrs .toMap + Try(toLong) per Set-Cookie header. Only on redirect responses with storage. Try-based long parse allocs on success path too. diff --git a/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala b/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala index 625ccd074e..da28f493c0 100644 --- a/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala +++ b/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala @@ -9,7 +9,7 @@ import sttp.model.Uri * request to a given URI, applying a subset of the [[https://www.rfc-editor.org/rfc/rfc6265 RFC 6265]] rules: * domain-matching, path-matching and the `Secure` attribute. * - * Intended use: attach a storage to a request using [[sttp.client4.RequestBuilder.cookieStorage]]. The + * Intended use: attach a storage to a request using [[sttp.client4.PartialRequestBuilder.cookieStorage]]. The * [[FollowRedirectsBackend]] (applied to all backends by default) then, for each request in a redirect chain, sends * the matching stored cookies and threads an updated storage through to the next request. This makes cookies set via * `Set-Cookie` during a redirect chain visible to subsequent requests in that chain - which otherwise doesn't happen, @@ -34,7 +34,7 @@ final class CookieStorage private (private val entries: Map[CookieStorage.Key, C val domain = c.domain.map(normalizeDomain).getOrElse(host) if (domain.isEmpty || !domainMatches(host, domain)) acc else { - val key = Key(c.name, domain, c.path.getOrElse(DefaultPath)) + val key = Key(c.name, domain, c.path.getOrElse(defaultPathOf(setBy))) if (c.removed) acc - key else acc.updated(key, Stored(c.value, c.secure, hostOnly)) } @@ -49,13 +49,12 @@ final class CookieStorage private (private val entries: Map[CookieStorage.Key, C val host = hostOf(uri) val path = pathOf(uri) val secure = uri.scheme.exists(_.equalsIgnoreCase("https")) - entries.collect { - case (key, stored) - if (if (stored.hostOnly) host == key.domain else domainMatches(host, key.domain)) && - pathMatches(path, key.path) && - (!stored.secure || secure) => - key.name -> stored.value - }.toSeq + entries.collect { case (key, stored) if matches(key, stored, host, path, secure) => key.name -> stored.value }.toSeq + } + + private def matches(key: Key, stored: Stored, host: String, path: String, secure: Boolean): Boolean = { + val domainMatch = if (stored.hostOnly) host == key.domain else domainMatches(host, key.domain) + domainMatch && pathMatches(path, key.path) && (!stored.secure || secure) } def isEmpty: Boolean = entries.isEmpty @@ -67,7 +66,7 @@ object CookieStorage { val empty: CookieStorage = new CookieStorage(Map.empty) /** The attribute key under which a [[CookieStorage]] is attached to a request; see - * [[sttp.client4.RequestBuilder.cookieStorage]]. + * [[sttp.client4.PartialRequestBuilder.cookieStorage]]. */ val attributeKey: AttributeKey[CookieStorage] = new AttributeKey[CookieStorage]("sttp.client4.wrappers.CookieStorage") @@ -93,6 +92,14 @@ object CookieStorage { private def pathOf(uri: Uri): String = "/" + uri.path.mkString("/") private def normalizeDomain(d: String): String = d.stripPrefix(".").toLowerCase + // RFC 6265, 5.1.4: the default-path of a cookie without a `Path` attribute is the setting request's directory - the + // path up to, but not including, the rightmost "/" (or "/" if there is none beyond the leading one) + private def defaultPathOf(uri: Uri): String = { + val p = pathOf(uri) + val lastSlash = p.lastIndexOf('/') + if (lastSlash <= 0) DefaultPath else p.substring(0, lastSlash) + } + // RFC 6265, 5.1.3: equal, or `host` is a subdomain of `domain` private def domainMatches(host: String, domain: String): Boolean = host == domain || host.endsWith("." + domain) @@ -103,9 +110,9 @@ object CookieStorage { (requestPath.startsWith(cookiePath) && (cookiePath.endsWith("/") || requestPath.charAt(cookiePath.length) == '/')) - // A minimal `Set-Cookie` parser reading only the attributes used for storage. CookieWithMeta isn't used, as it isn't - // available on Scala Native (its date handling depends on java.time formatting); `Expires` is ignored for the same - // reason - storage doesn't track time-based expiry anyway. + // A minimal `Set-Cookie` parser reading only the attributes used for storage. CookieWithMeta.parse isn't reused + // because its `Expires` handling relies on java.time date formatting, which doesn't work on Scala Native; `Expires` + // is ignored here anyway, as the storage doesn't track time-based expiry. private def parseSetCookie(headerValue: String): Option[Parsed] = headerValue.split(";").iterator.map(_.trim).filter(_.nonEmpty).toList match { case nameValue :: directives => diff --git a/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala b/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala index 8ea09c9d5a..5c69339d84 100644 --- a/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala +++ b/core/src/test/scala/sttp/client4/FollowRedirectsBackendTest.scala @@ -6,7 +6,7 @@ import org.scalatest.matchers.should.Matchers import sttp.client4.testing.{BackendStub, ResponseStub} import sttp.client4.wrappers.{CookieStorage, FollowRedirectsBackend, FollowRedirectsConfig} import sttp.model.internal.Rfc3986 -import sttp.model.{Header, HeaderNames, StatusCode, Uri} +import sttp.model.{Header, HeaderNames, ResponseMetadata, StatusCode, Uri} class FollowRedirectsBackendTest extends AnyFunSuite with Matchers with EitherValues { val testData = List( @@ -53,6 +53,9 @@ class FollowRedirectsBackendTest extends AnyFunSuite with Matchers with EitherVa result.body.value shouldBe "All good!" } + private def cookiesIn(r: GenericRequest[_, _]): Set[String] = + r.header(HeaderNames.Cookie).map(_.split("; ").toSet).getOrElse(Set.empty) + // a redirect chain example.com/0 -> /1 -> ... -> /n, where each hop sets a cookie `c`; records the cookies // (name=value) seen in the `Cookie` header of each request, by target id private def redirectChainSettingCookies(n: Int): (SyncBackend, collection.Map[Int, Set[String]]) = { @@ -61,7 +64,7 @@ class FollowRedirectsBackendTest extends AnyFunSuite with Matchers with EitherVa val stub = BackendStub.synchronous.whenRequestMatchesPartial { case r if r.uri.host.contains("example.com") => val id = r.uri.path.last.toInt - seen(id) = r.header(HeaderNames.Cookie).map(_.split("; ").toSet).getOrElse(Set.empty) + seen(id) = cookiesIn(r) if (id < n) ResponseStub.adjust( "", @@ -94,4 +97,29 @@ class FollowRedirectsBackendTest extends AnyFunSuite with Matchers with EitherVa seen.values.foreach(_ shouldBe empty) } + test("should harvest cookies from a redirect that a backend signals by throwing") { + var atTarget = Set.empty[String] + val stub = BackendStub.synchronous.whenRequestMatchesPartial { + // first hop: signal the redirect (with a Set-Cookie) by throwing, as some backends do + case r if r.uri == uri"https://example.com/0" => + throw ResponseException.UnexpectedStatusCode( + "", + ResponseMetadata( + StatusCode.Found, + "", + Vector(Header.location(uri"https://example.com/1"), Header(HeaderNames.SetCookie, "s=1")) + ) + ) + case r if r.uri == uri"https://example.com/1" => + atTarget = cookiesIn(r) + ResponseStub.adjust("done", StatusCode.Ok) + } + val backend = FollowRedirectsBackend(stub) + + val result = basicRequest.get(uri"https://example.com/0").cookieStorage(CookieStorage.empty).send(backend) + + result.code shouldBe StatusCode.Ok + atTarget shouldBe Set("s=1") + } + } diff --git a/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala b/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala index fb67fab0cb..b931726dc3 100644 --- a/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala +++ b/core/src/test/scala/sttp/client4/wrappers/CookieStorageTest.scala @@ -28,12 +28,25 @@ class CookieStorageTest extends AnyFunSuite with Matchers { storage.isEmpty shouldBe true } - test("respects the cookie path") { + test("normalizes the cookie domain (leading dot, case)") { + val storage = + CookieStorage.empty.setFromSetCookieHeaders(uri"https://example.com/", List("s=1; Domain=.EXAMPLE.com")) + names(storage.cookiesFor(uri"https://sub.example.com/")) shouldBe Set("s") + } + + test("respects the cookie path, requiring a path-segment boundary") { val storage = CookieStorage.empty.setFromSetCookieHeaders(uri"https://example.com/admin", List("s=1; Path=/admin")) names(storage.cookiesFor(uri"https://example.com/admin/x")) shouldBe Set("s") + storage.cookiesFor(uri"https://example.com/administrator") shouldBe empty // prefix, but not a segment boundary storage.cookiesFor(uri"https://example.com/public") shouldBe empty } + test("defaults a path-less cookie to the setting request's directory (RFC 6265 5.1.4)") { + val storage = CookieStorage.empty.setFromSetCookieHeaders(uri"https://example.com/admin/page", List("s=1")) + names(storage.cookiesFor(uri"https://example.com/admin/other")) shouldBe Set("s") + storage.cookiesFor(uri"https://example.com/elsewhere") shouldBe empty + } + test("does not send a secure cookie over http") { val storage = CookieStorage.empty.setFromSetCookieHeaders(uri"https://example.com/", List("s=1; Secure")) storage.cookiesFor(uri"http://example.com/") shouldBe empty From 5b51d37f0e49bd483cf13f9d78e10537d2ed0c8a Mon Sep 17 00:00:00 2001 From: Adam Warski Date: Fri, 29 May 2026 16:40:22 +0200 Subject: [PATCH 5/6] Delete .claude/perf-notes.md --- .claude/perf-notes.md | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 .claude/perf-notes.md diff --git a/.claude/perf-notes.md b/.claude/perf-notes.md deleted file mode 100644 index 93b3bee012..0000000000 --- a/.claude/perf-notes.md +++ /dev/null @@ -1,8 +0,0 @@ - -## CookieStorage / FollowRedirectsBackend (feature/cookie-storage-redirects) -- Hot path: FollowRedirectsBackend wraps all backends; sendWithCounter runs per request + per redirect hop. -- No-storage common case: applyStoredCookies + updateStoredCookies each do request.attribute(key) = AttributeMap.get = Map.get(typeName String) -> Option. AttributeMap.get returns Some/None then matches None. Near-zero cost (one immutable Map.get + boxing). Acceptable. -- updateStoredCookies only called on redirect responses (not common path) - fine. -- CookieStorage.cookiesFor: iterates ALL entries (no domain index) - O(n) per request; n = total cookies stored. Fine for jar use. -- Uri.path is `def` returning .toList (allocates) every call; pathOf builds "/"+mkString - allocs per cookiesFor. -- parseSetCookie: split(";") + .toList + attrs .toMap + Try(toLong) per Set-Cookie header. Only on redirect responses with storage. Try-based long parse allocs on success path too. From abacbc7811f11b66c25a4c0c4ad82fc106fd4d8c Mon Sep 17 00:00:00 2001 From: adamw Date: Fri, 29 May 2026 16:38:54 +0000 Subject: [PATCH 6/6] Clarify CookieStorage doc: why CookieWithMeta is avoided on Native 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) --- .../scala/sttp/client4/wrappers/CookieStorage.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala b/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala index da28f493c0..e12806dca0 100644 --- a/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala +++ b/core/src/main/scala/sttp/client4/wrappers/CookieStorage.scala @@ -15,10 +15,12 @@ import sttp.model.Uri * `Set-Cookie` during a redirect chain visible to subsequent requests in that chain - which otherwise doesn't happen, * as the `Cookie` header is a sensitive header, stripped when following redirects. * - * Cookies are represented as plain name/value pairs (rather than [[sttp.model.headers.CookieWithMeta]]) so that the - * storage is usable on all platforms, including Scala Native. Time-based expiry (`Max-Age` > 0, `Expires`) is not - * tracked, as the storage has no notion of the current time; a `Set-Cookie` with `Max-Age` <= 0 removes a matching - * cookie, so a server can still clear cookies within a chain. + * Cookies are represented as plain name/value pairs rather than [[sttp.model.headers.CookieWithMeta]]. That type is + * available on all platforms, but its `Set-Cookie` rendering and parsing reach `java.time` date formatting (for + * `Expires`, via `ZoneId`/`DateTimeFormatter`), a subset of `java.time` not supported on Scala Native; referencing it + * from this shared code pulls those symbols in and breaks the Native link. Time-based expiry (`Max-Age` > 0, `Expires`) + * is not tracked anyway, as the storage has no notion of the current time; a `Set-Cookie` with `Max-Age` <= 0 removes a + * matching cookie, so a server can still clear cookies within a chain. */ final class CookieStorage private (private val entries: Map[CookieStorage.Key, CookieStorage.Stored]) { import CookieStorage._