Switch to a more lenient cookie parsing method

[erewok]

This PR is for issue #898, requesting a more lenient cookie-parsing technique for Starlette. @tomchristie requested a failing test to start, so that's what has been included here. I reused the example from my issue. Happy to modify or update as desired by the maintainers.

[erewok]

Also, for reference, here's the latest spec I have found, RFC 6265, which obsoletes the prior spec. This document offers the following parsing syntax:

   cookie-header = "Cookie:" OWS cookie-string OWS
   cookie-string = cookie-pair *( ";" SP cookie-pair )
   cookie-pair       = cookie-name "=" cookie-value
   cookie-name       = token
   cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
   cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                       ; US-ASCII characters excluding CTLs,
                       ; whitespace DQUOTE, comma, semicolon,
                       ; and backslash
 token             = <token, defined in [RFC2616], Section 2.2>

According to this spec, the Starlette cookie parser (and Python's http.cookies.SimpleCookie.load) are not properly handling cookies.

[erewok]

I also found a related discussion in Tornado, where they also copied what Django is doing after finding that SimpleCookie would not work for the project.

[erewok]

I can add all of the tests they used in Tornado as well, which should give pretty broad coverage of various scenarios.

[erewok]

I have a working version of cookie parsing which will pass all of these tests. I am stuck on mypy telling me the following:

starlette/requests.py:50: error: Module has no attribute "_unquote"
Found 1 error in 1 file (checked 32 source files)

We have two options here, I believe:

• Ignore this mypy error, or
• Pull the entirety of this _unquote function from the Python standard library into this codebase (this is what Tornado did, but not for mypy: they write that they "did not want to have to depend on non-public interfaces", which seems like a reasonable opinion to me, honestly).

I will wait to commit and push what I have until feedback from the maintainers on proceeding with this PR. I believe we have a pretty comprehensive perspective on what other major Python projects are doing in this area and what the typical path is and so I think this is the right approach for Starlette.

Feedback welcome.

[erewok]

I decided to push my changes so they can be compared with other solutions. I would appreciate resolving this in Starlette, so we can pull our own custom cookie-parsing out of our projects. Let me know what you think.

[erewok]

I can't reproduce this failing test locally and it's unrelated to any modules that I modified. Any suggestions?

[tomchristie]

Seems great yup!

I'll have to look into the failing database test separately.
Ended up not being noticed previously as travis wasn't reporting correctly - will look into switching to GitHub actions for CI too.

[tomchristie]

Let me know what you think.

Seems good to merge, right? (Or did I misunderstand something?)

[erewok]

Yes, I thought it was ready but that failing test really threw me off! If you're sure it's unrelated, then I think it's ready to merge. (This PR closes #898)