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.

Sign up for GitHub

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

Jump to bottom

preserve cookie escaping for old servers #1453

Merged
merged 9 commits into from
Dec 8, 2016
Merged

preserve cookie escaping for old servers #1453

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fd9b060
Merge remote-tracking branch 'KeepSafe/master'
thehesiod Dec 6, 2016
f5dd285
fix cookies with cranky old servers
thehesiod Dec 6, 2016
3f8cf9d
fix unit tests and add new test for quoted values
thehesiod Dec 6, 2016
85a7e43
fix extra line
thehesiod Dec 6, 2016
5e27754
pycharm did the wrong thing
thehesiod Dec 6, 2016
40f8f7d
update docs
thehesiod Dec 6, 2016
4888559
need to filter out extra properties in morsels
thehesiod Dec 7, 2016
cf108b2
Merge remote-tracking branch 'KeepSafe/master' into cookie-fix
thehesiod Dec 7, 2016
2d58bae
remove debug code
thehesiod Dec 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion aiohttp/client_reqrep.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ def update_cookies(self, cookies):

for name, value in cookies.items():
if isinstance(value, http.cookies.Morsel):
c[value.key] = value.value
# Set with Morsel to preserve coded_value based on version
c[value.key] = value
else:
c[name] = value

Expand Down
4 changes: 3 additions & 1 deletion aiohttp/cookiejar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ def filter_cookies(self, request_url=URL()):
if is_not_secure and cookie["secure"]:
continue

filtered[name] = cookie.value
# It's critical we use the Morsel so the coded_value
# (based on cookie version) is preserved
filtered[name] = cookie

return filtered

Expand Down
19 changes: 16 additions & 3 deletions tests/test_cookiejar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,20 +240,33 @@ def test_preserving_ip_domain_cookies(loop):
))
cookies_sent = jar.filter_cookies(URL("http://127.0.0.1/")).output(
header='Cookie:')
assert cookies_sent == ('Cookie: ip-cookie=second\r\n'
'Cookie: shared-cookie=first')
assert cookies_sent == ('Cookie: ip-cookie=second; Domain=127.0.0.1; '
'Path=/\r\nCookie: shared-cookie=first')


def test_preserving_quoted_cookies(loop):
jar = CookieJar(loop=loop, unsafe=True)
jar.update_cookies(SimpleCookie(
"ip-cookie=\"second\"; Domain=127.0.0.1;"
))
cookies_sent = jar.filter_cookies(URL("http://127.0.0.1/")).output(
header='Cookie:')
assert cookies_sent == ('Cookie: ip-cookie=\"second\"; Domain=127.0.0.1; '
'Path=/')


def test_ignore_domain_ending_with_dot(loop):
jar = CookieJar(loop=loop, unsafe=True)
jar.update_cookies(SimpleCookie("cookie=val; Domain=example.com.;"),
URL("http://www.example.com"))
cookies_sent = jar.filter_cookies(URL("http://www.example.com/"))
assert cookies_sent.output(header='Cookie:') == "Cookie: cookie=val"
assert cookies_sent.output(header='Cookie:') \
== "Cookie: cookie=val; Domain=www.example.com; Path=/"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong as this will appear as 3 cookies: cookie, Domain and Path.
Please see rfc for Cookie header http://httpwg.org/specs/rfc6265.html#sane-cookie
Its the Set-Cookie header that can accept domain/expire/path option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, sorry I was figuring that would get filtered out later, I've updated the code to filter these out upfront

cookies_sent = jar.filter_cookies(URL("http://example.com/"))
assert cookies_sent.output(header='Cookie:') == ""



class TestCookieJarBase(unittest.TestCase):

def setUp(self):
Expand Down