From 91e9be49e9dc0c08418a6797ded770fd1995288d Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Thu, 30 Jul 2026 19:45:16 +0530 Subject: [PATCH] fix: honor session cookies across HTTP client request paths Httpx send_request/stream now send outbound session cookies like crawl. Impit respects persist_cookies_per_session, keys the client cache by jar identity, and closes cached clients on cleanup. Httpx fingerprint headers are generated from a single profile so Accept and User-Agent stay consistent. --- src/crawlee/http_clients/_httpx.py | 19 ++-- src/crawlee/http_clients/_impit.py | 76 +++++++++++---- tests/unit/http_clients/test_http_clients.py | 99 ++++++++++++++++++++ 3 files changed, 171 insertions(+), 23 deletions(-) diff --git a/src/crawlee/http_clients/_httpx.py b/src/crawlee/http_clients/_httpx.py index ec3705a016..ae8d6304bb 100644 --- a/src/crawlee/http_clients/_httpx.py +++ b/src/crawlee/http_clients/_httpx.py @@ -284,6 +284,7 @@ def _build_request( method=method, headers=dict(headers) if headers else None, content=payload, + cookies=session.cookies.jar if session else None, extensions={'crawlee_session': session if self._persist_cookies_per_session else None}, timeout=timeout or httpx.USE_CLIENT_DEFAULT, ) @@ -333,15 +334,19 @@ def _get_client(self, proxy_url: str | None) -> httpx.AsyncClient: def _combine_headers(self, explicit_headers: HttpHeaders | None) -> HttpHeaders | None: """Merge default headers with explicit headers for an HTTP request. - Generate a final set of request headers by combining default headers, a random User-Agent header, - and any explicitly provided headers. + Generate a final set of request headers by combining default headers from a single fingerprint + (Accept, Accept-Language, User-Agent) and any explicitly provided headers. Using one fingerprint + avoids mixing Accept headers from one browser profile with a User-Agent from another. """ - common_headers = self._header_generator.get_common_headers() if self._header_generator else HttpHeaders() - user_agent_header = ( - self._header_generator.get_random_user_agent_header() if self._header_generator else HttpHeaders() - ) + if self._header_generator: + generated_headers = self._header_generator.get_specific_headers( + header_names={'Accept', 'Accept-Language', 'User-Agent'}, + ) + else: + generated_headers = HttpHeaders() + explicit_headers = explicit_headers or HttpHeaders() - headers = common_headers | user_agent_header | explicit_headers + headers = generated_headers | explicit_headers return headers or None @staticmethod diff --git a/src/crawlee/http_clients/_impit.py b/src/crawlee/http_clients/_impit.py index 11e8c81ada..7059977534 100644 --- a/src/crawlee/http_clients/_impit.py +++ b/src/crawlee/http_clients/_impit.py @@ -2,6 +2,8 @@ import asyncio from contextlib import asynccontextmanager +from copy import deepcopy +from http.cookiejar import CookieJar from logging import getLogger from typing import TYPE_CHECKING, Any, TypedDict @@ -20,7 +22,6 @@ if TYPE_CHECKING: from collections.abc import AsyncGenerator, AsyncIterator from datetime import timedelta - from http.cookiejar import CookieJar from crawlee import Request from crawlee._types import HttpMethod, HttpPayload @@ -30,6 +31,9 @@ logger = getLogger(__name__) +# Cache key: (proxy_url, id(cookie_jar) or None) +_ClientCacheKey = tuple[str | None, int | None] + class _ClientCacheEntry(TypedDict): """Type definition for client cache entries.""" @@ -116,7 +120,26 @@ def __init__( self._async_client_kwargs = async_client_kwargs - self._client_by_proxy_url = LRUCache[str | None, _ClientCacheEntry](maxsize=10) + self._client_cache = LRUCache[_ClientCacheKey, _ClientCacheEntry](maxsize=10) + + def _resolve_cookie_jar(self, session: Session | None) -> CookieJar | None: + """Resolve the cookie jar to use for a request. + + When cookie persistence is enabled, Impit mutates the session jar in place (same as attaching the jar + directly). When persistence is disabled, return a deep-copied jar so existing cookies are still sent + outbound, but response `Set-Cookie` values do not update the session. + """ + if session is None: + return None + + if self._persist_cookies_per_session: + return session.cookies.jar + + # Copy cookies so Impit can attach a jar for outbound Cookie headers without mutating the session. + jar = CookieJar() + for cookie in session.cookies.jar: + jar.set_cookie(deepcopy(cookie)) + return jar @override async def crawl( @@ -128,7 +151,7 @@ async def crawl( statistics: Statistics | None = None, timeout: timedelta | None = None, ) -> HttpCrawlingResult: - client = self._get_client(proxy_info.url if proxy_info else None, session.cookies.jar if session else None) + client = self._get_client(proxy_info.url if proxy_info else None, self._resolve_cookie_jar(session)) try: response = await client.request( @@ -169,7 +192,7 @@ async def send_request( if isinstance(headers, dict) or headers is None: headers = HttpHeaders(headers or {}) - client = self._get_client(proxy_info.url if proxy_info else None, session.cookies.jar if session else None) + client = self._get_client(proxy_info.url if proxy_info else None, self._resolve_cookie_jar(session)) try: response = await client.request( @@ -203,7 +226,7 @@ async def stream( ) -> AsyncGenerator[HttpResponse]: validate_http_url(url) - client = self._get_client(proxy_info.url if proxy_info else None, session.cookies.jar if session else None) + client = self._get_client(proxy_info.url if proxy_info else None, self._resolve_cookie_jar(session)) try: response = await client.request( @@ -222,18 +245,31 @@ async def stream( finally: response.close() + @staticmethod + def _make_cache_key(proxy_url: str | None, cookie_jar: CookieJar | None) -> _ClientCacheKey: + return (proxy_url, id(cookie_jar) if cookie_jar is not None else None) + + async def _close_client(self, client: AsyncClient) -> None: + # Impit exposes cleanup via the async context manager protocol. + result = client.__aexit__(None, None, None) + if hasattr(result, '__await__'): + await result # type: ignore[misc] + def _get_client(self, proxy_url: str | None, cookie_jar: CookieJar | None) -> AsyncClient: - """Retrieve or create an HTTP client for the given proxy URL. + """Retrieve or create an HTTP client for the given proxy URL and cookie jar. - If a client for the specified proxy URL does not exist, create and store a new one. + Clients are cached by `(proxy_url, cookie_jar identity)` so sessions with different jars do not share + a client. When cookie persistence is disabled, each request uses a fresh jar copy and therefore a + short-lived client that is not retained in the cache. """ - cached_data = self._client_by_proxy_url.get(proxy_url) - if cached_data: - client = cached_data['client'] - client_cookie_jar = cached_data['cookie_jar'] - if client_cookie_jar is cookie_jar: - # If the cookie jar matches, return the existing client. - return client + # Ephemeral jars (persist_cookies_per_session=False) must not pollute / thrash the LRU cache. + cacheable = cookie_jar is None or self._persist_cookies_per_session + cache_key = self._make_cache_key(proxy_url, cookie_jar) if cacheable else None + + if cache_key is not None: + cached_data = self._client_cache.get(cache_key) + if cached_data and cached_data['cookie_jar'] is cookie_jar: + return cached_data['client'] # Prepare a default kwargs for the new client. kwargs: dict[str, Any] = { @@ -249,7 +285,13 @@ def _get_client(self, proxy_url: str | None, cookie_jar: CookieJar | None) -> As client = AsyncClient(**kwargs, cookie_jar=cookie_jar) - self._client_by_proxy_url[proxy_url] = _ClientCacheEntry(client=client, cookie_jar=cookie_jar) + if cache_key is not None: + # Close the client being evicted when the LRU is full, to avoid leaking connections. + if len(self._client_cache) >= self._client_cache.maxsize: + _evicted_key, evicted_entry = next(iter(self._client_cache.items())) + asyncio.get_running_loop().create_task(self._close_client(evicted_entry['client'])) + + self._client_cache[cache_key] = _ClientCacheEntry(client=client, cookie_jar=cookie_jar) return client @@ -270,4 +312,6 @@ def _is_proxy_error(error: HTTPError) -> bool: @override async def cleanup(self) -> None: """Clean up resources used by the HTTP client.""" - self._client_by_proxy_url.clear() + for entry in list(self._client_cache.values()): + await self._close_client(entry['client']) + self._client_cache.clear() diff --git a/tests/unit/http_clients/test_http_clients.py b/tests/unit/http_clients/test_http_clients.py index aa95e1f62e..d8ba4a834f 100644 --- a/tests/unit/http_clients/test_http_clients.py +++ b/tests/unit/http_clients/test_http_clients.py @@ -2,6 +2,7 @@ import asyncio import importlib +import json import os import sys from typing import TYPE_CHECKING @@ -12,8 +13,11 @@ from pydantic import ValidationError from crawlee import Request +from crawlee._types import HttpHeaders from crawlee.errors import ProxyError +from crawlee.fingerprint_suite import HeaderGenerator from crawlee.http_clients import CurlImpersonateHttpClient, HttpClient, HttpxHttpClient, ImpitHttpClient +from crawlee.sessions import Session from crawlee.statistics import Statistics from tests.unit.server import generate_file_content from tests.unit.server_endpoints import HELLO_WORLD @@ -323,3 +327,98 @@ def test_import_error_handled(optional_module_name: str, import_path: str) -> No sys.modules.pop(mod_name, None) with pytest.raises(ImportError): importlib.import_module(import_path) + + +async def test_send_request_sends_session_cookies(http_client: HttpClient, server_url: URL) -> None: + """`send_request` must attach existing session cookies (same as `crawl`).""" + session = Session() + session.cookies.set('auth', 'token-1', domain=server_url.host or '127.0.0.1', path='/') + + response = await http_client.send_request(str(server_url / 'cookies'), session=session) + body = json.loads(await response.read()) + + assert body['cookies'] == {'auth': 'token-1'} + + +async def test_stream_sends_session_cookies(http_client: HttpClient, server_url: URL) -> None: + """`stream` must attach existing session cookies (same as `crawl`).""" + session = Session() + session.cookies.set('auth', 'token-2', domain=server_url.host or '127.0.0.1', path='/') + + content = b'' + async with http_client.stream(str(server_url / 'cookies'), session=session) as response: + async for chunk in response.read_stream(): + content += chunk + + assert json.loads(content)['cookies'] == {'auth': 'token-2'} + + +@pytest.mark.parametrize( + 'custom_http_client', + [ + pytest.param(CurlImpersonateHttpClient(persist_cookies_per_session=False), id='curl'), + pytest.param(HttpxHttpClient(persist_cookies_per_session=False), id='httpx'), + pytest.param(ImpitHttpClient(persist_cookies_per_session=False), id='impit'), + ], + indirect=['custom_http_client'], +) +async def test_persist_cookies_per_session_false(custom_http_client: HttpClient, server_url: URL) -> None: + """When persistence is disabled, response Set-Cookie must not update the session jar.""" + session = Session() + request = Request.from_url(str(server_url.with_path('set_cookies').extend_query(a=1))) + + await custom_http_client.crawl(request, session=session) + + assert {cookie['name']: cookie['value'] for cookie in session.cookies.get_cookies_as_dicts()} == {} + + +@pytest.mark.parametrize( + 'custom_http_client', + [ + pytest.param(CurlImpersonateHttpClient(persist_cookies_per_session=True), id='curl'), + pytest.param(HttpxHttpClient(persist_cookies_per_session=True), id='httpx'), + pytest.param(ImpitHttpClient(persist_cookies_per_session=True), id='impit'), + ], + indirect=['custom_http_client'], +) +async def test_persist_cookies_per_session_true(custom_http_client: HttpClient, server_url: URL) -> None: + """When persistence is enabled, response Set-Cookie must update the session jar.""" + session = Session() + request = Request.from_url(str(server_url.with_path('set_cookies').extend_query(a=1))) + + await custom_http_client.crawl(request, session=session) + + assert {cookie['name']: cookie['value'] for cookie in session.cookies.get_cookies_as_dicts()} == {'a': '1'} + + +async def test_httpx_headers_come_from_single_fingerprint() -> None: + """Accept and User-Agent must come from the same generated fingerprint profile.""" + header_generator = HeaderGenerator() + fingerprint = {'Accept': 'text/html', 'Accept-Language': 'en-US', 'User-Agent': 'TestAgent/1.0'} + + with patch.object(header_generator, 'get_specific_headers', return_value=HttpHeaders(fingerprint)) as mocked: + client = HttpxHttpClient(header_generator=header_generator) + combined = client._combine_headers(None) + + mocked.assert_called_once_with(header_names={'Accept', 'Accept-Language', 'User-Agent'}) + assert combined is not None + assert combined['accept'] == 'text/html' + assert combined['accept-language'] == 'en-US' + assert combined['user-agent'] == 'TestAgent/1.0' + + +async def test_impit_cleanup_clears_client_cache(server_url: URL) -> None: + """`ImpitHttpClient.cleanup` must drop cached clients so the next request creates a fresh one.""" + client = ImpitHttpClient() + async with client: + await client.send_request(str(server_url)) + assert len(client._client_cache) == 1 + first_client = next(iter(client._client_cache.values()))['client'] + + await client.cleanup() + assert len(client._client_cache) == 0 + + await client.send_request(str(server_url)) + assert len(client._client_cache) == 1 + second_client = next(iter(client._client_cache.values()))['client'] + assert second_client is not first_client