aiohttp icon indicating copy to clipboard operation
aiohttp copied to clipboard

aiohttp.web fails to parse cookies after a cookie with quotes

Open nburns opened this issue 1 year ago • 7 comments

Describe the bug

Cookie values after a double quote " are not parsed. All subsequent cookies in the request are silently dropped.

To Reproduce

  1. setup the following simple aiohttp server:
#!/usr/bin/env python
from aiohttp import web
import json

async def hello(request):
    return web.Response(text=str(request.cookies))

app = web.Application()
app.add_routes([web.get("/", hello)])

web.run_app(app)
  1. run the server
  2. make a request like this which has cookies with a double quote in the value: curl http://localhost:8080 -H 'Cookie: baz="qux; foo=bar;'
  3. notice that the response body/cookies are empty not {'baz'='"qux', 'foo': 'bar'}

Expected behavior

cookie value would be parsed and returned with a double quote in the value, subsequent cookies would also not be silently dropped

Logs/tracebacks

correct behavior

> curl http://localhost:8080 -H 'Cookie: foo=bar;'
{'foo': 'bar'}
> curl http://localhost:8080 -H 'Cookie: foo=bar; baz=qux;'
{'foo': 'bar', 'baz': 'qux'}
> curl http://localhost:8080 -H 'Cookie: foo=bar; baz=qux; foo2=bar2'
{'foo': 'bar', 'baz': 'qux', 'foo2': 'bar2'}

the bug:

> curl http://localhost:8080 -H 'Cookie: foo=bar; baz="qux; foo2=bar2'
{'foo': 'bar'}

Python Version

$ python --version
Python 3.11.5

aiohttp Version

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.9.1
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author:
Author-email:
License: Apache 2
Location: /Users/nick/.asdf/installs/python/3.11.5/lib/python3.11/site-packages
Requires: aiosignal, attrs, frozenlist, multidict, yarl
Required-by: openai

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.0.4
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache 2
Location: /Users/nick/.asdf/installs/python/3.11.5/lib/python3.11/site-packages
Requires:
Required-by: aiohttp, yarl

yarl Version

$ python -m pip show yarl
Name: yarl
Version: 1.9.4
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache-2.0
Location: /Users/nick/.asdf/installs/python/3.11.5/lib/python3.11/site-packages
Requires: idna, multidict
Required-by: aiohttp

OS

macos 14.1.1 (23B81)

Related component

Server

Additional context

We recently deployed a service using aiohttp and noticed that some users would be "logged in" i.e. have a valid session by our main server (webapp2), and not be logged in on our async aiohttp server. It was really hard to narrow down the specfic issue but eventually we found that a requests with a cookie value containing json before our session cookie would cause the user to appear logged out to our aiohttp server

other servers handle the data more robustly for example flask, will correctly parse the 2nd cookie and cookies after it:

#!/usr/bin/env python3

from flask import Flask, request

app = Flask(__name__)

@app.route("/")
def hello():
    return str(request.cookies)

def main():
    app.run(debug=True)

if __name__ == '__main__':
    main()

Code of Conduct

  • [X] I agree to follow the aio-libs Code of Conduct

nburns avatar Dec 22 '23 22:12 nburns

looks like the issue might stem from the usage of SimpleCookie in the standard lib

> ipython

In [1]: from http import cookies

In [2]: c = cookies.SimpleCookie()

In [3]: c.load('foo=bar; baz="qux; oh=no;')

In [4]: c.items()
Out[4]: dict_items([('foo', <Morsel: foo=bar>)])

edit: found the python bug: https://github.com/python/cpython/issues/92936

nburns avatar Dec 22 '23 23:12 nburns

Possibly, though we also do something with handling quoted cookies ourselves, as per #5397. I don't remember the details though.

Dreamsorcerer avatar Dec 23 '23 13:12 Dreamsorcerer

@Dreamsorcerer I updated my original description, the real issue is that you lose the cookies (silently with no error) that appear after a cookie with a value including a double quote in a request.

Sort of a show stopper bug in practice if you use/depend on cookie based client sessions in your web app, because (we saw this especially with chrome, chrome seems to send more cookies than say safari) if any cookie with a double quote gets set and appears before your session cookie it will break sessions for that client. (And it's pretty tricky to reproduce/discover if you're unaware)

nburns avatar Dec 26 '23 23:12 nburns

Yes, I understood the issue, just lacking time to investigate if it relates to the code mentioned in the other issue, or if it's purely an issue upstream.

if any cookie with a double quote gets set

Presumably, only a cookie with a single (or odd?) number of quotes, maybe even only a cookie which starts with a quote and doesn't include a second one.

Dreamsorcerer avatar Dec 26 '23 23:12 Dreamsorcerer

In my testing it's any number of quotes in any position in the cookie value

nburns avatar Jan 02 '24 17:01 nburns

My point was meant to mean that it must handle quoted cookies fine: 'foo=bar; baz="qux"; oh=no;' In other words, it only fails on invalid cookie values. Given they are invalid, it could be difficult to clearly define the correct behaviour.

e.g. In your original post you say that Flask correctly parses 3 cookies from that string, but couldn't the intention have been for 2 cookies ({'foo': 'bar', 'baz': '"qux; oh=no'})?

However, the RFC would actually treat this as 3 values. So, feel free to make a bug report upstream, as it does say that user agents MUST use that algorithm: https://www.rfc-editor.org/rfc/rfc6265.html#section-5.2

Dreamsorcerer avatar Jan 02 '24 18:01 Dreamsorcerer

I think the original implementation was following https://datatracker.ietf.org/doc/html/rfc2109.html Which has now been obsoleted by the above mentioned RFC (and another before that).

Dreamsorcerer avatar Jan 02 '24 18:01 Dreamsorcerer