From 91e9be49e9dc0c08418a6797ded770fd1995288d Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Thu, 30 Jul 2026 19:45:16 +0530 Subject: [PATCH 1/5] 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 From dccf1e72cb5869593a982778f1e7d405063ac05b Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Mon, 3 Aug 2026 19:02:25 +0530 Subject: [PATCH 2/5] fix: address Impit cookie review feedback Send cookies via Cookie header when persist_cookies_per_session is False so clients stay cached, and use LRUCache.popitem for eviction without fire-and-forget close tasks. --- src/crawlee/http_clients/_impit.py | 96 ++++++++++++++++-------------- 1 file changed, 52 insertions(+), 44 deletions(-) diff --git a/src/crawlee/http_clients/_impit.py b/src/crawlee/http_clients/_impit.py index 7059977534..47a7a78534 100644 --- a/src/crawlee/http_clients/_impit.py +++ b/src/crawlee/http_clients/_impit.py @@ -2,10 +2,9 @@ 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 +from urllib.request import Request as UrllibRequest from cachetools import LRUCache from impit import AsyncClient, Browser, HTTPError, Response, TimeoutException, TransportError @@ -22,6 +21,7 @@ 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 @@ -122,24 +122,41 @@ def __init__( 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. + def _prepare_cookies_and_headers( + self, + *, + session: Session | None, + url: str, + headers: HttpHeaders | dict[str, str] | None, + ) -> tuple[CookieJar | None, HttpHeaders | None]: + """Resolve cookie jar / Cookie header based on `persist_cookies_per_session`. - 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. + When persistence is enabled, attach the session jar to Impit so response cookies update it. + When persistence is disabled, send existing cookies via the `Cookie` header and keep the + shared client (no jar) so clients stay cached and reusable. """ + if isinstance(headers, dict) or headers is None: + headers = HttpHeaders(headers or {}) + if session is None: - return None + return None, headers or None if self._persist_cookies_per_session: - return session.cookies.jar + return session.cookies.jar, headers or None - # 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 + cookie_header = self._get_cookie_header(session.cookies.jar, url, headers) + if cookie_header and 'cookie' not in headers: + headers = headers | HttpHeaders({'Cookie': cookie_header}) + + return None, headers or None + + @staticmethod + def _get_cookie_header(jar: CookieJar, url: str, headers: HttpHeaders | None = None) -> str: + """Build a Cookie request header from a jar without attaching the jar to the client.""" + # UrllibRequest is only used to format Cookie headers via CookieJar; it never opens a connection. + request = UrllibRequest(url, headers=dict(headers) if headers else {}) # noqa: S310 + jar.add_cookie_header(request) + return request.get_header('Cookie') or '' @override async def crawl( @@ -151,14 +168,19 @@ async def crawl( statistics: Statistics | None = None, timeout: timedelta | None = None, ) -> HttpCrawlingResult: - client = self._get_client(proxy_info.url if proxy_info else None, self._resolve_cookie_jar(session)) + cookie_jar, headers = self._prepare_cookies_and_headers( + session=session, + url=request.url, + headers=request.headers, + ) + client = self._get_client(proxy_info.url if proxy_info else None, cookie_jar) try: response = await client.request( url=request.url, method=request.method, content=request.payload, - headers=dict(request.headers) if request.headers else None, + headers=dict(headers) if headers else None, timeout=timeout.total_seconds() if timeout else None, ) except TimeoutException as exc: @@ -189,10 +211,8 @@ async def send_request( ) -> HttpResponse: validate_http_url(url) - if isinstance(headers, dict) or headers is None: - headers = HttpHeaders(headers or {}) - - client = self._get_client(proxy_info.url if proxy_info else None, self._resolve_cookie_jar(session)) + cookie_jar, headers = self._prepare_cookies_and_headers(session=session, url=url, headers=headers) + client = self._get_client(proxy_info.url if proxy_info else None, cookie_jar) try: response = await client.request( @@ -226,7 +246,8 @@ async def stream( ) -> AsyncGenerator[HttpResponse]: validate_http_url(url) - client = self._get_client(proxy_info.url if proxy_info else None, self._resolve_cookie_jar(session)) + cookie_jar, headers = self._prepare_cookies_and_headers(session=session, url=url, headers=headers) + client = self._get_client(proxy_info.url if proxy_info else None, cookie_jar) try: response = await client.request( @@ -249,27 +270,18 @@ async def stream( 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 and cookie jar. 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. + a client. When cookie persistence is disabled, cookies are sent via headers and `cookie_jar` is `None`, + so a shared client can be reused for the proxy. """ - # 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 + cache_key = self._make_cache_key(proxy_url, cookie_jar) - 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'] + 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] = { @@ -285,13 +297,11 @@ def _get_client(self, proxy_url: str | None, cookie_jar: CookieJar | None) -> As client = AsyncClient(**kwargs, 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'])) + # Evict the least-recently-used entry explicitly before inserting. + if cache_key not in self._client_cache and len(self._client_cache) >= self._client_cache.maxsize: + self._client_cache.popitem() - self._client_cache[cache_key] = _ClientCacheEntry(client=client, cookie_jar=cookie_jar) + self._client_cache[cache_key] = _ClientCacheEntry(client=client, cookie_jar=cookie_jar) return client @@ -312,6 +322,4 @@ def _is_proxy_error(error: HTTPError) -> bool: @override async def cleanup(self) -> None: """Clean up resources used by the HTTP client.""" - for entry in list(self._client_cache.values()): - await self._close_client(entry['client']) self._client_cache.clear() From 086b115b7cc2b454518e2ea41c365fb5b81692ec Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Tue, 4 Aug 2026 01:18:58 +0530 Subject: [PATCH 3/5] fix: address remaining Impit review nits Simplify cookie header merge, drop redundant LRU popitem, and move client-specific Httpx/Impit tests into dedicated test modules. --- src/crawlee/http_clients/_impit.py | 17 ++++------ tests/unit/http_clients/test_http_clients.py | 35 -------------------- tests/unit/http_clients/test_httpx.py | 19 +++++++++++ tests/unit/http_clients/test_impit.py | 25 ++++++++++++++ 4 files changed, 50 insertions(+), 46 deletions(-) create mode 100644 tests/unit/http_clients/test_impit.py diff --git a/src/crawlee/http_clients/_impit.py b/src/crawlee/http_clients/_impit.py index 47a7a78534..1111cf090a 100644 --- a/src/crawlee/http_clients/_impit.py +++ b/src/crawlee/http_clients/_impit.py @@ -128,7 +128,7 @@ def _prepare_cookies_and_headers( session: Session | None, url: str, headers: HttpHeaders | dict[str, str] | None, - ) -> tuple[CookieJar | None, HttpHeaders | None]: + ) -> tuple[CookieJar | None, HttpHeaders]: """Resolve cookie jar / Cookie header based on `persist_cookies_per_session`. When persistence is enabled, attach the session jar to Impit so response cookies update it. @@ -139,16 +139,15 @@ def _prepare_cookies_and_headers( headers = HttpHeaders(headers or {}) if session is None: - return None, headers or None + return None, headers if self._persist_cookies_per_session: - return session.cookies.jar, headers or None + return session.cookies.jar, headers - cookie_header = self._get_cookie_header(session.cookies.jar, url, headers) - if cookie_header and 'cookie' not in headers: + if cookie_header := self._get_cookie_header(session.cookies.jar, url, headers): headers = headers | HttpHeaders({'Cookie': cookie_header}) - return None, headers or None + return None, headers @staticmethod def _get_cookie_header(jar: CookieJar, url: str, headers: HttpHeaders | None = None) -> str: @@ -156,7 +155,7 @@ def _get_cookie_header(jar: CookieJar, url: str, headers: HttpHeaders | None = N # UrllibRequest is only used to format Cookie headers via CookieJar; it never opens a connection. request = UrllibRequest(url, headers=dict(headers) if headers else {}) # noqa: S310 jar.add_cookie_header(request) - return request.get_header('Cookie') or '' + return request.get_header('Cookie', '') @override async def crawl( @@ -297,10 +296,6 @@ def _get_client(self, proxy_url: str | None, cookie_jar: CookieJar | None) -> As client = AsyncClient(**kwargs, cookie_jar=cookie_jar) - # Evict the least-recently-used entry explicitly before inserting. - if cache_key not in self._client_cache and len(self._client_cache) >= self._client_cache.maxsize: - self._client_cache.popitem() - self._client_cache[cache_key] = _ClientCacheEntry(client=client, cookie_jar=cookie_jar) return client diff --git a/tests/unit/http_clients/test_http_clients.py b/tests/unit/http_clients/test_http_clients.py index d8ba4a834f..592c0eb5ae 100644 --- a/tests/unit/http_clients/test_http_clients.py +++ b/tests/unit/http_clients/test_http_clients.py @@ -13,9 +13,7 @@ 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 @@ -389,36 +387,3 @@ async def test_persist_cookies_per_session_true(custom_http_client: HttpClient, 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 diff --git a/tests/unit/http_clients/test_httpx.py b/tests/unit/http_clients/test_httpx.py index c98ca4bbf7..b0a97d8755 100644 --- a/tests/unit/http_clients/test_httpx.py +++ b/tests/unit/http_clients/test_httpx.py @@ -3,9 +3,12 @@ import json import logging from typing import TYPE_CHECKING +from unittest.mock import patch import pytest +from crawlee._types import HttpHeaders +from crawlee.fingerprint_suite import HeaderGenerator from crawlee.fingerprint_suite._browserforge_adapter import get_available_header_values from crawlee.fingerprint_suite._consts import COMMON_ACCEPT_LANGUAGE from crawlee.http_clients import HttpxHttpClient @@ -54,3 +57,19 @@ async def test_common_headers_and_user_agent(server_url: URL, header_network: di assert 'user-agent' in response_headers assert 'python-httpx' not in response_headers['user-agent'] assert response_headers['user-agent'] in get_available_header_values(header_network, {'User-Agent', 'user-agent'}) + + +async def test_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' diff --git a/tests/unit/http_clients/test_impit.py b/tests/unit/http_clients/test_impit.py new file mode 100644 index 0000000000..08cd577a02 --- /dev/null +++ b/tests/unit/http_clients/test_impit.py @@ -0,0 +1,25 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +from crawlee.http_clients import ImpitHttpClient + +if TYPE_CHECKING: + from yarl import URL + + +async def test_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 From d4f5faeb085d85077ae2ce12229df7315f8d3883 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Tue, 4 Aug 2026 16:32:54 +0530 Subject: [PATCH 4/5] fix: address httpx redirect cookies and Impit cache review Key httpx clients by session jar so cookies survive redirects, reuse _build_request from crawl, simplify Impit LRU keying by jar identity, deprecate unused HeaderGenerator helpers, and cover redirect plus persist=False cookie-header paths in tests. --- .../fingerprint_suite/_header_generator.py | 20 +++++++- src/crawlee/http_clients/_httpx.py | 50 +++++++++++-------- src/crawlee/http_clients/_impit.py | 49 ++++++------------ tests/unit/http_clients/test_http_clients.py | 23 +++++++++ tests/unit/http_clients/test_httpx.py | 17 ++++--- tests/unit/http_clients/test_impit.py | 19 ++++++- 6 files changed, 111 insertions(+), 67 deletions(-) diff --git a/src/crawlee/fingerprint_suite/_header_generator.py b/src/crawlee/fingerprint_suite/_header_generator.py index 1c7111db57..6527a24cda 100644 --- a/src/crawlee/fingerprint_suite/_header_generator.py +++ b/src/crawlee/fingerprint_suite/_header_generator.py @@ -1,5 +1,6 @@ from __future__ import annotations +import warnings from typing import TYPE_CHECKING, Literal from crawlee._types import HttpHeaders @@ -50,12 +51,29 @@ def get_common_headers(self) -> HttpHeaders: We do not modify the "Accept-Encoding", "Connection" and other headers. They should be included and handled by the HTTP client or browser. + + .. deprecated:: + Use `get_specific_headers` instead. """ + warnings.warn( + 'get_common_headers is deprecated, use get_specific_headers instead.', + DeprecationWarning, + stacklevel=2, + ) all_headers = self._generator.generate() return self._select_specific_headers(all_headers, header_names={'Accept', 'Accept-Language'}) def get_random_user_agent_header(self) -> HttpHeaders: - """Get a random User-Agent header.""" + """Get a random User-Agent header. + + .. deprecated:: + Use `get_specific_headers` instead. + """ + warnings.warn( + 'get_random_user_agent_header is deprecated, use get_specific_headers instead.', + DeprecationWarning, + stacklevel=2, + ) all_headers = self._generator.generate() return self._select_specific_headers(all_headers, header_names={'User-Agent'}) diff --git a/src/crawlee/http_clients/_httpx.py b/src/crawlee/http_clients/_httpx.py index ae8d6304bb..6c4b3de74e 100644 --- a/src/crawlee/http_clients/_httpx.py +++ b/src/crawlee/http_clients/_httpx.py @@ -20,6 +20,7 @@ if TYPE_CHECKING: from collections.abc import AsyncGenerator, AsyncIterator from datetime import timedelta + from http.cookiejar import CookieJar from ssl import SSLContext from crawlee import Request @@ -145,7 +146,7 @@ def __init__( self._transport: _HttpxTransport | None = None - self._client_by_proxy_url = dict[str | None, httpx.AsyncClient]() + self._client_cache = dict[tuple[str | None, int | None], httpx.AsyncClient]() @override async def crawl( @@ -157,17 +158,17 @@ async def crawl( statistics: Statistics | None = None, timeout: timedelta | None = None, ) -> HttpCrawlingResult: - client = self._get_client(proxy_info.url if proxy_info else None) - headers = self._combine_headers(request.headers) + cookie_jar = session.cookies.jar if session else None + client = self._get_client(proxy_info.url if proxy_info else None, cookie_jar) - http_request = client.build_request( + http_request = self._build_request( + client=client, url=request.url, method=request.method, - headers=headers, - content=request.payload, - cookies=session.cookies.jar if session else None, - extensions={'crawlee_session': session if self._persist_cookies_per_session else None}, - timeout=timeout.total_seconds() if timeout is not None else httpx.USE_CLIENT_DEFAULT, + headers=request.headers, + payload=request.payload, + session=session, + timeout=httpx.Timeout(timeout.total_seconds()) if timeout is not None else None, ) try: @@ -202,7 +203,8 @@ async def send_request( ) -> HttpResponse: validate_http_url(url) - client = self._get_client(proxy_info.url if proxy_info else None) + cookie_jar = session.cookies.jar if session else None + client = self._get_client(proxy_info.url if proxy_info else None, cookie_jar) http_request = self._build_request( client=client, @@ -240,7 +242,8 @@ async def stream( ) -> AsyncGenerator[HttpResponse]: validate_http_url(url) - client = self._get_client(proxy_info.url if proxy_info else None) + cookie_jar = session.cookies.jar if session else None + client = self._get_client(proxy_info.url if proxy_info else None, cookie_jar) http_request = self._build_request( client=client, @@ -289,13 +292,13 @@ def _build_request( timeout=timeout or httpx.USE_CLIENT_DEFAULT, ) - def _get_client(self, proxy_url: str | None) -> httpx.AsyncClient: - """Retrieve or create an HTTP client for the given proxy URL. + def _get_client(self, proxy_url: str | None, cookie_jar: CookieJar | None = None) -> httpx.AsyncClient: + """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, id(cookie_jar))` so that redirects use the session cookie jar + attached to the client rather than per-request `cookies=` which httpx strips on redirect. """ if not self._transport: - # Configure connection pool limits and keep-alive connections for transport limits = self._async_client_kwargs.get( 'limits', httpx.Limits(max_connections=1000, max_keepalive_connections=200) ) @@ -307,8 +310,9 @@ def _get_client(self, proxy_url: str | None) -> httpx.AsyncClient: limits=limits, ) - if proxy_url not in self._client_by_proxy_url: - # Prepare a default kwargs for the new client. + cache_key = (proxy_url, id(cookie_jar) if cookie_jar is not None else None) + + if cache_key not in self._client_cache: kwargs: dict[str, Any] = { 'proxy': proxy_url, 'http1': self._http1, @@ -316,7 +320,6 @@ def _get_client(self, proxy_url: str | None) -> httpx.AsyncClient: 'follow_redirects': True, } - # Update the default kwargs with any additional user-provided kwargs. kwargs.update(self._async_client_kwargs) kwargs.update( @@ -326,10 +329,13 @@ def _get_client(self, proxy_url: str | None) -> httpx.AsyncClient: } ) + if cookie_jar is not None: + kwargs['cookies'] = cookie_jar + client = httpx.AsyncClient(**kwargs) - self._client_by_proxy_url[proxy_url] = client + self._client_cache[cache_key] = client - return self._client_by_proxy_url[proxy_url] + return self._client_cache[cache_key] def _combine_headers(self, explicit_headers: HttpHeaders | None) -> HttpHeaders | None: """Merge default headers with explicit headers for an HTTP request. @@ -365,9 +371,9 @@ def _is_proxy_error(error: httpx.TransportError) -> bool: return False async def cleanup(self) -> None: - for client in self._client_by_proxy_url.values(): + for client in self._client_cache.values(): await client.aclose() - self._client_by_proxy_url.clear() + self._client_cache.clear() if self._transport: await self._transport.aclose() self._transport = None diff --git a/src/crawlee/http_clients/_impit.py b/src/crawlee/http_clients/_impit.py index 1111cf090a..5aa503c7fa 100644 --- a/src/crawlee/http_clients/_impit.py +++ b/src/crawlee/http_clients/_impit.py @@ -2,8 +2,9 @@ import asyncio from contextlib import asynccontextmanager +from http.cookiejar import CookieJar from logging import getLogger -from typing import TYPE_CHECKING, Any, TypedDict +from typing import TYPE_CHECKING, Any from urllib.request import Request as UrllibRequest from cachetools import LRUCache @@ -21,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 @@ -31,16 +31,6 @@ 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.""" - - client: AsyncClient - cookie_jar: CookieJar | None - class _ImpitResponse: """Adapter class for `impit.Response` to conform to the `HttpResponse` protocol.""" @@ -120,7 +110,7 @@ def __init__( self._async_client_kwargs = async_client_kwargs - self._client_cache = LRUCache[_ClientCacheKey, _ClientCacheEntry](maxsize=10) + self._client_cache = LRUCache[tuple[str | None, CookieJar | None], AsyncClient](maxsize=10) def _prepare_cookies_and_headers( self, @@ -131,9 +121,9 @@ def _prepare_cookies_and_headers( ) -> tuple[CookieJar | None, HttpHeaders]: """Resolve cookie jar / Cookie header based on `persist_cookies_per_session`. - When persistence is enabled, attach the session jar to Impit so response cookies update it. - When persistence is disabled, send existing cookies via the `Cookie` header and keep the - shared client (no jar) so clients stay cached and reusable. + When persistence is enabled, attach the session jar to impit so response cookies update it. When persistence + is disabled, send existing cookies via the Cookie header and keep the shared client (no jar) so clients stay + cached and reusable. """ if isinstance(headers, dict) or headers is None: headers = HttpHeaders(headers or {}) @@ -150,10 +140,9 @@ def _prepare_cookies_and_headers( return None, headers @staticmethod - def _get_cookie_header(jar: CookieJar, url: str, headers: HttpHeaders | None = None) -> str: + def _get_cookie_header(jar: CookieJar, url: str, headers: HttpHeaders) -> str: """Build a Cookie request header from a jar without attaching the jar to the client.""" - # UrllibRequest is only used to format Cookie headers via CookieJar; it never opens a connection. - request = UrllibRequest(url, headers=dict(headers) if headers else {}) # noqa: S310 + request = UrllibRequest(url, headers=dict(headers)) # noqa: S310 jar.add_cookie_header(request) return request.get_header('Cookie', '') @@ -265,24 +254,18 @@ 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) - def _get_client(self, proxy_url: str | None, cookie_jar: CookieJar | None) -> AsyncClient: """Retrieve or create an HTTP client for the given proxy URL and cookie jar. - Clients are cached by `(proxy_url, cookie_jar identity)` so sessions with different jars do not share - a client. When cookie persistence is disabled, cookies are sent via headers and `cookie_jar` is `None`, - so a shared client can be reused for the proxy. + Clients are cached by `(proxy_url, cookie_jar)` — CookieJar hashes by identity so sessions with different + jars get separate clients. When cookie persistence is disabled, `cookie_jar` is `None` and a shared client + is reused for the proxy. """ - cache_key = self._make_cache_key(proxy_url, cookie_jar) + cache_key = (proxy_url, cookie_jar) - cached_data = self._client_cache.get(cache_key) - if cached_data and cached_data['cookie_jar'] is cookie_jar: - return cached_data['client'] + if cache_key in self._client_cache: + return self._client_cache[cache_key] - # Prepare a default kwargs for the new client. kwargs: dict[str, Any] = { 'proxy': proxy_url, 'http3': self._http3, @@ -291,12 +274,10 @@ def _get_client(self, proxy_url: str | None, cookie_jar: CookieJar | None) -> As 'browser': self._browser, } - # Update the default kwargs with any additional user-provided kwargs. kwargs.update(self._async_client_kwargs) client = AsyncClient(**kwargs, cookie_jar=cookie_jar) - - self._client_cache[cache_key] = _ClientCacheEntry(client=client, cookie_jar=cookie_jar) + self._client_cache[cache_key] = client return client diff --git a/tests/unit/http_clients/test_http_clients.py b/tests/unit/http_clients/test_http_clients.py index 592c0eb5ae..64cafd6c36 100644 --- a/tests/unit/http_clients/test_http_clients.py +++ b/tests/unit/http_clients/test_http_clients.py @@ -387,3 +387,26 @@ async def test_persist_cookies_per_session_true(custom_http_client: HttpClient, await custom_http_client.crawl(request, session=session) assert {cookie['name']: cookie['value'] for cookie in session.cookies.get_cookies_as_dicts()} == {'a': '1'} + + +@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_session_cookies_survive_redirect(custom_http_client: HttpClient, server_url: URL) -> None: + """Pre-seeded session cookies must be present after a redirect (not stripped on the second hop).""" + session = Session() + session.cookies.set('tracker', 'abc', domain=server_url.host or '127.0.0.1', path='/') + + cookies_url = str(server_url / 'cookies') + redirect_url = str((server_url / 'redirect').update_query(url=cookies_url)) + + response = await custom_http_client.send_request(redirect_url, session=session) + body = json.loads(await response.read()) + + assert body['cookies']['tracker'] == 'abc' diff --git a/tests/unit/http_clients/test_httpx.py b/tests/unit/http_clients/test_httpx.py index b0a97d8755..0f0bcbcc85 100644 --- a/tests/unit/http_clients/test_httpx.py +++ b/tests/unit/http_clients/test_httpx.py @@ -3,12 +3,11 @@ import json import logging from typing import TYPE_CHECKING -from unittest.mock import patch +from unittest.mock import Mock import pytest from crawlee._types import HttpHeaders -from crawlee.fingerprint_suite import HeaderGenerator from crawlee.fingerprint_suite._browserforge_adapter import get_available_header_values from crawlee.fingerprint_suite._consts import COMMON_ACCEPT_LANGUAGE from crawlee.http_clients import HttpxHttpClient @@ -53,7 +52,6 @@ async def test_common_headers_and_user_agent(server_url: URL, header_network: di assert 'accept-language' in response_headers assert response_headers['accept-language'] == COMMON_ACCEPT_LANGUAGE - # By default, HTTPX uses its own User-Agent, which should be replaced by the one from the header generator. assert 'user-agent' in response_headers assert 'python-httpx' not in response_headers['user-agent'] assert response_headers['user-agent'] in get_available_header_values(header_network, {'User-Agent', 'user-agent'}) @@ -61,14 +59,17 @@ async def test_common_headers_and_user_agent(server_url: URL, header_network: di async def test_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) + header_generator = Mock() + header_generator.get_specific_headers = Mock(return_value=HttpHeaders(fingerprint)) - mocked.assert_called_once_with(header_names={'Accept', 'Accept-Language', 'User-Agent'}) + client = HttpxHttpClient(header_generator=header_generator) + combined = client._combine_headers(None) + + header_generator.get_specific_headers.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' diff --git a/tests/unit/http_clients/test_impit.py b/tests/unit/http_clients/test_impit.py index 08cd577a02..6e022c0af7 100644 --- a/tests/unit/http_clients/test_impit.py +++ b/tests/unit/http_clients/test_impit.py @@ -1,8 +1,10 @@ from __future__ import annotations +import json from typing import TYPE_CHECKING from crawlee.http_clients import ImpitHttpClient +from crawlee.sessions import Session if TYPE_CHECKING: from yarl import URL @@ -14,12 +16,25 @@ async def test_cleanup_clears_client_cache(server_url: URL) -> None: 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'] + first_client = next(iter(client._client_cache.values())) 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'] + second_client = next(iter(client._client_cache.values())) assert second_client is not first_client + + +async def test_persist_false_still_sends_session_cookies(server_url: URL) -> None: + """When persist_cookies_per_session=False, pre-seeded session cookies are still sent via Cookie header.""" + client = ImpitHttpClient(persist_cookies_per_session=False) + session = Session() + session.cookies.set('seed', 'value123', domain=server_url.host or '127.0.0.1', path='/') + + async with client: + response = await client.send_request(str(server_url / 'cookies'), session=session) + body = json.loads(await response.read()) + + assert body['cookies'] == {'seed': 'value123'} From 6aaadc4fed3047f1c6931376e68b1400600d2287 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Tue, 4 Aug 2026 16:37:22 +0530 Subject: [PATCH 5/5] test: assert httpx headers come from a single generate() call Mocking get_specific_headers could not catch a regression that mixes Accept and User-Agent from separate fingerprint generations. --- tests/unit/http_clients/test_httpx.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/unit/http_clients/test_httpx.py b/tests/unit/http_clients/test_httpx.py index 0f0bcbcc85..bff691f9f3 100644 --- a/tests/unit/http_clients/test_httpx.py +++ b/tests/unit/http_clients/test_httpx.py @@ -7,7 +7,7 @@ import pytest -from crawlee._types import HttpHeaders +from crawlee.fingerprint_suite import HeaderGenerator from crawlee.fingerprint_suite._browserforge_adapter import get_available_header_values from crawlee.fingerprint_suite._consts import COMMON_ACCEPT_LANGUAGE from crawlee.http_clients import HttpxHttpClient @@ -57,19 +57,19 @@ async def test_common_headers_and_user_agent(server_url: URL, header_network: di assert response_headers['user-agent'] in get_available_header_values(header_network, {'User-Agent', 'user-agent'}) -async def test_headers_come_from_single_fingerprint() -> None: - """Accept and User-Agent must come from the same generated fingerprint profile.""" +def test_headers_come_from_single_fingerprint() -> None: + """Accept and User-Agent must come from one `generate()` call, not mixed profiles.""" fingerprint = {'Accept': 'text/html', 'Accept-Language': 'en-US', 'User-Agent': 'TestAgent/1.0'} - header_generator = Mock() - header_generator.get_specific_headers = Mock(return_value=HttpHeaders(fingerprint)) + # Avoid HeaderGenerator.__init__ loading browserforge; only exercise get_specific_headers. + header_generator = HeaderGenerator.__new__(HeaderGenerator) + header_generator._generator = Mock() + header_generator._generator.generate = Mock(return_value=fingerprint) client = HttpxHttpClient(header_generator=header_generator) combined = client._combine_headers(None) - header_generator.get_specific_headers.assert_called_once_with( - header_names={'Accept', 'Accept-Language', 'User-Agent'} - ) + header_generator._generator.generate.assert_called_once() assert combined is not None assert combined['accept'] == 'text/html' assert combined['accept-language'] == 'en-US'