diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index 1e95864c6..272ff0dcd 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -279,14 +279,11 @@ If you want to use this handler you need to replace the default ones for the some websites may be different. Additionally, these are the Scrapy features that are explicitly not supported when using it: - - Proxy support (the :reqmeta:`proxy` meta key). - - Per-request bind address support (the :reqmeta:`bindaddress` meta key). The global :setting:`DOWNLOAD_BIND_ADDRESS` setting is supported but the port number, if specified, will be ignored. - - The :setting:`DOWNLOADER_CLIENT_TLS_CIPHERS` and - :setting:`DOWNLOADER_CLIENT_TLS_METHOD` settings. + - The :setting:`DOWNLOADER_CLIENT_TLS_METHOD` setting. - Settings specific to the Twisted networking or HTTP implementation, like :setting:`DNS_RESOLVER`. diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index ba7b3ee3c..d1e4851d9 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -917,7 +917,7 @@ Response objects :type request: scrapy.Request :param certificate: an object representing the server's SSL certificate. - :type certificate: twisted.internet.ssl.Certificate + :type certificate: typing.Any :param ip_address: The IP address of the server from which the Response originated. :type ip_address: :class:`ipaddress.IPv4Address` or :class:`ipaddress.IPv6Address` @@ -1009,8 +1009,8 @@ Response objects .. attribute:: Response.certificate - A :class:`twisted.internet.ssl.Certificate` object representing - the server's SSL certificate. + An object representing the server's SSL certificate. Its type and + contents depend on the download handler that produced the response. Only populated for ``https`` responses, ``None`` otherwise. diff --git a/scrapy/core/downloader/handlers/_base_http.py b/scrapy/core/downloader/handlers/_base_http.py new file mode 100644 index 000000000..83d130462 --- /dev/null +++ b/scrapy/core/downloader/handlers/_base_http.py @@ -0,0 +1,25 @@ +from __future__ import annotations + +from abc import ABC +from typing import TYPE_CHECKING + +from .base import BaseDownloadHandler + +if TYPE_CHECKING: + from scrapy.crawler import Crawler + + +class BaseHttpDownloadHandler(BaseDownloadHandler, ABC): + """Base class for built-in HTTP download handlers.""" + + def __init__(self, crawler: Crawler): + super().__init__(crawler) + self._default_maxsize: int = crawler.settings.getint("DOWNLOAD_MAXSIZE") + self._default_warnsize: int = crawler.settings.getint("DOWNLOAD_WARNSIZE") + self._fail_on_dataloss: bool = crawler.settings.getbool( + "DOWNLOAD_FAIL_ON_DATALOSS" + ) + self._tls_verbose_logging: bool = crawler.settings.getbool( + "DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING" + ) + self._fail_on_dataloss_warned: bool = False diff --git a/scrapy/core/downloader/handlers/_base_streaming.py b/scrapy/core/downloader/handlers/_base_streaming.py new file mode 100644 index 000000000..16b655716 --- /dev/null +++ b/scrapy/core/downloader/handlers/_base_streaming.py @@ -0,0 +1,307 @@ +from __future__ import annotations + +import base64 +import logging +import time +from abc import ABC, abstractmethod +from io import BytesIO +from typing import TYPE_CHECKING, Any, ClassVar, Generic, NoReturn, TypedDict, TypeVar +from urllib.parse import quote, urlsplit + +from scrapy import Request, signals +from scrapy.exceptions import ( + DownloadCancelledError, + NotConfigured, + ResponseDataLossError, +) +from scrapy.utils._download_handlers import ( + check_stop_download, + get_dataloss_msg, + get_maxsize_msg, + get_warnsize_msg, + make_response, + normalize_bind_address, +) +from scrapy.utils.asyncio import is_asyncio_available +from scrapy.utils.url import add_http_if_no_scheme + +from ._base_http import BaseHttpDownloadHandler + +if TYPE_CHECKING: + from collections.abc import AsyncIterable + from contextlib import AbstractAsyncContextManager + from ipaddress import IPv4Address, IPv6Address + + from _typeshed import SizedBuffer + + # typing.NotRequired requires Python 3.11 + from typing_extensions import NotRequired + + from scrapy.crawler import Crawler + from scrapy.http import Headers, Response + + +logger = logging.getLogger(__name__) + +_ResponseT = TypeVar("_ResponseT") + + +class _BaseResponseArgs(TypedDict): + status: int + url: str + headers: Headers + certificate: NotRequired[Any] + ip_address: NotRequired[IPv4Address | IPv6Address | None] + protocol: str | None + + +class BaseStreamingDownloadHandler(BaseHttpDownloadHandler, ABC, Generic[_ResponseT]): + """A base class for HTTP download handlers that follow the streaming logic flow.""" + + _DEFAULT_CONNECT_TIMEOUT: ClassVar[float] = 10 + experimental: ClassVar[bool] = False + requires_asyncio: ClassVar[bool] = True + # require subclasses to disable proxies explicitly with an explanation + supports_proxies: ClassVar[bool] = True + supports_per_request_bindaddress: ClassVar[bool] = False + + def __init__(self, crawler: Crawler): + if self.requires_asyncio and not is_asyncio_available(): # pragma: no cover + raise NotConfigured( + f"{type(self).__name__} requires the asyncio support. Make" + f" sure that you have either enabled the asyncio Twisted" + f" reactor in the TWISTED_REACTOR setting or disabled the" + f" TWISTED_REACTOR_ENABLED setting. See the asyncio documentation" + f" of Scrapy for more information." + ) + self._check_deps_installed() + super().__init__(crawler) + if self.experimental: + logger.warning( + f"{type(self).__name__} is experimental and is not recommended for production use." + ) + self._bind_address = normalize_bind_address( + crawler.settings.get("DOWNLOAD_BIND_ADDRESS") + ) + self._proxy_auth_encoding: str = crawler.settings.get("HTTPPROXY_AUTH_ENCODING") + # these are useful for many handlers but used in different ways by them + self._pool_size_total: int = crawler.settings.getint("CONCURRENT_REQUESTS") + self._pool_size_per_host: int = crawler.settings.getint( + "CONCURRENT_REQUESTS_PER_DOMAIN" + ) + + @staticmethod + @abstractmethod + def _check_deps_installed() -> None: + """Raise NotConfigured if the required deps are not installed.""" + raise NotImplementedError + + @abstractmethod + def _make_request( + self, request: Request, timeout: float + ) -> AbstractAsyncContextManager[_ResponseT]: + """Return an async context manager yielding the library-specific response. + + Exceptions raised by the library should be reraised as Scrapy-specific ones. + """ + raise NotImplementedError + + @staticmethod + @abstractmethod + def _extract_headers(response: _ResponseT) -> Headers: + """Convert library-specific response headers to a + :class:`~scrapy.http.headers.Headers` object.""" + raise NotImplementedError + + @staticmethod + @abstractmethod + def _build_base_response_args( + response: _ResponseT, request: Request, headers: Headers + ) -> _BaseResponseArgs: + """Build kwargs for :func:`scrapy.utils._download_handlers.make_response`.""" + raise NotImplementedError + + @staticmethod + @abstractmethod + def _iter_body_chunks(response: _ResponseT) -> AsyncIterable[SizedBuffer]: + """Return an async iterable yielding body chunks from the response.""" + raise NotImplementedError + + @staticmethod + @abstractmethod + def _is_dataloss_exception(exc: Exception) -> bool: + """Return True if ``exc`` represents dataloss.""" + raise NotImplementedError + + def _log_tls_info(self, response: _ResponseT, request: Request) -> None: + """Log TLS connection details, if possible.""" + + async def download_request(self, request: Request) -> Response: + if not self.supports_proxies and request.meta.get("proxy"): + raise NotImplementedError(f"{type(self).__name__} doesn't support proxies.") + if not self.supports_per_request_bindaddress and request.meta.get( + "bindaddress" + ): + logger.error( + f"The 'bindaddress' request meta key is not supported by" + f" {type(self).__name__} and will be ignored." + ) + timeout: float = request.meta.get( + "download_timeout", self._DEFAULT_CONNECT_TIMEOUT + ) + start_time = time.monotonic() + async with self._make_request(request, timeout) as response: + request.meta["download_latency"] = time.monotonic() - start_time + return await self._read_response(response, request) + + async def _read_response(self, response: _ResponseT, request: Request) -> Response: + maxsize: int = request.meta.get("download_maxsize", self._default_maxsize) + warnsize: int = request.meta.get("download_warnsize", self._default_warnsize) + + headers = self._extract_headers(response) + content_length = headers.get("Content-Length") + expected_size = int(content_length) if content_length is not None else None + if maxsize and expected_size and expected_size > maxsize: + self._cancel_maxsize(expected_size, maxsize, request, expected=True) + + reached_warnsize = False + if warnsize and expected_size and expected_size > warnsize: + reached_warnsize = True + logger.warning( + get_warnsize_msg(expected_size, warnsize, request, expected=True) + ) + + make_response_base_args = self._build_base_response_args( + response, request, headers + ) + + if self._tls_verbose_logging: + self._log_tls_info(response, request) + + if stop_download := check_stop_download( + signals.headers_received, + self.crawler, + request, + headers=headers, + body_length=expected_size, + ): + return make_response( + **make_response_base_args, + stop_download=stop_download, + ) + + response_body = BytesIO() + bytes_received = 0 + try: + async for chunk in self._iter_body_chunks(response): + response_body.write(chunk) + bytes_received += len(chunk) + + if stop_download := check_stop_download( + signals.bytes_received, self.crawler, request, data=chunk + ): + return make_response( + **make_response_base_args, + body=response_body.getvalue(), + stop_download=stop_download, + ) + + if maxsize and bytes_received > maxsize: + response_body.truncate(0) + self._cancel_maxsize( + bytes_received, maxsize, request, expected=False + ) + + if warnsize and bytes_received > warnsize and not reached_warnsize: + reached_warnsize = True + logger.warning( + get_warnsize_msg( + bytes_received, warnsize, request, expected=False + ) + ) + except Exception as e: + if not self._is_dataloss_exception(e): + raise + fail_on_dataloss: bool = request.meta.get( + "download_fail_on_dataloss", self._fail_on_dataloss + ) + if not fail_on_dataloss: + return make_response( + **make_response_base_args, + body=response_body.getvalue(), + flags=["dataloss"], + ) + if not self._fail_on_dataloss_warned: + logger.warning(get_dataloss_msg(request.url)) + self._fail_on_dataloss_warned = True + raise ResponseDataLossError(str(e)) from e + + return make_response( + **make_response_base_args, + body=response_body.getvalue(), + ) + + def _get_bind_address_host(self) -> str | None: + """Return the host portion of the bind address. + + Needed for handlers that don't support the bind port. + """ + if self._bind_address is None: + return None + host, port = self._bind_address + if port != 0: + logger.warning( + "DOWNLOAD_BIND_ADDRESS specifies a port (%s), but %s does not " + "support binding to a specific local port. Ignoring the port " + "and binding only to %r.", + port, + type(self).__name__, + host, + ) + return host + + @staticmethod + def _cancel_maxsize( + size: int, limit: int, request: Request, *, expected: bool + ) -> NoReturn: + warning_msg = get_maxsize_msg(size, limit, request, expected=expected) + logger.warning(warning_msg) + raise DownloadCancelledError(warning_msg) + + @staticmethod + def _extract_proxy(request: Request) -> tuple[str | None, str | None]: + """Return a tuple of the proxy URL with a scheme and the value of the + Proxy-Authorization header. + + This is useful for handlers that take the proxy headers separately. + """ + proxy: str | None = request.meta.get("proxy") + if not proxy: + return None, None + proxy = add_http_if_no_scheme(proxy) + auth_header: list[bytes] | None = request.headers.pop( + b"Proxy-Authorization", None + ) + return proxy, auth_header[0].decode("ascii") if auth_header else None + + def _extract_proxy_url_with_creds(self, request: Request) -> str | None: + """Return the proxy URL with the userinfo added based on the + Proxy-Authorization header. + + This is useful for handlers that cannot take the proxy headers + separately. + """ + proxy_url, auth_header = self._extract_proxy(request) + if proxy_url is None or auth_header is None: + return proxy_url + scheme, token = auth_header.split(" ", 1) + if scheme != "Basic": + raise ValueError( + f"Expected Basic auth in Proxy-Authorization, got {scheme}" + ) + user, password = ( + base64.b64decode(token).decode(self._proxy_auth_encoding).split(":", 1) + ) + parts = urlsplit(proxy_url) + netloc = f"{quote(user)}:{quote(password)}@{parts.netloc}" + return parts._replace(netloc=netloc).geturl() diff --git a/scrapy/core/downloader/handlers/_httpx.py b/scrapy/core/downloader/handlers/_httpx.py index 313fb1c4f..43e1cd965 100644 --- a/scrapy/core/downloader/handlers/_httpx.py +++ b/scrapy/core/downloader/handlers/_httpx.py @@ -3,45 +3,34 @@ from __future__ import annotations import ipaddress -import logging import ssl -import time -from http.cookiejar import Cookie, CookieJar -from io import BytesIO -from typing import TYPE_CHECKING, Any, NoReturn, TypedDict +from contextlib import asynccontextmanager +from typing import TYPE_CHECKING, ClassVar -from scrapy import Request, signals from scrapy.exceptions import ( CannotResolveHostError, - DownloadCancelledError, DownloadConnectionRefusedError, DownloadFailedError, DownloadTimeoutError, NotConfigured, - ResponseDataLossError, UnsupportedURLSchemeError, ) -from scrapy.http import Headers, Response -from scrapy.utils._download_handlers import ( - BaseHttpDownloadHandler, - check_stop_download, - get_dataloss_msg, - get_maxsize_msg, - get_warnsize_msg, - make_response, - normalize_bind_address, +from scrapy.http import Headers +from scrapy.utils._download_handlers import NullCookieJar +from scrapy.utils.ssl import ( + _log_sslobj_debug_info, + _make_insecure_ssl_ctx, + _make_ssl_context, ) -from scrapy.utils.asyncio import is_asyncio_available -from scrapy.utils.ssl import _log_sslobj_debug_info, _make_ssl_context + +from ._base_streaming import BaseStreamingDownloadHandler, _BaseResponseArgs if TYPE_CHECKING: - from contextlib import AbstractAsyncContextManager - from http.client import HTTPResponse - from ipaddress import IPv4Address, IPv6Address - from urllib.request import Request as ULRequest + from collections.abc import AsyncIterator from httpcore import AsyncNetworkStream + from scrapy import Request from scrapy.crawler import Crawler @@ -50,89 +39,90 @@ try: except ImportError: httpx = None # type: ignore[assignment] -logger = logging.getLogger(__name__) + +if TYPE_CHECKING: + _Base = BaseStreamingDownloadHandler[httpx.Response] +else: + _Base = BaseStreamingDownloadHandler -class _BaseResponseArgs(TypedDict): - status: int - url: str - headers: Headers - ip_address: IPv4Address | IPv6Address - protocol: str - - -# workaround for (and from) https://github.com/encode/httpx/issues/2992 -class _NullCookieJar(CookieJar): # pragma: no cover - """A CookieJar that rejects all cookies.""" - - def extract_cookies(self, response: HTTPResponse, request: ULRequest) -> None: - pass - - def set_cookie(self, cookie: Cookie) -> None: - pass - - -class HttpxDownloadHandler(BaseHttpDownloadHandler): - _DEFAULT_CONNECT_TIMEOUT = 10 +class HttpxDownloadHandler(_Base): + experimental: ClassVar[bool] = True def __init__(self, crawler: Crawler): - # we skip HttpxDownloadHandler tests with the non-asyncio reactor - if not is_asyncio_available(): # pragma: no cover - raise NotConfigured( - f"{type(self).__name__} requires the asyncio support. Make" - f" sure that you have either enabled the asyncio Twisted" - f" reactor in the TWISTED_REACTOR setting or disabled the" - f" TWISTED_REACTOR_ENABLED setting. See the asyncio" - f" documentation of Scrapy for more information." - ) + super().__init__(crawler) + self._verify_certificates: bool = crawler.settings.getbool( + "DOWNLOAD_VERIFY_CERTIFICATES" + ) + self._ssl_context: ssl.SSLContext = _make_ssl_context(crawler.settings) + self._bind_host: str | None = self._get_bind_address_host() + self._limits: httpx.Limits = httpx.Limits( + # hard limit on simultaneous connections + max_connections=self._pool_size_total, + # total number of idle connections in the pool (extra ones are closed) + max_keepalive_connections=self._pool_size_total, + ) + + self._default_client: httpx.AsyncClient = self._make_client() + # httpx doesn't support per-request proxies: https://github.com/encode/httpx/discussions/3183, + # so we keep a pool of clients per proxy URL. LRU eviction can be added here if needed. + self._proxy_clients: dict[str, httpx.AsyncClient] = {} + + @staticmethod + def _check_deps_installed() -> None: if httpx is None: # pragma: no cover raise NotConfigured( - f"{type(self).__name__} requires the httpx library to be installed." + "HttpxDownloadHandler requires the httpx library to be installed." ) - super().__init__(crawler) - logger.warning( - "HttpxDownloadHandler is experimental and is not recommended for production use." - ) - bind_address = crawler.settings.get("DOWNLOAD_BIND_ADDRESS") - bind_address = normalize_bind_address(bind_address) - self._bind_address: str | None = None + def _make_client(self, proxy_url: str | None = None) -> httpx.AsyncClient: + if proxy_url: + if proxy_url.startswith("https:") and not self._verify_certificates: + proxy_ssl_context = _make_insecure_ssl_ctx() + else: + proxy_ssl_context = None + proxy = httpx.Proxy(proxy_url, ssl_context=proxy_ssl_context) + else: + proxy = None - if bind_address is not None: - host, port = bind_address - if port != 0: - logger.warning( - "DOWNLOAD_BIND_ADDRESS specifies a port (%s), but %s does not " - "support binding to a specific local port. Ignoring the port " - "and binding only to %r.", - port, - type(self).__name__, - host, - ) - self._bind_address = host - - self._client = httpx.AsyncClient( - cookies=_NullCookieJar(), + client = httpx.AsyncClient( + cookies=NullCookieJar(), transport=httpx.AsyncHTTPTransport( - verify=_make_ssl_context(crawler.settings), - local_address=self._bind_address, + verify=self._ssl_context, + local_address=self._bind_host, + limits=self._limits, + trust_env=False, + proxy=proxy, ), ) # https://github.com/encode/httpx/discussions/1566 for header_name in ("accept", "accept-encoding", "user-agent"): - self._client.headers.pop(header_name, None) + client.headers.pop(header_name, None) + return client - async def download_request(self, request: Request) -> Response: - self._warn_unsupported_meta(request.meta) + def _get_client(self, proxy_url: str | None) -> httpx.AsyncClient: + if proxy_url is None: + return self._default_client + if cached := self._proxy_clients.get(proxy_url): + return cached + client = self._make_client(proxy_url) + self._proxy_clients[proxy_url] = client + return client - timeout: float = request.meta.get( - "download_timeout", self._DEFAULT_CONNECT_TIMEOUT - ) - start_time = time.monotonic() + @asynccontextmanager + async def _make_request( + self, request: Request, timeout: float + ) -> AsyncIterator[httpx.Response]: + client = self._get_client(self._extract_proxy_url_with_creds(request)) try: - async with self._get_httpx_response(request, timeout) as httpx_response: - request.meta["download_latency"] = time.monotonic() - start_time - return await self._read_response(httpx_response, request) + async with client.stream( + request.method, + request.url, + content=request.body, + headers=request.headers.to_tuple_list(), + timeout=timeout, + ) as response: + yield response except httpx.TimeoutException as e: raise DownloadTimeoutError( f"Getting {request.url} took longer than {timeout} seconds." @@ -149,159 +139,55 @@ class HttpxDownloadHandler(BaseHttpDownloadHandler): ): raise CannotResolveHostError(error_message) from e raise DownloadConnectionRefusedError(str(e)) from e - except httpx.NetworkError as e: + except httpx.ProxyError as e: + raise DownloadConnectionRefusedError(str(e)) from e + except (httpx.NetworkError, httpx.RemoteProtocolError) as e: raise DownloadFailedError(str(e)) from e - except httpx.RemoteProtocolError as e: - raise DownloadFailedError(str(e)) from e - - def _warn_unsupported_meta(self, meta: dict[str, Any]) -> None: - if meta.get("bindaddress"): - # configurable only per-client: - # https://github.com/encode/httpx/issues/755#issuecomment-2746121794 - logger.error( - f"The 'bindaddress' request meta key is not supported by" - f" {type(self).__name__} and will be ignored." - ) - if meta.get("proxy"): - # configurable only per-client: - # https://github.com/encode/httpx/issues/486 - logger.error( - f"The 'proxy' request meta key is not supported by" - f" {type(self).__name__} and will be ignored." - ) - - def _get_httpx_response( - self, request: Request, timeout: float - ) -> AbstractAsyncContextManager[httpx.Response]: - return self._client.stream( - request.method, - request.url, - content=request.body, - headers=request.headers.to_tuple_list(), - timeout=timeout, - ) - - async def _read_response( - self, httpx_response: httpx.Response, request: Request - ) -> Response: - maxsize: int = request.meta.get("download_maxsize", self._default_maxsize) - warnsize: int = request.meta.get("download_warnsize", self._default_warnsize) - - content_length = httpx_response.headers.get("Content-Length") - expected_size = int(content_length) if content_length is not None else None - if maxsize and expected_size and expected_size > maxsize: - self._cancel_maxsize(expected_size, maxsize, request, expected=True) - - reached_warnsize = False - if warnsize and expected_size and expected_size > warnsize: - reached_warnsize = True - logger.warning( - get_warnsize_msg(expected_size, warnsize, request, expected=True) - ) - - headers = Headers(httpx_response.headers.multi_items()) - network_stream: AsyncNetworkStream = httpx_response.extensions["network_stream"] - - make_response_base_args: _BaseResponseArgs = { - "status": httpx_response.status_code, - "url": request.url, - "headers": headers, - "ip_address": self._get_server_ip(network_stream), - "protocol": httpx_response.http_version, - } - - self._log_tls_info(network_stream) - - if stop_download := check_stop_download( - signals.headers_received, - self.crawler, - request, - headers=headers, - body_length=expected_size, - ): - return make_response( - **make_response_base_args, - stop_download=stop_download, - ) - - response_body = BytesIO() - bytes_received = 0 - try: - async for chunk in httpx_response.aiter_raw(): - response_body.write(chunk) - bytes_received += len(chunk) - - if stop_download := check_stop_download( - signals.bytes_received, self.crawler, request, data=chunk - ): - return make_response( - **make_response_base_args, - body=response_body.getvalue(), - stop_download=stop_download, - ) - - if maxsize and bytes_received > maxsize: - response_body.truncate(0) - self._cancel_maxsize( - bytes_received, maxsize, request, expected=False - ) - - if warnsize and bytes_received > warnsize and not reached_warnsize: - reached_warnsize = True - logger.warning( - get_warnsize_msg( - bytes_received, warnsize, request, expected=False - ) - ) - except httpx.RemoteProtocolError as e: - # special handling of the dataloss case - if ( - "peer closed connection without sending complete message body" - not in str(e) - ): - raise - fail_on_dataloss: bool = request.meta.get( - "download_fail_on_dataloss", self._fail_on_dataloss - ) - if not fail_on_dataloss: - return make_response( - **make_response_base_args, - body=response_body.getvalue(), - flags=["dataloss"], - ) - self._log_dataloss_warning(request.url) - raise ResponseDataLossError(str(e)) from e - - return make_response( - **make_response_base_args, - body=response_body.getvalue(), - ) @staticmethod - def _get_server_ip(network_stream: AsyncNetworkStream) -> IPv4Address | IPv6Address: - extra_server_addr = network_stream.get_extra_info("server_addr") - return ipaddress.ip_address(extra_server_addr[0]) + def _extract_headers(response: httpx.Response) -> Headers: + return Headers(response.headers.multi_items()) - def _log_tls_info(self, network_stream: AsyncNetworkStream) -> None: - if not self._tls_verbose_logging: - return + @staticmethod + def _build_base_response_args( + response: httpx.Response, + request: Request, + headers: Headers, + ) -> _BaseResponseArgs: + network_stream: AsyncNetworkStream = response.extensions["network_stream"] + server_addr = network_stream.get_extra_info("server_addr") + ip_address = ipaddress.ip_address(server_addr[0]) + ssl_object = network_stream.get_extra_info("ssl_object") + if isinstance(ssl_object, ssl.SSLObject): + cert = ssl_object.getpeercert(binary_form=True) + else: + cert = None + return { + "status": response.status_code, + "url": request.url, + "headers": headers, + "certificate": cert, + "ip_address": ip_address, + "protocol": response.http_version, + } + + @staticmethod + def _iter_body_chunks(response: httpx.Response) -> AsyncIterator[bytes]: + return response.aiter_raw() + + @staticmethod + def _is_dataloss_exception(exc: Exception) -> bool: + return isinstance( + exc, httpx.RemoteProtocolError + ) and "peer closed connection without sending complete message body" in str(exc) + + def _log_tls_info(self, response: httpx.Response, request: Request) -> None: + network_stream: AsyncNetworkStream = response.extensions["network_stream"] extra_ssl_object = network_stream.get_extra_info("ssl_object") if isinstance(extra_ssl_object, ssl.SSLObject): _log_sslobj_debug_info(extra_ssl_object) - def _log_dataloss_warning(self, url: str) -> None: - if self._fail_on_dataloss_warned: - return - logger.warning(get_dataloss_msg(url)) - self._fail_on_dataloss_warned = True - - @staticmethod - def _cancel_maxsize( - size: int, limit: int, request: Request, *, expected: bool - ) -> NoReturn: - warning_msg = get_maxsize_msg(size, limit, request, expected=expected) - logger.warning(warning_msg) - raise DownloadCancelledError(warning_msg) - async def close(self) -> None: - await self._client.aclose() + await self._default_client.aclose() + for client in self._proxy_clients.values(): + await client.aclose() diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index fbe24e52e..85a5ae0d7 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -41,7 +41,6 @@ from scrapy.exceptions import ( ) from scrapy.http import Headers, Response from scrapy.utils._download_handlers import ( - BaseHttpDownloadHandler, check_stop_download, get_dataloss_msg, get_maxsize_msg, @@ -57,6 +56,8 @@ from scrapy.utils.python import to_bytes, to_unicode from scrapy.utils.ssl import _log_ssl_conn_debug_info from scrapy.utils.url import add_http_if_no_scheme +from ._base_http import BaseHttpDownloadHandler + if TYPE_CHECKING: from twisted.internet.base import ReactorBase from twisted.internet.interfaces import IConsumer diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index a80ea3da8..09b1c8b32 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -20,7 +20,6 @@ if TYPE_CHECKING: from collections.abc import Callable, Iterable, Mapping from ipaddress import IPv4Address, IPv6Address - from twisted.internet.ssl import Certificate from twisted.python.failure import Failure # typing.Self requires Python 3.11 @@ -77,7 +76,7 @@ class Response(object_ref): body: bytes = b"", flags: list[str] | None = None, request: Request | None = None, - certificate: Certificate | None = None, + certificate: Any = None, ip_address: IPv4Address | IPv6Address | None = None, protocol: str | None = None, ): @@ -87,7 +86,7 @@ class Response(object_ref): self._set_url(url) self.request: Request | None = request self._flags: list[str] | None = list(flags) if flags else None - self.certificate: Certificate | None = certificate + self.certificate: Any = certificate self.ip_address: IPv4Address | IPv6Address | None = ip_address self.protocol: str | None = protocol diff --git a/scrapy/utils/_download_handlers.py b/scrapy/utils/_download_handlers.py index 9538dd81e..23b715ac6 100644 --- a/scrapy/utils/_download_handlers.py +++ b/scrapy/utils/_download_handlers.py @@ -2,8 +2,8 @@ from __future__ import annotations -from abc import ABC from contextlib import contextmanager +from http.cookiejar import CookieJar from typing import TYPE_CHECKING, Any from twisted.internet.defer import CancelledError @@ -15,7 +15,6 @@ from twisted.web.client import ResponseFailed from twisted.web.error import SchemeNotSupported from scrapy import responsetypes -from scrapy.core.downloader.handlers.base import BaseDownloadHandler from scrapy.exceptions import ( CannotResolveHostError, DownloadCancelledError, @@ -29,29 +28,24 @@ from scrapy.utils.log import logger if TYPE_CHECKING: from collections.abc import Iterator + from http.client import HTTPResponse + from http.cookiejar import Cookie from ipaddress import IPv4Address, IPv6Address - - from twisted.internet.ssl import Certificate + from urllib.request import Request as ULRequest from scrapy import Request from scrapy.crawler import Crawler from scrapy.http import Headers, Response -class BaseHttpDownloadHandler(BaseDownloadHandler, ABC): - """Base class for built-in HTTP download handlers.""" +class NullCookieJar(CookieJar): # pragma: no cover + """A CookieJar that rejects all cookies.""" - def __init__(self, crawler: Crawler): - super().__init__(crawler) - self._default_maxsize: int = crawler.settings.getint("DOWNLOAD_MAXSIZE") - self._default_warnsize: int = crawler.settings.getint("DOWNLOAD_WARNSIZE") - self._fail_on_dataloss: bool = crawler.settings.getbool( - "DOWNLOAD_FAIL_ON_DATALOSS" - ) - self._tls_verbose_logging: bool = crawler.settings.getbool( - "DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING" - ) - self._fail_on_dataloss_warned: bool = False + def extract_cookies(self, response: HTTPResponse, request: ULRequest) -> None: + pass + + def set_cookie(self, cookie: Cookie) -> None: + pass @contextmanager @@ -103,7 +97,7 @@ def make_response( headers: Headers, body: bytes = b"", flags: list[str] | None = None, - certificate: Certificate | None = None, + certificate: Any = None, ip_address: IPv4Address | IPv6Address | None = None, protocol: str | None = None, stop_download: StopDownload | None = None, diff --git a/scrapy/utils/ssl.py b/scrapy/utils/ssl.py index a8b9cd225..3fa2c77ba 100644 --- a/scrapy/utils/ssl.py +++ b/scrapy/utils/ssl.py @@ -54,6 +54,18 @@ def _make_ssl_context(settings: BaseSettings) -> ssl.SSLContext: return ctx +def _make_insecure_ssl_ctx() -> ssl.SSLContext: + """Create an SSL context that doesn't verify certificates. + + Compared to :func:`~scrapy.utils.ssl._make_ssl_context` this is much more + simple. + """ + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + ctx.check_hostname = False + ctx.verify_mode = ssl.CERT_NONE + return ctx + + def _log_sslobj_debug_info(sslobj: ssl.SSLObject) -> None: cipher = sslobj.cipher() logger.debug( @@ -61,8 +73,11 @@ def _log_sslobj_debug_info(sslobj: ssl.SSLObject) -> None: f" using protocol {sslobj.version()}," f" cipher {cipher[0] if cipher else None}" ) - # The peer certificate is unavailable on SSLObject unless peer - # certificate verification is enabled, which we don't want. + if cert := sslobj.getpeercert(): + # Not available without certificate verification + logger.debug( + f'SSL connection certificate: issuer "{cert["issuer"]}", subject "{cert["subject"]}"' + ) # pyOpenSSL utils diff --git a/tests/test_crawl.py b/tests/test_crawl.py index 811ce1d18..52abb1ccc 100644 --- a/tests/test_crawl.py +++ b/tests/test_crawl.py @@ -8,6 +8,7 @@ from typing import TYPE_CHECKING, Any from urllib.parse import urlencode, urlparse import pytest +from cryptography.x509 import load_der_x509_certificate from testfixtures import LogCapture from twisted.internet.defer import succeed from twisted.internet.ssl import Certificate @@ -641,11 +642,6 @@ class TestCrawlSpider: yield crawler.crawl(seed=url, mockserver=self.mockserver) assert crawler.spider.meta["responses"][0].certificate is None - @pytest.mark.xfail( - 'config.getoption("--reactor") == "none"', - reason="Not implemented in HttpxDownloadHandler", - strict=True, - ) @pytest.mark.parametrize( "url", [ @@ -669,9 +665,14 @@ class TestCrawlSpider: await crawler.crawl_async(seed=url, mockserver=mockserver) assert isinstance(crawler.spider, SingleRequestSpider) cert = crawler.spider.meta["responses"][0].certificate - assert isinstance(cert, Certificate) - assert cert.getSubject().commonName == b"localhost" - assert cert.getIssuer().commonName == b"localhost" + assert cert is not None + if isinstance(cert, Certificate): # Twisted + assert cert.getSubject().commonName == b"localhost" + assert cert.getIssuer().commonName == b"localhost" + elif isinstance(cert, bytes): # DER bytes + cert_x509 = load_der_x509_certificate(cert) + assert cert_x509.subject.rfc4514_string() == "CN=localhost,O=Scrapy,C=IE" + assert cert_x509.issuer.rfc4514_string() == "CN=localhost,O=Scrapy,C=IE" @pytest.mark.parametrize( "url", diff --git a/tests/test_downloader_handler_httpx.py b/tests/test_downloader_handler_httpx.py index bc1acf9f9..f26a17f02 100644 --- a/tests/test_downloader_handler_httpx.py +++ b/tests/test_downloader_handler_httpx.py @@ -36,7 +36,6 @@ pytest.importorskip("httpx") class HttpxDownloadHandlerMixin: @property def download_handler_cls(self) -> type[DownloadHandlerProtocol]: - # the import will fail if httpx is not installed from scrapy.core.downloader.handlers._httpx import ( # noqa: PLC0415 HttpxDownloadHandler, ) @@ -73,27 +72,13 @@ class TestHttp(HttpxDownloadHandlerMixin, TestHttpBase): assert "DOWNLOAD_BIND_ADDRESS specifies a port (12345)" in caplog.text assert "Ignoring the port" in caplog.text - @coroutine_test - async def test_unsupported_proxy( - self, caplog: pytest.LogCaptureFixture, mockserver: MockServer - ) -> None: - meta = {"proxy": "127.0.0.2"} - request = Request(mockserver.url("/text"), meta=meta) - async with self.get_dh() as download_handler: - response = await download_handler.download_request(request) - assert response.body == b"Works" - assert ( - "The 'proxy' request meta key is not supported by HttpxDownloadHandler" - in caplog.text - ) - class TestHttps(HttpxDownloadHandlerMixin, TestHttpsBase): handler_supports_bindaddress_meta = False tls_log_message = "SSL connection to 127.0.0.1 using protocol TLSv1.3, cipher" @pytest.mark.skip(reason="The check is Twisted-specific") - def test_verify_certs_deprecated(self): + def test_verify_certs_deprecated(self) -> None: # type: ignore[override] pass @@ -126,23 +111,15 @@ class TestHttpWithCrawler(HttpxDownloadHandlerMixin, TestHttpWithCrawlerBase): class TestHttpsWithCrawler(TestHttpWithCrawler): is_secure = True - @pytest.mark.skip(reason="response.certificate is not implemented") - @coroutine_test - async def test_response_ssl_certificate(self, mockserver: MockServer) -> None: - pass - -@pytest.mark.skip(reason="Proxy support is not implemented yet") class TestHttpProxy(HttpxDownloadHandlerMixin, TestHttpProxyBase): - pass + expected_http_proxy_request_body = b"http://example.com/" -@pytest.mark.skip(reason="Proxy support is not implemented yet") -class TestHttpsProxy(HttpxDownloadHandlerMixin, TestHttpProxyBase): +class TestHttpsProxy(TestHttpProxy): is_secure = True -@pytest.mark.skip(reason="Proxy support is not implemented yet") @pytest.mark.requires_mitmproxy class TestMitmProxy(HttpxDownloadHandlerMixin, TestMitmProxyBase): pass diff --git a/tests/test_downloader_handlers_http_base.py b/tests/test_downloader_handlers_http_base.py index a8d002d30..51da8b38f 100644 --- a/tests/test_downloader_handlers_http_base.py +++ b/tests/test_downloader_handlers_http_base.py @@ -17,6 +17,7 @@ from typing import TYPE_CHECKING, Any, ClassVar from urllib.parse import urlparse import pytest +from cryptography.x509 import load_der_x509_certificate from twisted.internet.ssl import Certificate from twisted.python.failure import Failure @@ -981,15 +982,19 @@ class TestHttpWithCrawlerBase(ABC): if not self.is_secure: pytest.skip("Only applies to HTTPS") # copy of TestCrawl.test_response_ssl_certificate() - # the current test implementation can only work for Twisted-based download handlers crawler = get_crawler(SingleRequestSpider, self.settings_dict) url = mockserver.url("/echo?body=test", is_secure=self.is_secure) await crawler.crawl_async(seed=url, mockserver=mockserver) assert isinstance(crawler.spider, SingleRequestSpider) cert = crawler.spider.meta["responses"][0].certificate - assert isinstance(cert, Certificate) - assert cert.getSubject().commonName == b"localhost" - assert cert.getIssuer().commonName == b"localhost" + assert cert is not None + if isinstance(cert, Certificate): # Twisted + assert cert.getSubject().commonName == b"localhost" + assert cert.getIssuer().commonName == b"localhost" + elif isinstance(cert, bytes): # DER bytes + cert_x509 = load_der_x509_certificate(cert) + assert cert_x509.subject.rfc4514_string() == "CN=localhost,O=Scrapy,C=IE" + assert cert_x509.issuer.rfc4514_string() == "CN=localhost,O=Scrapy,C=IE" @coroutine_test async def test_response_ip_address(self, mockserver: MockServer) -> None: @@ -1279,7 +1284,6 @@ class TestRealWebsiteBase(ABC): raise NotImplementedError @property - @abstractmethod def platform_cert_store_works(self) -> bool: """Whether valid certificates can be verified. @@ -1331,3 +1335,22 @@ class TestRealWebsiteBase(ABC): response = await download_handler.download_request(request) assert response.status == 200 assert "All products | Books to Scrape - Sandbox" in response.text + + @pytest.mark.parametrize("verify_certs", [True, False]) + @coroutine_test + async def test_tls_logging( + self, caplog: pytest.LogCaptureFixture, verify_certs: bool + ) -> None: + if verify_certs and not self.platform_cert_store_works: + pytest.skip("Cannot verify certificates") + request = Request("https://books.toscrape.com/") + async with self.get_dh( + { + "DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING": True, + "DOWNLOAD_VERIFY_CERTIFICATES": verify_certs, + } + ) as download_handler: + with caplog.at_level("DEBUG"): + response = await download_handler.download_request(request) + assert response.status == 200 + assert "SSL connection to books.toscrape.com using protocol" in caplog.text diff --git a/tox.ini b/tox.ini index 72686d3d0..da5a8a9a6 100644 --- a/tox.ini +++ b/tox.ini @@ -113,7 +113,7 @@ deps = Twisted==21.7.0 cryptography==37.0.0 cssselect==0.9.1 - httpx==0.26.0 + httpx==0.27.1 itemadapter==0.1.0 lxml==4.6.4 parsel==1.5.0 @@ -165,7 +165,7 @@ deps = brotli==1.2.0; implementation_name != "pypy" brotlicffi==1.2.0.0; implementation_name == "pypy" google-cloud-storage==1.29.0 - httpx==0.26.0 + httpx==0.27.1 ipython==7.1.0 robotexclusionrulesparser==1.6.2 uvloop==0.16.0; platform_system != "Windows" and implementation_name != "pypy"