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

Cannot set set-cookie response headers multiple times #6032

Closed
SylvainGarrigues opened this issue Aug 18, 2022 · 6 comments · Fixed by #6033
Closed

Cannot set set-cookie response headers multiple times #6032

SylvainGarrigues opened this issue Aug 18, 2022 · 6 comments · Fixed by #6033
Labels
bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.

Comments

@SylvainGarrigues
Copy link

Describe the bug

Since the setHeaders takes a POJO and also checks that a response header as not been already set, one cannot set multiple cookies with multiple set-cookie headers.

Reproduction

Source code for setHeaders checks whether an header is already been set, so can’t even call it multiple times.

Logs

No response

System Info

Any system

Severity

serious, but I can work around it

Additional Information

No response

@dummdidumm dummdidumm changed the title Cannot set response headers (e.g. set-cookie) multiple times Cannot set set-cookie response headers multiple times Aug 18, 2022
@dummdidumm
Copy link
Member

set-cookie is a special case because it's the only header that allows an array input through the setHeaders method. I'd say it's sensible to support that use case and merge the arrays and check no entry appears twice.

@dummdidumm dummdidumm added feature request New feature or request p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. labels Aug 18, 2022
@Conduitry
Copy link
Member

If this is referring to the load argument setHeaders, the docs do already call that out as a specially handled case https://kit.svelte.dev/docs/load#input-methods-setheaders - I guess it's not actually working that way?

@dummdidumm dummdidumm added bug Something isn't working and removed feature request New feature or request labels Aug 18, 2022
@dummdidumm
Copy link
Member

You are right, then it's a bug 😄

@SylvainGarrigues
Copy link
Author

SylvainGarrigues commented Aug 18, 2022

My bad, set-cookie is indeed handled as a special case correctly if I pass in an array like the doc says, leading in multiple set-cookie being set. Sorry for the annoyance.

@Conduitry
Copy link
Member

It's good that the array version is working, but the docs do say that you should be able to call it multiple times as well. Reopening.

@Conduitry Conduitry reopened this Aug 18, 2022
@SylvainGarrigues
Copy link
Author

As far as I am concerned, not being able to call it multiple times is not an issue (I prefer the array version), and we could just fix the doc :) But it's not my call to decide. Thanks anyway for your very prompt feedback.

dummdidumm added a commit that referenced this issue Aug 18, 2022
Rich-Harris added a commit that referenced this issue Aug 18, 2022
* [fix] handle set-cookie in setHeaders

Fixes #6032

* Update packages/kit/src/runtime/server/index.js

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* Update packages/kit/src/runtime/server/index.js

* separate out cookie handling more explicitly

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
Co-authored-by: Rich Harris <hello@rich-harris.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants