[Client] Inconsistent quoted cookie handling

[Null665]

🐞 Describe the bug

Quoted cookies are unquoted if domain attribute is not set.

💡 To Reproduce
Run this code:

import aiohttp
import asyncio
from http.cookies import SimpleCookie


async def request_with_cookie(session_cookie: str, request_cookie: str):
    session_cookie = SimpleCookie(session_cookie)
    request_cookie = SimpleCookie(request_cookie)

    async with aiohttp.ClientSession(cookies=session_cookie) as session:
        async with session.get(
            'https://httpbin.org/headers',
            cookies=request_cookie,
        ) as response:
            html = await response.text()
            print("Body:", html)


async def main():
    await request_with_cookie(
        'sess="quoted_value"; domain=.httpbin.org',
        'req="quoted_value"'
    )

Outputs

Body: {
  "headers": {
    "Accept": "*/*", 
    "Accept-Encoding": "gzip, deflate", 
    "Cookie": "req=quoted_value; sess=\"quoted_value\"", 
    "Host": "httpbin.org", 
    "User-Agent": "Python/3.8 aiohttp/3.7.3", 
    "X-Amzn-Trace-Id": "Root=1-5ffc692a-7ee8f7946bd59b5f3dc67407"
  }
}

Cookie with domain attribute is quoted, and cookie without domain attribute is not quoted

💡 Expected behavior

I think if user inputs a quoted cookie, the quotes should not be dropped.
At least it should be consistent, and not dependent on domain attribute.

📋 Logs/tracebacks

📋 Your version of the Python

Python 3.8.5

📋 Your version of the aiohttp/yarl/multidict distributions

$ python -m pip show aiohttp
Version: 3.7.3
...

$ python -m pip show multidict
Version: 4.7.6
...

$ python -m pip show yarl
Version: 1.4.2

...

📋 Additional context

Additional code to what I think is the cause

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


def process_cookie_string(cookie_str: str) -> None:
    cookies = SimpleCookie(cookie_str)
    # Done internally by aiohttp - filtering cookies by domain.
    # https://github.com/aio-libs/aiohttp/blob/v3.7.3/aiohttp/client.py#L477
    tmp_cookie_jar = CookieJar()
    tmp_cookie_jar.update_cookies(cookies)
    req_cookies = tmp_cookie_jar.filter_cookies(yarl.URL('https://www.example.com/'))

    for name, value in req_cookies.items():
        print(f"{name=}, {value.value=}, {value.coded_value=}")

if __name__ == '__main__':
    process_cookie_string('name="value"')
    process_cookie_string('name="value"; Domain=example.com')

name='name', value.value='value', value.coded_value='value'
name='name', value.value='value', value.coded_value='"value"'

Also, these example might be a bit of a stretch because official docs say that cookies should be passed as dict.
But is there any reason why I shouldn't be able to send quoted cookies?

[Dreamsorcerer]

I've added the tests for it, but haven't looked at how to fix it yet. Feel free to propose a fix into that PR if you've got an idea.

[Om1kami]

I would like to try to solve this. suspect this is the job of the SimpleCookie class

[Dreamsorcerer]

Go ahead. Should be easy to narrow down from the code in that test. I suspect it happens at the filter_cookies() method:
https://github.com/aio-libs/aiohttp/pull/5398/files#diff-e6fbfbc09805fb38b06bbc571c3be8d507ba8c05f2c335e562432c8fb2ba3a87R313

[Dreamsorcerer]

I've had a look at this now, and it seems non-trivial. My suspicion is that the not domain code should be removed:
https://github.com/aio-libs/aiohttp/blob/fix-quoted-cookie/aiohttp/cookiejar.py#L237-L239

But, if I remove that check (and move the remaining if statements into an if domain: block), then the ("custom-cookie=value/one;", 'Cookie: custom-cookie="value/one"', True) test case fails instead. Not really sure how that should be fixed correctly.

[hen68]

If you remove quotes from the cookie, then both options works fine:

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


def process_cookie_string(cookie_str: str) -> None:
    cookies = SimpleCookie(cookie_str)
    # Done internally by aiohttp - filtering cookies by domain.
    # https://github.com/aio-libs/aiohttp/blob/v3.7.3/aiohttp/client.py#L477
    tmp_cookie_jar = CookieJar()
    tmp_cookie_jar.update_cookies(cookies)
    req_cookies = tmp_cookie_jar.filter_cookies(yarl.URL('https://www.example.com/'))

    for name, value in req_cookies.items():
        print(f"{name=}, {value.value=}, {value.coded_value=}")

if __name__ == '__main__':
    process_cookie_string('name=value')
    process_cookie_string('name=value; Domain=example.com')

Output:

name='name', value.value='value', value.coded_value='value'
name='name', value.value='value', value.coded_value='value'

Updating cookies in client session works as well as a result

[Dreamsorcerer]

Well, yes, because the issue is about quoted cookies. :P
Non-quoted cookies are working fine.