requests icon indicating copy to clipboard operation
requests copied to clipboard

Parse cookies set by Django app

Open jazzdev opened this issue 2 years ago • 1 comments
trafficstars

I'm using requests to automate a custom Django app. The Django app sets cookies that are not parsed by requests.

This addresses two issues that keep requests from parsing the cookies:

  1. A Set-Cookie header with multiple cookies will fail to parse
  2. A Set-Cookie with a Max-Age attribute that is a floating point number will fail to parse

Django does both of these things.

This PR handles multiple cookies by replacing a single Set-Cookie header with multiple cookies with multiple Set-Cookie headers with one cookie each.

This PR rounds down Max-Age to an int.

4 new tests are added.

jazzdev avatar Feb 25 '23 22:02 jazzdev

I believe this would be an issue with any HTTP client using the standard library for cookie parsing. I'm not sure that makes it appropriate to fix here

sigmavirus24 avatar Feb 26 '23 15:02 sigmavirus24

Hi @jazzdev, looking over the PR, it seems Django may be doing the wrong thing in this case. It's not clear from what's been provided whether this is due to user input or a defect in Django. Requests uses the standard library to parse cookies and I'm not sure we want to start building out our own parsing.

This is what I see in the specifications regarding what you're seeing (emphasis mine):

If the attribute-name case-insensitively matches the string "Max- Age", the user agent MUST process the cookie-av as follows.

If the first character of the attribute-value is not a DIGIT or a "-" character, ignore the cookie-av.

If the remainder of attribute-value contains a non-DIGIT character, ignore the cookie-av.

Let delta-seconds be the attribute-value converted to an integer.

- RFC 6265 5.2.2

A value containing anything other than a DIGIT or a "-" character means the entire value is ignored.

For bundling multiple cookies into a single Set-Cookie header, it's not strictly a spec violation but definitely goes against what's advised.

Origin servers SHOULD NOT fold multiple Set-Cookie header fields into a single header field.

The latest HTTP/1.1 RFC (9110) takes it a step farther stating they mechanically cannot be combined due to parsing implementation issues like you're seeing. Regarding collapsing cookies based on matching keys:

Since it cannot be combined into a single field value, recipients ought to handle "Set-Cookie" as a special case while processing fields. RFC 9110 5.3

--

For those reasons, I think we're not inclined to add this functionality. It's going to move us into an awkward position where we cannot do the correct thing in all cases. Trying to support violations of the RFCs has bitten us previously so ideally this should be fixed either in the app or Django depending on the origin of the issue.

nateprewitt avatar Jul 30 '23 01:07 nateprewitt