preserve cookie escaping for old servers #1453
Conversation
|
w/o this fix you hit max_redirects trying to re-auth since it doesn't recognize the auth cookie it set |
|
btw, for changes like these do we bump up versioning? What's the versioning strategy? |
|
|
There are two values for a cookie, a coded_value and the non-coded value. Python always encodes the coded_value according to cookie version 1 rules (double quoting the value if the value contains certain characters). The issue is when you've received a version=0 value from a server w/o dbl quoting, it would then dbl quote it and send it back causing the server to not recognize its own cookie. Actually the test is failing because we're now preserving the domain whereas it was not before. I'll check to see what is correct. Btw if anything we now preserve more information than less. Basically we should send cookies back in the format they were received. |
|
ok updated the unittests to expect the domain post filter, and added one to verify that double quoted values are preserved. I believe the tests were previously incorrect as if you do an "output" pre filter it now matches post-filter. |
|
it's kinda confusing given there's http.cookies.SimpleCookie, not to be mistaken by http.cookiejar.Cookie which is expected by http.cookiejar.CookieJar.set_cookie. And then aiohttp has its own CookieJar. |
Current coverage is 98.83% (diff: 100%)@@ master #1453 diff @@
==========================================
Files 30 30
Lines 6939 6943 +4
Methods 0 0
Messages 0 0
Branches 1149 1149
==========================================
+ Hits 6858 6862 +4
Misses 40 40
Partials 41 41
|
thehesiod
changed the title
|
|
||
|
|
||
| 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=/" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
ah, sorry I was figuring that would get filtered out later, I've updated the code to filter these out upfront
|
Thank you |
|
thanks! sorry about the misstep :) |
this preserves the coded value so quotes won't get inserted and break cookie processing from certain servers like: http://hydro1.sci.gsfc.nasa.gov/data/NLDAS/NLDAS_FORA0125_H.002/2010/360/NLDAS_FORA0125_H.A20101226.0000.002.grb which uses: http://disc.sci.gsfc.nasa.gov/alerts/access-to-ges-disc-data-will-require-all-users-to-be-registered-with-the-earthdata-login-system
This updates aiohttp to mimic chrome's behavior in terms of preserving the cooking encoding.