Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

fix(http): prevent caching of responses with Set-Cookie headers#69385

Merged
kirjs merged 1 commit into
angular:mainangular/angular:mainfrom
SkyZeroZx:fix-http/here-we-go-againSkyZeroZx/angular:fix-http/here-we-go-againCopy head branch name to clipboard
Jun 24, 2026
Merged

fix(http): prevent caching of responses with Set-Cookie headers#69385
kirjs merged 1 commit into
angular:mainangular/angular:mainfrom
SkyZeroZx:fix-http/here-we-go-againSkyZeroZx/angular:fix-http/here-we-go-againCopy head branch name to clipboard

Conversation

@SkyZeroZx

Copy link
Copy Markdown
Contributor

Skip HttpTransferCache serialization for HTTP responses that contain a Set-Cookie header.

Cookie-setting responses commonly represent session-specific, user-specific, or security-sensitive state. Serializing their bodies into SSR TransferState can embed sensitive data into the generated HTML, where it may be reused during hydration or replayed by a shared cache/CDN.

See

Skip HttpTransferCache serialization for HTTP responses that contain a
Set-Cookie header.

Cookie-setting responses commonly represent session-specific,
user-specific, or security-sensitive state. Serializing their bodies into
SSR TransferState can embed sensitive data into the generated HTML, where
it may be reused during hydration or replayed by a shared cache/CDN.
@pullapprove pullapprove Bot requested a review from kirjs June 17, 2026 05:00
@angular-robot angular-robot Bot added the area: common/http Issues related to HTTP and HTTP Client label Jun 17, 2026
@ngbot ngbot Bot added this to the Backlog milestone Jun 17, 2026
Comment thread packages/common/http/src/transfer_cache.ts

@alan-agius4 alan-agius4 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems redundant to me unless you can reproduce it.

From the spec, Set-Cookie is automatically stripped on Node.js unless credentials is set to include.

@SkyZeroZx

SkyZeroZx commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

This change seems redundant to me unless you can reproduce it.

From the spec, Set-Cookie is automatically stripped on Node.js unless credentials is set to include.

@alan-agius4 This is about using Set-Cookie server-side as a signal that the response body may be session-specific and should not be serialized into TransferState, similar to how CDNs such as Cloudflare and Google Cloud CDN treat responses with Set-Cookie.

Here is a minimal Node.js example showing that the Set-Cookie header is preserved server-side in all cases:

https://gist.github.com/SkyZeroZx/c889f14f983739c67339dc4195807b34

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Jun 18, 2026
@alan-agius4 alan-agius4 requested review from JeanMeche and removed request for kirjs June 18, 2026 08:12
@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: merge The PR is ready for merge by the caretaker labels Jun 18, 2026
@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 18, 2026
@SkyZeroZx

Copy link
Copy Markdown
Contributor Author

Considering that this is similar to GHSA-q6f4-qqrg-jv6x, and following the same approach as skip transfer cache for credentialed requests and uncacheable request/responses

I think we should also backport this PR

@kirjs

kirjs commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

This PR was merged into the repository. The changes were merged into the following branches:

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

Labels

action: merge The PR is ready for merge by the caretaker area: common/http Issues related to HTTP and HTTP Client target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.