Title: urllib2 does not handle cookies with `,`
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 2.7
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Grzegorz Sikorski, SilentGhost
Priority: normal Keywords:

Created on 2016-10-06 15:01 by Grzegorz Sikorski, last changed 2016-10-12 17:01 by SilentGhost. This issue is now closed.

File name Uploaded Description Edit Grzegorz Sikorski, 2016-10-12 10:27 request test
test.js Grzegorz Sikorski, 2016-10-12 10:29
Messages (6)
msg278196 - (view) Author: Grzegorz Sikorski (Grzegorz Sikorski) Date: 2016-10-06 15:01
I have a usecase when the server sends two cookies in separate `Set-Cookie` headers. One of the cookie includes a `,` (comma). It seems this is not handled properly, as the library always try to fold multiple headers with the same name into a single comma-separated string. While this is valid for other header fields, `Set-Cookie` should never be folded, as RFC 6265 says:
   Origin servers SHOULD NOT fold multiple Set-Cookie header fields into
   a single header field.  The usual mechanism for folding HTTP headers
   fields (i.e., as defined in [RFC2616]) might change the semantics of
   the Set-Cookie header field because the %x2C (",") character is used
   by Set-Cookie in a way that conflicts with such folding.
msg278239 - (view) Author: Grzegorz Sikorski (Grzegorz Sikorski) Date: 2016-10-07 13:06
It looks urllib2 works with this scenario, but upper level request fails.
msg278259 - (view) Author: SilentGhost (SilentGhost) * (Python triager) Date: 2016-10-07 18:23
Could you please post an example of what you're being sent from the server and how your code handles / fails to deal with it.
msg278524 - (view) Author: Grzegorz Sikorski (Grzegorz Sikorski) Date: 2016-10-12 10:27
I was debugging this and found out that urllib2 works more-less correct. The only problem I would see is referring to the header by `res.headers['Set-Cookie']`, as it returns comma-separated string, which cannot be processed properly in case the cookie value includes the `,` (see attached example). IMO this should return a tuple instead of single string, but as I said is minor.

More issues I found with actual `requests` library, as it does not send cookies if the server response with 302 (redirect). Again, this may not be related to the urllib at all.
msg278525 - (view) Author: Grzegorz Sikorski (Grzegorz Sikorski) Date: 2016-10-12 10:29
I attach example express/nodejs server which by default returns a cookie with the comma (see expiry time format). The output from the python test file I posted in previous message is:
cookie1=exampleCookie; Path=/, cookie2=cookie%20with%20comma; Max-Age=60; Path=/; Expires=Wed, 12 Oct 2016 10:24:06 GMT; HttpOnly; Secure
X-Powered-By: Express
Set-Cookie: cookie1=exampleCookie; Path=/
Set-Cookie: cookie2=cookie%20with%20comma; Max-Age=60; Path=/; Expires=Wed, 12 Oct 2016 10:24:06 GMT; HttpOnly; Secure
Date: Wed, 12 Oct 2016 10:23:06 GMT
Connection: close
Content-Length: 0
msg278532 - (view) Author: SilentGhost (SilentGhost) * (Python triager) Date: 2016-10-12 17:01
On python2 you should be able to use Cookie module to parse cookies, rather than doing that manually. Commas are handled correctly there. On python3 the same functionality can be found in http.cookies.
The SimpleCookie.load could be used directly to have the header parsed.

I cannot say much about requests's choices or decisions, but if you think their behaviour is not correct I can only point you to their bug tracker.

All the behaviour of the stdlib modules seem correct to me, so I'm going to close this issue.
Date User Action Args
2016-10-12 17:01:38SilentGhostsetstatus: open -> closed
resolution: not a bug
messages: + msg278532

stage: test needed -> resolved
2016-10-12 10:29:15Grzegorz Sikorskisetfiles: + test.js

messages: + msg278525
2016-10-12 10:27:13Grzegorz Sikorskisetfiles: +

messages: + msg278524
2016-10-07 18:23:23SilentGhostsetversions: + Python 2.7
nosy: + SilentGhost

messages: + msg278259

stage: test needed
2016-10-07 13:06:45Grzegorz Sikorskisetmessages: + msg278239
2016-10-06 15:01:13Grzegorz Sikorskicreate