aio-libs / aiohttp

Asynchronous HTTP client/server framework for asyncio and Python
https://docs.aiohttp.org
Other
15.18k stars 2.02k forks source link

aiohttp ignoring cookie_jar quote_cookie parameter #9336

Open TomerSalt opened 2 months ago

TomerSalt commented 2 months ago

Describe the bug

sess = aiohttp.ClientSession(connector=conn, cookie_jar=aiohttp.CookieJar(quote_cookie=False)).request(...)

When using an client session object with a cookie jar with the cookie quoting routine disabled the requests sent still uses quoting routine when invoking the request:

[client_reqrep.py:468] ClientRequest().update_cookies():

def update_cookies(self, cookies: Optional[LooseCookies]) -> None:
        """Update request cookies header."""
        if not cookies:
            return

        c = SimpleCookie()
        .
        .
        .
        self.headers[hdrs.COOKIE] = c.output(header="", sep=";").strip()

The variable c used to output the cookie is a SimpleCookie object which defaults to using the quoting routine. therefor when for example received a cookie with value such as id=!IagQHEStKHLB3DFBAufSQIeK+Olw= the header will output as Cookie: id="!IagQHEStKHLB3DFBAufSQIeK+Olw=".

A possible fix is to add a method to the CookieJar object like is_quote_disabled() and then in the update cookies use the self.session.is_quoting_disabled() to specifically handle the cookie in such case. i used:

self.headers[hdrs.COOKIE] = '; '.join([f"{name}={mrsl.value}" for name,mrsl in cookies.items()])

To Reproduce

use the following script to reproduce:

import aiohttp
import asyncio

async def req():
    tcp_conn = aiohttp.TCPConnector(limit=10, force_close=True)
    c_jar = aiohttp.CookieJar(quote_cookie=False)
    sess = aiohttp.ClientSession(connector=tcp_conn, cookie_jar=c_jar)
    cookies = {"name": "val="}
    resp = await sess.request(url="http://localhost:8000/", method="GET", cookies=cookies)
    print(resp.request_info.headers.get("Cookie")) # name="val="
    await sess.close()

asyncio.run(req())

Expected behavior

When specifying quote_cookie to False, cookie which by specifications are supposed to be quoted shouldn't be quoted because of the implicit flag.

Logs/tracebacks

-

Python Version

$ python --version
Python 3.9.6

aiohttp Version

$ python -m pip show aiohttp
Version: 3.10.5

multidict Version

$ python -m pip show multidict
Version: 6.1.0

yarl Version

$ python -m pip show yarl
Version: 1.11.1

OS

Mac OS

Related component

Client

Additional context

No response

Code of Conduct

Dreamsorcerer commented 2 months ago

Please create a complete reproducer (or better, a failing test in a PR).

TomerSalt commented 2 months ago

import aiohttp import asyncio

async def req(): tcp_conn = aiohttp.TCPConnector(limit=10, force_close=True) c_jar = aiohttp.CookieJar(quote_cookie=False) sess = aiohttp.ClientSession(connector=tcp_conn, cookie_jar=c_jar) cookies = {"name": "val="} resp = await sess.request(url="http://localhost:8000/", method="GET", cookies=cookies) print(resp.request_info.headers.get("Cookie")) # name="val=" await sess.close()

asyncio.run(req())

i updated the reproduce section to contain a full example

Cycloctane commented 1 month ago

ClientSession()._request() creates a temporary CookieJar tmp_cookie_jar to handle the cookies from the request() parameters. This tmp_cookie_jar only uses default (quote_cookie=True).

https://github.com/aio-libs/aiohttp/blob/24b0e6f7c9d5a0815aebdc0dde5ff9a860a550d9/aiohttp/client.py#L576-L583

If cookies are directly passed to the manually created CookieJar, they will remain unquoted.

import aiohttp
import asyncio

async def req():
    tcp_conn = aiohttp.TCPConnector(limit=10, force_close=True)
    c_jar = aiohttp.CookieJar(quote_cookie=False)
    sess = aiohttp.ClientSession(connector=tcp_conn, cookie_jar=c_jar)
    cookies = {"name": "val="}
    c_jar.update_cookies(cookies)
    resp = await sess.request(url="http://localhost:8000/", method="GET")
    print(resp.request_info.headers.get("Cookie")) # name=val=
    await sess.close()

asyncio.run(req())
Dreamsorcerer commented 1 month ago

So, perhaps we should be reusing the settings from the global CookieJar. Think you could turn that into a full test?