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

What I did to avoid the duplicate cookie problem #1258

Merged
merged 3 commits into from
Aug 14, 2016
Merged

What I did to avoid the duplicate cookie problem #1258

merged 3 commits into from
Aug 14, 2016

Conversation

bitemyapp
Copy link
Contributor

Putting this up because this is what we're still using at the moment, not sure you remember that conversation or not.

From this: #1247

@snoyberg
Copy link
Member

CC @MaxGabriel

@bitemyapp
Copy link
Contributor Author

@snoyberg By the by, part of the reason I want this out of our packages: listing in our stack.yaml is that it seems like Stack ends up rebuilding everything in our packages list unconditionally, even stuff with extra-dep and that I know is cached. Any idea what might be going on there?

@MaxGabriel
Copy link
Member

I think this PR matches the behavior recommended in RFC 6265:

Servers SHOULD NOT include more than one Set-Cookie header field in
the same response with the same cookie-name. (See Section 5.2 for
how user agents handle this case.)

And it solves the issue, so I think this should be merged, and #1248 should be merged as well

@bitemyapp
Copy link
Contributor Author

@snoyberg 👍 to what @MaxGabriel said :)

@snoyberg
Copy link
Member

Go for it if I don't get too it first. I'm on quasi vacation right now, so
I might be slower than usual.

On Sat, Aug 13, 2016, 6:06 AM Chris Allen notifications@github.com wrote:

@snoyberg https://github.com/snoyberg 👍 to what @MaxGabriel
https://github.com/MaxGabriel said :)


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#1258 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADBB9GVa2auw-o9SpIlOepRHhHaXx16ks5qfTSrgaJpZM4Jgpqk
.

@bitemyapp
Copy link
Contributor Author

@snoyberg I don't have commit bit, do you @MaxGabriel?

@MaxGabriel MaxGabriel merged commit 9fb876e into yesodweb:master Aug 14, 2016
@MaxGabriel
Copy link
Member

Ok, both PRs are merged now, had to fix a changelog conflict in mine.

@lloucas-imvu
Copy link

I think this PR matches the behavior recommended in RFC 6265:

Servers SHOULD NOT include more than one Set-Cookie header field in
the same response with the same cookie-name. (See Section 5.2 for
how user agents handle this case.)
And it solves the issue, so I think this should be merged, and #1248 should be merged as well

headerToPair (DeleteCookie key path) =

It seems to me that this explicitly does include 2 Set-Cookie header fields with the same name.
In fact I am seeing this behaviour after updating to yesod-core 1.4.24

< Set-Cookie: sandboxCookie=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT
< Set-Cookie: sandboxCookie=12345; Path=/; Expires=Wed, 01-Jan-2020 00:00:00 GMT; Domain=.d.c
< Set-Cookie: sandboxCookie-id=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT
< Set-Cookie: sandboxCookie-id=12345; Path=/; Expires=Wed, 01-Jan-2020 00:00:00 GMT; Domain=.d.c

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

Successfully merging this pull request may close these issues.

None yet

4 participants