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

Set-Cookie Headers behavior #294

Closed
ksmithut opened this issue Jul 6, 2022 · 7 comments
Closed

Set-Cookie Headers behavior #294

ksmithut opened this issue Jul 6, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@ksmithut
Copy link

ksmithut commented Jul 6, 2022

The behavior of setting cookies with the Headers type does not allow you to set multiple cookies. See this issue for undici (node's implementation of fetch) for what they might be doing about it. Deno seems to behave the way that will work with browsers (setting a new header entry for each set-cookie) and it seems like that's the only header that needs to behave that way, which is why it seems that Headers.prototype.getSetCookie is a proposal to solve for this issue.

@cyco130
Copy link

cyco130 commented Jul 20, 2022

+1 for this. We have a project called HatTip that aims to be a universal JavaScript HTTP server library/framework. This is one of the two failing cases in Bun integration (the other is related to streaming).

@harryrabin
Copy link

+1 as well. We need Deno's implementation of Header. I know it technically goes against the spec but until there are any implementors of getSetCookie we're out of luck. @cyco130 I'm very interested in your project but I like the idea of having native support to build things around the Zig-based Bun.serve.

@Lightnet
Copy link

You can try npm cookie that work okay. https://www.npmjs.com/package/cookie

const headers = new Headers();
headers.append('Set-Cookie',cookie.serialize())
new response("test",{headers})
```
Haven't try add more cookies but should work. I read it somewhere in web api that follow the format. Unless I am wrong.

@ksmithut
Copy link
Author

@Lightnet Headers.prototype.append(name, value) per the fetch spec will append the second cookie to the existing header value, so that when the headers are sent to the client, it sends a header like Set-Cookie: {cookie1}, {cookie2} but browsers can't read multiple cookies from one header. The syntax is to ambiguous as the comma could be read as part of the first cookie value. The HTTP/Browser spec that is used expects separate Set-Cookie headers for each cookie. This wasn't a problem with the fetch spec because fetch was originally meant to only be used in client JavaScript and it didn't need to set cookies.

ariesclark added a commit to ariesclark/oily.js that referenced this issue Jul 31, 2022
@harryrabin
Copy link

@ksmithut My thoughts exactly, couldn't have said it better!

@Electroid
Copy link
Contributor

There seems to be two reasonable solutions to this issue.

  1. getAll() but restrict to Set-Cookie. (implemented by Cloudflare)
interface Headers {
  getAll(name: "Set-Cookie"): string[]; 
}
  1. getSetCookie() (implemented by... no one?)
interface Headers {
  setSetCookie(): string[];
}

Personally, I think getAll() is more elegant than getSetCookie. It's odd to have a "get" "set".

@Electroid Electroid added enhancement New feature or request web-api labels Nov 1, 2022
@github-actions github-actions bot removed the web-api label Nov 2, 2022
@Jarred-Sumner
Copy link
Collaborator

Both getAll() and getSetCookie() have been implemented, along with support for iterating and toJSON()

d84f79b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants