Cookie values are quoted, even when added to a CookieJar with quote_cookie=False

[dstampher]

Describe the bug

cookie_jar = CookieJar(quote_cookie=False)

And adding some cookies to the jar.  Some of these cookies contain characters such as "/".  When a request is sent and I inspect it, I can see that the cookies value is in quotes, and so it is rejected by the server.

So basically the quote_cookie flag does not work as intended.

To Reproduce

Follow steps

Expected behavior

Cookie values to not be quoted when quote_cookie=False

Logs/tracebacks

N/A

Python Version

$ python --version

aiohttp Version

$ python -m pip show aiohttp

multidict Version

$ python -m pip show multidict

yarl Version

$ python -m pip show yarl

OS

WINDOWS 10

Related component

Client

Additional context

No response

Code of Conduct

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

[dstampher]

I seem to have fixed this by changing line 281 in cookiejar.py to this:

       ` mrsl_val.set(cookie.key, cookie.value, cookie.value if not self._quote_cookie else cookie.coded_value)`

I don't know if this is the best fix but it did work in my initial test. There might be a better way to fix this problem.

[15532th]

Permalink to the line in question. On top of that line there is a comment, stating

        # It's critical we use the Morsel so the coded_value
        # (based on cookie version) is preserved

The _quote_cookie value affects whether filtered is a http.cookies.BaseCookie or a http.cookies.SimpleCookie instance. The only noticeable difference they seem to have is that BaseCookie does not perform quoting and SimpleCookies does, but there is a special case in there.

Here is code from http.cookies.BaseCookie.__set_item__:

    def __setitem__(self, key, value):
        """Dictionary style assignment."""
        if isinstance(value, Morsel):
            # allow assignment of constructed Morsels (e.g. for pickling)
            dict.__setitem__(self, key, value)
        else:
            rval, cval = self.value_encode(value)
            self.__set(key, rval, cval)

If the value is a Morsel instance, it gets assigned as is, effectively bypassing quoting, which is pretty much what the comment in filter_cookies says.

Since aiohttp.CookieJar.filter_cookies does not., in fact, perform any quoting regardless of the _quote_cookie value, the state of cookie value in request is determined before it is added to the jar. Therefore, in order to ensure the request will use unquoted cookies values, they simply should not be quoted when they are added to the jar. One way to do it would be to use http.cookies.BaseCookie to build required cookies and then pass it to aiohttp.CookieJar.update_cookies():

    cookies = http.cookiesBaseCookie()
    ...  # add required cookies here
    jar = aiohttp.CookieJar()
    jar.update_cookies(cookies)

[Dreamsorcerer]

Can someone provide an actual reproducer?

Some of these cookies contain characters such as "/". When a request is sent and I inspect it, I can see that the cookies value is in quotes

We literally have a test that confirms this doesn't happen. Running locally:

>>> from aiohttp import CookieJar
>>> from http.cookies import SimpleCookie
>>> jar = CookieJar(quote_cookie=False)
>>> jar.update_cookies(SimpleCookie("key=value/one;"))
>>> print(next(iter(jar)))
Set-Cookie: key=value/one; Path=/

Follow steps

Actually provide some, and we might look at your issue.

[15532th]

Oh, sure, you should have asked sooner. Here is a minimal example based on the code snippet you provided:

>>> from aiohttp import CookieJar
>>> from http.cookies import SimpleCookie
>>> jar = CookieJar(quote_cookie=False)
>>> jar.update_cookies(SimpleCookie({"key": "value/one;"})) # cookie passed as dict instead of string
>>> print(next(iter(jar)))
Set-Cookie: key="value/one\073"; Path=/

Note that your example does not perform quoting even when it is enabled:

>>> from aiohttp import CookieJar
>>> from http.cookies import SimpleCookie
>>> jar = CookieJar(quote_cookie=True) # quoting enabled
>>> jar.update_cookies(SimpleCookie("key=value/one;"))
>>> print(next(iter(jar)))
Set-Cookie: key=value/one; Path=/

I had to downgrade to aiohttp==3.9.5 to be able to run these examples, because https://github.com/aio-libs/aiohttp/commit/266559cb1e3bd3c28a8ece53f5f77a189229e7fe made it impossible to create aiohttp.CookieJar outside of async context. It is possible to make the examples work with aiohttp==3.10.5 by wrapping them in coroutine, and they produce the same results:

import asyncio
from http.cookies import SimpleCookie
from aiohttp import CookieJar

async def test():
    jar = CookieJar(quote_cookie=False)
    jar.update_cookies(SimpleCookie({"key": "value/one;"}))
    print(next(iter(jar))) # prints Set-Cookie: key="value/one\073"; Path=/

asyncio.run(test())

Additional details about my environment:

Windows 10
Python 3.10.6
aiohttp 3.9.5 and 3.10.5
multidict 5.2.0
yarl 1.9.2

[Dreamsorcerer]

I've just realised the quote_cookies argument is only used in filter_cookies(), so my previous test was pointless.

Do you have a complete reproducer of the issue? A client script or something that is failing or sending the wrong cookies.

I had to downgrade to aiohttp==3.9.5 to be able to run these examples

Ah, probably caused by a legacy loop parameter that's been removed on master (which I tested from).

[15532th]

The actual code from real project was referenced above, here is a self-contained example:

import asyncio
from http import cookiejar
from http.cookies import SimpleCookie
from pathlib import Path

from aiohttp import CookieJar
from yarl import URL

COOKIE_FILE_CONTENT = '''# Netscape HTTP Cookie File

.example.com	TRUE	/	TRUE	1773329281	key	"value/one;"
'''

COOKIE_FILE_PATH = 'test.cookie'


async def test():
    path = Path(COOKIE_FILE_PATH)
    with path.open('wt', encoding=None) as fp:
        fp.write(COOKIE_FILE_CONTENT)

    http_jar = cookiejar.MozillaCookieJar(path)
    http_jar.load(ignore_discard=True, ignore_expires=True)
    path.unlink()

    cookies = SimpleCookie()
    for cookie in http_jar:
        name = cookie.name
        cookies[name] = cookie.value
        cookies[name]['domain'] = cookie.domain

    aiohttp_jar = CookieJar(quote_cookie=False)
    aiohttp_jar.update_cookies(cookies)

    cookies_sent = aiohttp_jar.filter_cookies(URL("https://example.com")).output(header="Cookie:")
    print(cookies_sent)  # prints Cookie: key="\"value/one\073\""


asyncio.run(test())

The app allows users to provide authorization cookies as a text file in the Netscape format, typically generated by a third-party browser extension. They are loaded and parsed into http.cookiejar.MozillaCookieJar, and then converted to aiohttp.CookieJar. While it is not strictly related to this issue, perhaps aiohttp provides some other, simpler way to accomplish this task that I missed?

[Dreamsorcerer]

OK, but the value is quoted in the original MozillaCookieJar, so I think if you want it unquoted, you'd need to unquote it (or to just avoid the double quoting, use BaseCookie instead of SimpleCookie). I don't think there's any expectation for CookieJar to unquote cookies.

I was assuming there was an issue using the client, without directly providing Cookie objects, but the original reporter has provided so little information that I have no idea if it's related to your code or not...

[15532th]

My initial expectations before coming across this issue were that content of a CookieJar(quote_cookie=False) will always be unquoted regardless of the input. Given that Morsel object contains both raw and quoted values, it should technically be possible to achieve.

Original reporter provided a change that fixed the issue. The changed code doesn't get executed at all possible code paths, so it is possible to rule some possibilities out based on that. The nature of the change (replacing quoted value with the raw one) makes me believe that it was the same case of feeding already quoted cookie to CookieJar.update_cookies, though there is indeed no way to know for sure.

[Dreamsorcerer]

My initial expectations before coming across this issue were that content of a CookieJar(quote_cookie=False) will always be unquoted regardless of the input.

That's not my interpretation, it looks to me like enabling the feature should make an effort to quote values. I don't think there's any reason to expect it to unquote values.

But, note that you've double-quoted the values in your example, so there's no way the cookiejar could result in unquoted cookies. cookie.coded_value is '"\\"value/one\\073\\""'. cookie.value is '"value/one;"'. So, if we used value, you`d still end up with a quoted cookie...

[15532th]

You could argue that '"value/one;"' is a perfectly valid raw cookie value that just happened to look like it's quoted. There absolutely could exist a frontend-side javascript code that sets cookie value to a string containing quotes, with the backend expecting them in exactly that format.

When a cookie is provided as a string, there is no telling whether the value is quoted or not, so expecting it to be unquoted is unreasonable, but with a Morsel instance both quoted and raw values are preserved, making it possible to access unquoted value, which is exactly what the fix suggested by the issue opener does.

That said, I have long implemented the workaround of using BaseCookie in my code and it seems to work fine so far, so if making CookieJar.update_cookies to handle the quote_cookie option smarter is not an option, then simple noting this caveat and describing the workaround in the option description in docs seems like a sufficient resolution to this issue.

I would also suggest to consider, as a separate feature, adding ability for the CookieJar to store and load files in the Netscape format in addition to currently supported pickle. Right now client code that needs to support user-provided cookies files has to either rely on MozillaCookieJar and manually convert it into SimpleCookie before passing to aiohttp.CookieJar, or just parse the file directly into a SimpleCookie instance by hands.

[Dreamsorcerer]

You could argue that '"value/one;"' is a perfectly valid raw cookie value that just happened to look like it's quoted.

Isn't this exactly why you'd want the quoted value? If you used SimpleCookie to quote it, then the correct output is "\\"value/one\\073\\"", as it currently produces. This will produce a quoted cookie with a valid JSON string.

If we unquote your cookie then you end up with a quoted cookie containing value/one, which is completely different.

I would also suggest to consider, as a separate feature, adding ability for the CookieJar to store and load files in the Netscape format in addition to currently supported pickle.

If it's not too much code, then we could consider it if you want to implement it.

[15532th]

There is no need to perform unquoting (the process of removing quotes and restoring escaped special characters). As I said, Morsel object contains both escaped and raw values, so "quoting" and "unquoting" is simple a decision to use either quoted or raw value, not a string transformation.