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

Support for sameSite === 'Unset' #100

Closed
AKushWarrior opened this issue Mar 12, 2020 · 8 comments
Closed

Support for sameSite === 'Unset' #100

AKushWarrior opened this issue Mar 12, 2020 · 8 comments

Comments

@AKushWarrior
Copy link

AKushWarrior commented Mar 12, 2020

Various sites (ex. https://slader.com) don't set the sameSite cookie attribute, and instead leave it as 'Unset'. This currently tosses a TypeError, whereas behavior should be the same as 'none'.

@AKushWarrior AKushWarrior changed the title Support for sameSite === Support for sameSite === 'Unset' Mar 12, 2020
@dougwilson
Copy link
Contributor

I'm not sure you are posting on the right repo. This module allow you to send either none or simply not set, neither throws a TypeError.

If you can provide the exact steps to reproduce the error, the version of this module you are using, and the exact error message, I can always reopen to investigate.

@AKushWarrior
Copy link
Author

AKushWarrior commented Mar 12, 2020

Essentially, if you parse a cookie from a site (slader.com, as I mentioned above) and try to send a modified version, the serialize function fails. It does so because of this switch statement. I'm proposing adding 'Unset' as a case in the switch statement, so that trying to send the same cookie doesn't fail.

@dougwilson
Copy link
Contributor

I'm not sure I understand what you mean by "if you parse a cookie from a site", can you elaborate? This module only parses the "Cookie" header (NOT the "Set-Cookie" header)...

@AKushWarrior
Copy link
Author

Sorry, let me clear some things up. I'm using the serialize function to serialize a set-cookie header. This header is read from a crawled site and has to be re-serialized with certain changed attributes. I'm not modifying the 'same-site' field, so I didn't expect this package to error when I attempted to re-serialize the header.

Using 'parse' was a total mental error by me. I've been mixing and matching on npm for a while now, and I got my terminology mixed up.

@dougwilson
Copy link
Contributor

dougwilson commented Mar 12, 2020

So this module will error on by design if the value you pass to sameSite option is not valid according to the specification of the Set-Cookie header. Having no SameSite attribute is valid, of course, and the API of this module is to not provide the sameSite option in that case (or use sameSite: undefined, sameSite: null, sameSite: false, etc.).

@AKushWarrior
Copy link
Author

https://tools.ietf.org/id/draft-ietf-httpbis-rfc6265bis-03.html#the-samesite-attribute Take a look at the last sentence of this. Values that are not listed should be treated as 'None', not an error, so as to maintain support for broken websites.

@dougwilson
Copy link
Contributor

That wording is in regards to parsing the attribute in the Set-Cookie header. It means your parsing code should be generating { sameSite: 'None' } when encountering SameSite=Unset on a Set-Cookie header.

@AKushWarrior
Copy link
Author

Okay, I see now. This PR and issue should be for my parsing library :/

Have a good day!

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

No branches or pull requests

2 participants