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

Avoid sending cookie attributes in Cookie header #613

Merged
merged 1 commit into from Dec 18, 2015

Conversation

@neumond
Copy link
Contributor

@neumond neumond commented Nov 2, 2015

Current implementation sends attributes of every cookie key-value pair in Cookie HTTP header:

key=value;
httponly;
Path=/;
secure;
key=value;
httponly;
Path=/;
secure;
key=value;
httponly;
Path=/;
expires=Sun, 31-Jan-2016 11:00:00 GMT;
Domain=.example.net.;

This behaviour is wrong, you can inspect specs to see that attributes must exist only in incoming Set-Cookie header and affect only cookie storage logic in decisions whether to send particular pair or not.

https://tools.ietf.org/html/rfc6265#section-3.1

@neumond
Copy link
Contributor Author

@neumond neumond commented Nov 2, 2015

https://travis-ci.org/KeepSafe/aiohttp/jobs/88759770

Strange. Is that good idea to take cookie name from Morsel?

@asvetlov
Copy link
Member

@asvetlov asvetlov commented Nov 4, 2015

Well, I support the idea for sending key-value pairs only.
The PR should respect both given Cookie header and cookies parameter (which is SimpleCookie),
Tests are required also.

@neumond
Copy link
Contributor Author

@neumond neumond commented Dec 18, 2015

I think it's done now. Please check.

asvetlov added a commit that referenced this pull request Dec 18, 2015
Avoid sending cookie attributes in Cookie header
@asvetlov asvetlov merged commit 14f0441 into aio-libs:master Dec 18, 2015
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@asvetlov
Copy link
Member

@asvetlov asvetlov commented Dec 18, 2015

Cool! Thanks!

@lock
Copy link

@lock lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.