feat(http-clients)!: Merge a Cookie header with stored cookies

docs/upgrading/upgrading_to_v2.md

@@ -356,6 +356,12 @@ The crawlers pass the remaining `navigation_timeout` to the HTTP client, so it b
 
 The `headers` argument of a client still applies to every request. Its `Authorization`, `Proxy-Authorization` and `Cookie` headers are dropped when a redirect leaves the origin, the same as the headers passed with a request. With `HttpxHttpClient`, the headers of the client now win over the generated browser headers. In v1, the generated browser headers replaced them.
 
+### A `Cookie` header is merged with the session cookies
+
+A `Cookie` header passed with a request or set on the client is merged with the session cookies while the redirect chain stays on its origin. A cookie in the header wins over a session cookie of the same name and isn't stored in the session. In v1, `ImpitHttpClient`, `HttpxHttpClient` and `context.send_request` of `PlaywrightCrawler` sent the header instead of the session cookies, so an empty `Cookie` header kept them out. `CurlImpersonateHttpClient` sent the session cookies in a second `Cookie` header.
+
+Without a session, or with `persist_cookies_per_session=False`, the cookies a server sets during a redirect chain are sent with the following requests of that chain. In v1, `ImpitHttpClient` and `HttpxHttpClient` didn't send them.
+
 ### Some client arguments are ignored
 
 The following keyword arguments of the underlying libraries are now ignored with a warning:

src/crawlee/http_clients/_base.py

@@ -83,11 +83,27 @@ def _redirect_method(status_code: int, method: HttpMethod) -> HttpMethod:
     return method
 
 
+def _split_cookie_header(value: str) -> list[tuple[str, str]]:
+    """Split a `Cookie` header into `(name, pair)` tuples, keeping each pair as it was written."""
+    pairs = [pair.strip() for pair in value.split(';')]
+    return [(pair.partition('=')[0].strip(), pair) for pair in pairs if pair]
+
+
 def _add_cookie_header(headers: HttpHeaders, cookies: SessionCookies, url: str) -> HttpHeaders:
-    """Add the stored cookies matching `url` as the `Cookie` header, unless the headers already have one."""
-    if 'cookie' in headers or not (cookie_header := cookies.get_cookie_string(url)):
+    """Add the stored cookies matching `url` to the `Cookie` header.
+
+    A cookie already in the header wins over a stored cookie of the same name.
+    """
+    explicit_cookies = _split_cookie_header(headers.get('cookie', ''))
+    explicit_names = {name for name, _ in explicit_cookies}
+    stored_cookie_string = cookies.get_cookie_string(url)
+    stored_pairs = [pair for name, pair in _split_cookie_header(stored_cookie_string) if name not in explicit_names]
+
+    if not stored_pairs:
         return headers
 
+    explicit_pairs = [pair for _, pair in explicit_cookies]
+    cookie_header = '; '.join(stored_pairs + explicit_pairs)
     return headers | HttpHeaders({'cookie': cookie_header})
 
 
