Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ssr): duplicate custom headers #12518

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

ematipico
Copy link
Member

@ematipico ematipico commented Nov 25, 2024

Changes

Closes #12419
Closes PLT-2669

This PR changes the merging strategy of the headers in SSR when we render an error page. It relies on a Map to remove the duplicates.

Testing

Added a new test

Docs

N/A

Copy link

changeset-bot bot commented Nov 25, 2024

🦋 Changeset detected

Latest commit: 12ed91c

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

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

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Nov 25, 2024
Copy link

codspeed-hq bot commented Nov 25, 2024

CodSpeed Performance Report

Merging #12518 will not alter performance

Comparing fix/duplicate-headers (12ed91c) with main (b140a3f)

Summary

✅ 6 untouched benchmarks

@ascorbic
Copy link
Contributor

I'm not sure if it's likely to come up here, but duplicate headers are legal, so the original headers may already have duplicates that would be stripped here

@ematipico
Copy link
Member Author

I'm not sure if it's likely to come up here, but duplicate headers are legal, so the original headers may already have duplicates that would be stripped here

They are, but once they are "compiled", the key is one their values are concatenated into a single string divided by comma, correct?

This means that the original response will already have the duplicate headers "compiled", correct?

@ascorbic
Copy link
Contributor

I thought that just applied to get(), but I see it's for iterators too, so I guess not a problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Catch-all routes duplicates custom headers
3 participants