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.

Sign up for GitHub

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

Jump to bottom

cookies inside endpoints called from event.fetch are not correctly set #9901

Closed
david-plugge opened this issue May 10, 2023 · 2 comments · Fixed by #9908
Closed

cookies inside endpoints called from event.fetch are not correctly set #9901

david-plugge opened this issue May 10, 2023 · 2 comments · Fixed by #9908

Comments

@david-plugge
Copy link
Contributor

Describe the bug

When calling an endpoint from event.fetch that sets a cookie with httpOnly: false the actual response sets the cookie with httpOnly: true

Reproduction

https://github.com/david-plugge-reproductions/sveltekit-fetch-httponly-cookie

Logs

No response

System Info

System:
  OS: Linux 5.15 Ubuntu 22.04.2 LTS 22.04.2 LTS (Jammy Jellyfish)
  CPU: (12) x64 AMD Ryzen 5 5600X 6-Core Processor
  Memory: 11.99 GB / 15.57 GB
  Container: Yes
  Shell: 5.8.1 - /usr/bin/zsh
Binaries:
  Node: 18.16.0 - /mnt/wslg/runtime-dir/fnm_multishells/197_1683713854937/bin/node
  npm: 9.5.1 - /mnt/wslg/runtime-dir/fnm_multishells/197_1683713854937/bin/npm

Severity

blocking all usage of SvelteKit

Additional Information

No response
@elliott-with-the-longest-name-on-github
Copy link
Contributor

I have this fixed locally -- just having to think through the ramifications. I'll try to get a PR open tomorrow.

@elliott-with-the-longest-name-on-github
Copy link
Contributor

elliott-with-the-longest-name-on-github commented May 11, 2023 •
edited by Conduitry
Loading

Here's a draft PR: #9908

I'm not sure if it'll actually get merged (I have a couple of concerns about the implementation that I'm discussing with Rich), but it at least identifies and documents the issue.

Merged
5 tasks
dummdidumm pushed a commit that referenced this issue May 12, 2023
@elliott-with-the-longest-name-on-github
fix: Don't set defaults when parsing cookies from headers (#9908)
26d2b7f 
Closes #9901

Bypasses setting Kit defaults on cookies parsed from fetch response headers. To illustrate what the problem was:

- Set a cookie in +server.ts: cookies.set('foo', 'bar', { httpOnly: false })
- fetch this endpoint from +page.server.ts
- As part of fetch, parse the cookies from the headers
- httpOnly isn't set (because false === unset)
- Adding this cookie to the cookies causes the default httpOnly: true to be added

The better solution here is not to apply defaults to cookies we parse from fetch response headers. If these cookies were set through cookies.set in a +server.ts endpoint, we've already set the defaults, and if they're not, we shouldn't touch them anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Don't set defaults when parsing cookies from headers sveltejs/kit
2 participants
@david-plugge @elliott-with-the-longest-name-on-github