Skip to content

Fix Set-Cookie name validation - #7522

Open
gwagjiug wants to merge 1 commit into
Effect-TS:mainfrom
gwagjiug:fix/cookie-name-validation
Open

Fix Set-Cookie name validation#7522
gwagjiug wants to merge 1 commit into
Effect-TS:mainfrom
gwagjiug:fix/cookie-name-validation

Conversation

@gwagjiug

Copy link
Copy Markdown

Type

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Cookies.fromSetCookie validated parsed cookie names with fieldContentRegExp, which allowed invalid names containing characters such as spaces. This was inconsistent with Cookies.makeCookie, which already validates names using the RFC 6265 token syntax.

This PR:

  • uses cookieNameRegExp when parsing Set-Cookie headers;
  • ignores cookies with invalid names;
  • preserves valid cookies received alongside invalid ones;
  • adds focused regression coverage;
  • adds an effect patch changeset.

Validation

  • pnpm test --run packages/effect/test/unstable/http/Cookies.test.ts — 10 tests passed
  • NODE_OPTIONS=--experimental-strip-types pnpm lint-fix
  • pnpm check

Related

No related issue.

@changeset-bot

changeset-bot Bot commented Aug 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1d729fc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
effect Patch
@effect/ai-anthropic Patch
@effect/ai-openai Patch
@effect/ai-openai-compat Patch
@effect/ai-openrouter Patch
@effect/atom-react Patch
@effect/atom-solid Patch
@effect/atom-vue Patch
@effect/docgen Patch
@effect/doctest Patch
@effect/openapi-generator Patch
@effect/opentelemetry Patch
@effect/platform-browser Patch
@effect/platform-bun Patch
@effect/platform-deno Patch
@effect/platform-node Patch
@effect/platform-node-shared Patch
@effect/sql-clickhouse Patch
@effect/sql-d1 Patch
@effect/sql-libsql Patch
@effect/sql-mssql Patch
@effect/sql-mysql2 Patch
@effect/sql-pg Patch
@effect/sql-pglite Patch
@effect/sql-sqlite-bun Patch
@effect/sql-sqlite-do Patch
@effect/sql-sqlite-node Patch
@effect/sql-sqlite-react-native Patch
@effect/sql-sqlite-wasm Patch
@effect/vitest Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@effect-slopcop effect-slopcop Bot added bug Something isn't working 4.0 labels Aug 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Bundle Size Analysis

Generated from PR build output; treat the content below as untrusted.

File Name Current Size Previous Size Difference
arbitrary-combinators.ts 33.76 KB 33.76 KB 0.00 KB (0.00%)
basic.ts 7.00 KB 7.00 KB 0.00 KB (0.00%)
batching.ts 9.87 KB 9.87 KB 0.00 KB (0.00%)
brand.ts 6.56 KB 6.56 KB 0.00 KB (0.00%)
cache.ts 10.74 KB 10.74 KB 0.00 KB (0.00%)
config.ts 21.45 KB 21.45 KB 0.00 KB (0.00%)
differ.ts 20.32 KB 20.32 KB 0.00 KB (0.00%)
http-client.ts 21.77 KB 21.76 KB +0.01 KB (+0.04%)
logger.ts 10.94 KB 10.94 KB 0.00 KB (0.00%)
metric.ts 8.98 KB 8.98 KB 0.00 KB (0.00%)
optic.ts 6.73 KB 6.73 KB 0.00 KB (0.00%)
pubsub.ts 15.12 KB 15.12 KB 0.00 KB (0.00%)
queue.ts 11.84 KB 11.84 KB 0.00 KB (0.00%)
schedule.ts 10.86 KB 10.86 KB 0.00 KB (0.00%)
schema-binary.ts 39.42 KB 39.42 KB 0.00 KB (0.00%)
schema-class.ts 20.07 KB 20.07 KB 0.00 KB (0.00%)
schema-fromJsonSchemaDocument.ts 31.02 KB 31.02 KB 0.00 KB (0.00%)
schema-representation-roundtrip.ts 26.21 KB 26.21 KB 0.00 KB (0.00%)
schema-string-transformation.ts 13.66 KB 13.66 KB 0.00 KB (0.00%)
schema-string.ts 11.18 KB 11.18 KB 0.00 KB (0.00%)
schema-template-literal.ts 15.47 KB 15.47 KB 0.00 KB (0.00%)
schema-toArbitrary.ts 33.31 KB 33.31 KB 0.00 KB (0.00%)
schema-toCodeDocument.ts 24.51 KB 24.51 KB 0.00 KB (0.00%)
schema-toCodecJson.ts 19.29 KB 19.29 KB 0.00 KB (0.00%)
schema-toEquivalence.ts 19.42 KB 19.42 KB 0.00 KB (0.00%)
schema-toFormatter.ts 19.51 KB 19.51 KB 0.00 KB (0.00%)
schema-toJsonSchemaDocument.ts 23.45 KB 23.45 KB 0.00 KB (0.00%)
schema-toRepresentation.ts 19.56 KB 19.56 KB 0.00 KB (0.00%)
schema.ts 19.28 KB 19.28 KB 0.00 KB (0.00%)
stm.ts 12.74 KB 12.74 KB 0.00 KB (0.00%)
stream.ts 9.83 KB 9.83 KB 0.00 KB (0.00%)

@gwagjiug

gwagjiug commented Aug 29, 2026

Copy link
Copy Markdown
Author

PR Change Explanation

This PR corrects the validation rule applied to cookie names when converting HTTP response Set-Cookie headers into a Cookies collection. The core change is to apply the same RFC 6265 token syntax already used by Cookies.makeCookie and the serialization path to cookies parsed from responses. As a result, only entries with invalid names are ignored, while valid cookies from the same response are preserved.

Background

Background for readers new to this area

HTTP cookies use different headers depending on the direction of communication. A server sends cookies through Set-Cookie response headers, and a client later returns stored values through the Cookie request header. This PR concerns the former boundary: converting string-based Set-Cookie response values into Effect’s structured cookie representation.

An Effect Cookies value wraps a readonly record keyed by cookie name. Cookies.fromSetCookie iterates over one or more Set-Cookie strings, parses each one with the internal parseSetCookie function, and passes successfully parsed Cookie values to fromIterable to build that record. This path is used by the Web Response-based HttpClientResponse.cookies, the Node HTTP client’s cookies getter, and JSON decoding through CookiesSchema.

Cookie names are not arbitrary strings. This module represents the RFC 6265 token character set allowed in cookie names with cookieNameRegExp. Cookies.makeCookie, which creates cookies directly, and serializeCookie, which converts them back into Set-Cookie strings, already used this rule through validateCookie.

Background specific to this change

Before this change, only parseSetCookie validated names with fieldContentRegExp. That regular expression accepts a broad range of header characters, including tabs and spaces, so a name containing a space, such as bad name in bad name=value, was successfully parsed.

As a result, the same cookie name behaved differently depending on how it entered the system:

  • Cookies.makeCookie("bad name", "value") rejects the name with InvalidCookieName.
  • Before this change, Cookies.fromSetCookie("bad name=value") added the cookie to the collection.
  • If that collection was serialized again with toSetCookieHeaders, serializeCookie validated the same name later in the flow.

In other words, the input parser was more permissive than the construction and serialization paths, so the cookie-name invariant inside a Cookies collection could vary depending on how the cookie was created.

Intuition

The core idea is to apply the same cookie-name rule before a cookie enters the collection, regardless of where it came from. There is no reason for directly created cookies and cookies read from HTTP responses to use different name rules, so the parser now reuses the existing cookieNameRegExp.

Small example

Suppose a server sends these two headers:

Set-Cookie: bad name=value
Set-Cookie: session=abc
Step Before After
Parse bad name=value Passes the broad fieldContentRegExp Fails cookieNameRegExp
Parse session=abc Succeeds Succeeds
Final collection Contains both bad name and session Contains only session

The flow after this change is:

flowchart LR
  A[Set-Cookie header list] --> B[Split each header into name and value]
  B --> C{Does the name satisfy the RFC 6265 token syntax?}
  C -- No --> D[Ignore only that entry]
  C -- Yes --> E[Create Cookie]
  E --> F[Store in Cookies collection]

When parseSetCookie returns undefined, the existing fromSetCookie loop simply does not add that entry. Therefore, this change does not require a new error channel or additional control flow. It reuses the existing behavior of skipping entries that cannot be parsed.

Code walkthrough

1. Unifying the cookie-name invariant at the input boundary

In packages/effect/src/unstable/http/Cookies.ts, parseSetCookie extracts the cookie name from the portion before the first =. This PR changes the following validation step to:

if (!cookieNameRegExp.test(name)) {
  return undefined
}

With this one-line change, the response parser uses the same name rule as validateCookie. If validation fails, parseSetCookie returns undefined as before, and its caller, fromSetCookie, does not add that entry to the cookies array. The parsing behavior for values and attributes such as Domain, Path, Max-Age, HttpOnly, Secure, and SameSite remains unchanged.

2. Giving response consumers and Schema paths the same collection contract

HttpClientResponse.cookies and the Node HTTP client’s cookies getter pass response Set-Cookie values to Cookies.fromSetCookie and cache the result. After this change, entries with invalid names are excluded at the input boundary before these consumers receive the resulting Cookies collection.

CookiesSchema also uses fromSetCookie when decoding an array of JSON strings and toSetCookieHeaders when encoding. Because parsing now uses the same cookieNameRegExp as makeCookie and serializeCookie, decoding and subsequent encoding share the same cookie-name validation rule. No public types or function signatures are changed.

3. The observable contract established by the regression test

The new fromSetCookie test in packages/effect/test/unstable/http/Cookies.test.ts provides an invalid cookie and a valid cookie in the same input list.

  • It verifies that Cookies.get(cookies, "bad name") returns None, establishing that an invalid cookie name does not enter the collection.
  • It verifies that Cookies.getValue(cookies, "session") returns "abc", establishing that one invalid entry does not cause valid entries from the same input to be discarded.

.changeset/strict-cookie-names.md records this consumer-visible behavior as a patch change for the effect package. No API usage or migration is required; the release note documents that invalid cookie names previously accepted from Set-Cookie headers are now ignored.

<!-- explain-my-pr:v1 key=#7522 head=1d729fc33ce1a9f28f9305efa86eb2e1f71d0e93 part=1/1 -->

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.0 bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant