-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Full cookie serialization #473
Conversation
@ketzusaka Do you know which browsers don't work with the single set-cookie header? |
@tannernelson I tested specifically on Chrome. I don't believe it was working in safari either as things weren't operating as I expected there, but I'd have to double check on that. Also, the format for the Expires attribute appears to require a format that would break multiple cookies in a single Set-Cookie header, and that is why it is recommended to use multiple header rows. |
@ketzusaka @tannernelson I added a mini commit on here that will give us multi cookie support until we implement something more comprehensive. This was tested on Chrome and Safari 👍 |
oh nice, surprised that works haha |
This PR moves the responsibility of serializing cookie data to the
Cookie
struct. This is a better separation of concerns in my eyes. TheCookies
serialization is updated to use the new method onCookie
, which fixes an issue where the attributes of a cookie were never set.There is still the issue where we're folding all of the cookie properties into a single Set-Cookie header, which doesn't seem to work in some browsers; that fix will come in a subsequent PR.