@@ -230,8 +246,8 @@ async def send_request(
         """Send an HTTP request via the client, following up to 10 redirects unless `follow_redirects` is disabled.
 
         Every request of the redirect chain sends the cookies matching its URL and stores the cookies of its response. A
-        `Cookie` header passed by the caller is sent instead of the stored cookies, and its cookies are not stored. The
-        `Authorization`, `Proxy-Authorization` and `Cookie` headers are dropped when a redirect leaves the origin.
+        cookie in the `Cookie` header passed by the caller wins over a stored cookie of the same name and is not stored.
+        The `Authorization`, `Proxy-Authorization` and `Cookie` headers are dropped when a redirect leaves the origin.
 
         The body of the returned response is not read yet. Read it, or call `aclose` on the response to release its
         connection.

tests/unit/crawlers/_playwright/test_playwright_crawler.py

@@ -566,6 +566,28 @@ async def request_handler(context: PlaywrightCrawlingContext) -> None:
     assert sent_cookies == {'a': '1', 'b': '2', 'check': 'test'}
 
 
+async def test_send_request_merges_cookie_header_with_page_cookies(server_url: URL) -> None:
+    """Test that `context.send_request` merges the `Cookie` header with the page cookies and the header wins."""
+    crawler = PlaywrightCrawler()
+    sent_cookies: list[list[str]] = []
+
+    @crawler.router.default_handler
+    async def request_handler(context: PlaywrightCrawlingContext) -> None:
+        await context.page.context.add_cookies(
+            [
+                {'name': 'manual', 'value': 'from_page', 'url': str(server_url)},
+                {'name': 'from_page', 'value': '1', 'url': str(server_url)},
+            ]
+        )
+        response = await context.send_request(str(server_url / 'headers'), headers={'cookie': 'manual=value'})
+        cookie_header = json.loads(await response.read())['cookie']
+        sent_cookies.append(sorted(cookie_header.split('; ')))
+
+    await crawler.run([str(server_url)])
+
+    assert sent_cookies == [['from_page=1', 'manual=value']]
+
+
 async def test_send_request_skips_cookies_deleted_by_page(server_url: URL) -> None:
     """Test that `context.send_request` doesn't send a session cookie that the page deleted in the handler."""
     crawler = PlaywrightCrawler(session_pool=SessionPool(max_pool_size=1))

tests/unit/http_clients/test_http_clients.py

@@ -659,15 +659,22 @@ async def test_cookie_header_rebuilt_per_hop(http_client: HttpClient, server_url
 
 
 async def test_cookie_header_wins_over_session(http_client: HttpClient, server_url: URL) -> None:
-    """Test that a `Cookie` header passed by the caller replaces the cookies of the session."""
-    session = Session(cookies=[CookieParam(name='from_jar', value='1', domain=server_url.host or '')])
+    """Test that the `Cookie` header of the caller is merged with the session cookies and wins on a shared name."""
+    session = Session(
+        cookies=[
+            CookieParam(name='manual', value='from_jar', domain=server_url.host or ''),
+            CookieParam(name='from_jar', value='1', domain=server_url.host or ''),
+        ]
+    )
 
     async with await http_client.send_request(
-        str(server_url / 'cookies'),
+        str(server_url / 'headers'),
         session=session,
         headers={'cookie': 'manual=value'},
     ) as response:
-        assert (await read_json(response))['cookies'] == {'manual': 'value'}
+        cookie_header = (await read_json(response))['cookie']
+
+    assert sorted(cookie_header.split('; ')) == ['from_jar=1', 'manual=value']
 
 
 async def test_cookie_header_not_stored_in_session(http_client: HttpClient, server_url: URL) -> None:
@@ -720,30 +727,30 @@ async def test_session_cookies_take_over_cross_origin(
 
 
 async def test_cookie_header_wins_over_session_on_redirect(http_client: HttpClient, server_url: URL) -> None:
-    """Test that a `Cookie` header of the caller keeps beating the session cookies after a redirect."""
-    session = Session(cookies=[CookieParam(name='from_jar', value='1', domain=server_url.host or '')])
-    redirect_url = (server_url / 'redirect').with_query(url=str(server_url / 'cookies'), status=302)
+    """Test that a cookie in the `Cookie` header of the caller keeps beating the session cookie after a redirect."""
+    session = Session(cookies=[CookieParam(name='manual', value='from_jar', domain=server_url.host or '')])
+    redirect_url = (server_url / 'redirect').with_query(url=str(server_url / 'headers'), status=302)
 
     async with await http_client.send_request(
         str(redirect_url),
         session=session,
         headers={'cookie': 'manual=value'},
     ) as response:
-        assert (await read_json(response))['cookies'] == {'manual': 'value'}
+        assert (await read_json(response))['cookie'] == 'manual=value'
 
 
-async def test_empty_cookie_header_suppresses_session_cookies(http_client: HttpClient, server_url: URL) -> None:
-    """Test that an empty `Cookie` header of the caller keeps the session cookies out of every hop."""
+async def test_empty_cookie_header_keeps_session_cookies(http_client: HttpClient, server_url: URL) -> None:
+    """Test that the session cookies reach every hop despite an empty `Cookie` header of the caller."""
     session = Session(cookies=[CookieParam(name='from_jar', value='1', domain=server_url.host or '')])
     redirect_url = (server_url / 'redirect').with_query(url=str(server_url / 'cookies'), status=302)
 
     async with await http_client.send_request(
         str(server_url / 'cookies'), session=session, headers={'cookie': ''}
     ) as direct:
-        assert (await read_json(direct))['cookies'] == {}
+        assert (await read_json(direct))['cookies'] == {'from_jar': '1'}
 
     async with await http_client.send_request(str(redirect_url), session=session, headers={'cookie': ''}) as redirected:
-        assert (await read_json(redirected))['cookies'] == {}
+        assert (await read_json(redirected))['cookies'] == {'from_jar': '1'}
 
 
 async def test_auth_kept_same_origin(http_client: HttpClient, server_url: URL) -> None:

tests/unit/http_clients/test_httpx.py

@@ -151,8 +151,8 @@ async def test_client_headers_win_over_generated(server_url: URL) -> None:
     assert headers['user-agent'] == 'client'
 
 
-async def test_client_cookie_header_wins_over_session(server_url: URL) -> None:
-    """Test that a `Cookie` header set on the underlying client replaces the cookies of the session."""
+async def test_client_cookie_header_merged_with_session(server_url: URL) -> None:
+    """Test that a `Cookie` header set on the client is sent along with the cookies of the session."""
     session = Session(cookies=[CookieParam(name='from_jar', value='1', domain=server_url.host or '')])
 
     async with (
@@ -161,7 +161,7 @@ async def test_client_cookie_header_wins_over_session(server_url: URL) -> None:
     ):
         cookies = (await read_json(response))['cookies']
 
-    assert cookies == {'from_client': '1'}
+    assert cookies == {'from_jar': '1', 'from_client': '1'}
 
 
 async def test_cookies_kwarg_ignored(server_url: URL) -> None: