From fa9a9033f0a73e5c3c1b3bd54408e3bab7ea1b91 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 8 Aug 2019 22:54:51 -0300 Subject: [PATCH 1/9] Remove check for Twisted>=14.0.0 16.0.0 is currently the minimum supported version --- scrapy/core/downloader/tls.py | 158 +++++++++++++++++----------------- 1 file changed, 78 insertions(+), 80 deletions(-) diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py index 74afb3f10..2e218dfb4 100644 --- a/scrapy/core/downloader/tls.py +++ b/scrapy/core/downloader/tls.py @@ -1,5 +1,8 @@ import logging + from OpenSSL import SSL +from twisted.internet.ssl import AcceptableCiphers +from twisted.internet._sslverify import ClientTLSOptions, verifyHostname, VerificationError from scrapy import twisted_version from scrapy.utils.ssl import x509name_to_string, get_temp_key_info @@ -22,95 +25,90 @@ openssl_methods = { } -if twisted_version >= (14, 0, 0): - # ClientTLSOptions requires a recent-enough version of Twisted. - # Not having ScrapyClientTLSOptions should not matter for older - # Twisted versions because it is not used in the fallback - # ScrapyClientContextFactory. +# ClientTLSOptions requires a recent-enough version of Twisted (14.0.0+) +# Not having ScrapyClientTLSOptions should not matter for older +# Twisted versions because it is not used in the fallback +# ScrapyClientContextFactory. - # taken from twisted/twisted/internet/_sslverify.py +# taken from twisted/twisted/internet/_sslverify.py - try: - # XXX: this try-except is not needed in Twisted 17.0.0+ because - # it requires pyOpenSSL 0.16+. - from OpenSSL.SSL import SSL_CB_HANDSHAKE_DONE, SSL_CB_HANDSHAKE_START - except ImportError: - SSL_CB_HANDSHAKE_START = 0x10 - SSL_CB_HANDSHAKE_DONE = 0x20 +try: + # XXX: this try-except is not needed in Twisted 17.0.0+ because + # it requires pyOpenSSL 0.16+. + from OpenSSL.SSL import SSL_CB_HANDSHAKE_DONE, SSL_CB_HANDSHAKE_START +except ImportError: + SSL_CB_HANDSHAKE_START = 0x10 + SSL_CB_HANDSHAKE_DONE = 0x20 - from twisted.internet.ssl import AcceptableCiphers - from twisted.internet._sslverify import (ClientTLSOptions, - verifyHostname, - VerificationError) - try: - # XXX: this import would fail on Debian jessie with system installed - # service_identity library, due to lack of cryptography.x509 dependency - # See https://github.com/pyca/service_identity/issues/21 - from service_identity.exceptions import CertificateError - verification_errors = (CertificateError, VerificationError) - except ImportError: - verification_errors = VerificationError +try: + # XXX: this import would fail on Debian jessie with system installed + # service_identity library, due to lack of cryptography.x509 dependency + # See https://github.com/pyca/service_identity/issues/21 + from service_identity.exceptions import CertificateError + verification_errors = (CertificateError, VerificationError) +except ImportError: + verification_errors = VerificationError - if twisted_version < (17, 0, 0): - from twisted.internet._sslverify import _maybeSetHostNameIndication - set_tlsext_host_name = _maybeSetHostNameIndication - else: - def set_tlsext_host_name(connection, hostNameBytes): - connection.set_tlsext_host_name(hostNameBytes) +if twisted_version < (17, 0, 0): + from twisted.internet._sslverify import _maybeSetHostNameIndication + set_tlsext_host_name = _maybeSetHostNameIndication +else: + def set_tlsext_host_name(connection, hostNameBytes): + connection.set_tlsext_host_name(hostNameBytes) - class ScrapyClientTLSOptions(ClientTLSOptions): - """ - SSL Client connection creator ignoring certificate verification errors - (for genuinely invalid certificates or bugs in verification code). +class ScrapyClientTLSOptions(ClientTLSOptions): + """ + SSL Client connection creator ignoring certificate verification errors + (for genuinely invalid certificates or bugs in verification code). - Same as Twisted's private _sslverify.ClientTLSOptions, - except that VerificationError, CertificateError and ValueError - exceptions are caught, so that the connection is not closed, only - logging warnings. Also, HTTPS connection parameters logging is added. - """ + Same as Twisted's private _sslverify.ClientTLSOptions, + except that VerificationError, CertificateError and ValueError + exceptions are caught, so that the connection is not closed, only + logging warnings. Also, HTTPS connection parameters logging is added. + """ - def __init__(self, hostname, ctx, verbose_logging=False): - super(ScrapyClientTLSOptions, self).__init__(hostname, ctx) - self.verbose_logging = verbose_logging + def __init__(self, hostname, ctx, verbose_logging=False): + super(ScrapyClientTLSOptions, self).__init__(hostname, ctx) + self.verbose_logging = verbose_logging - def _identityVerifyingInfoCallback(self, connection, where, ret): - if where & SSL_CB_HANDSHAKE_START: - set_tlsext_host_name(connection, self._hostnameBytes) - elif where & SSL_CB_HANDSHAKE_DONE: - if self.verbose_logging: - if hasattr(connection, 'get_cipher_name'): # requires pyOPenSSL 0.15 - if hasattr(connection, 'get_protocol_version_name'): # requires pyOPenSSL 16.0.0 - logger.debug('SSL connection to %s using protocol %s, cipher %s', - self._hostnameASCII, - connection.get_protocol_version_name(), - connection.get_cipher_name(), - ) - else: - logger.debug('SSL connection to %s using cipher %s', - self._hostnameASCII, - connection.get_cipher_name(), - ) - server_cert = connection.get_peer_certificate() - logger.debug('SSL connection certificate: issuer "%s", subject "%s"', - x509name_to_string(server_cert.get_issuer()), - x509name_to_string(server_cert.get_subject()), - ) - key_info = get_temp_key_info(connection._ssl) - if key_info: - logger.debug('SSL temp key: %s', key_info) + def _identityVerifyingInfoCallback(self, connection, where, ret): + if where & SSL_CB_HANDSHAKE_START: + set_tlsext_host_name(connection, self._hostnameBytes) + elif where & SSL_CB_HANDSHAKE_DONE: + if self.verbose_logging: + if hasattr(connection, 'get_cipher_name'): # requires pyOPenSSL 0.15 + if hasattr(connection, 'get_protocol_version_name'): # requires pyOPenSSL 16.0.0 + logger.debug('SSL connection to %s using protocol %s, cipher %s', + self._hostnameASCII, + connection.get_protocol_version_name(), + connection.get_cipher_name(), + ) + else: + logger.debug('SSL connection to %s using cipher %s', + self._hostnameASCII, + connection.get_cipher_name(), + ) + server_cert = connection.get_peer_certificate() + logger.debug('SSL connection certificate: issuer "%s", subject "%s"', + x509name_to_string(server_cert.get_issuer()), + x509name_to_string(server_cert.get_subject()), + ) + key_info = get_temp_key_info(connection._ssl) + if key_info: + logger.debug('SSL temp key: %s', key_info) - try: - verifyHostname(connection, self._hostnameASCII) - except verification_errors as e: - logger.warning( - 'Remote certificate is not valid for hostname "{}"; {}'.format( - self._hostnameASCII, e)) + try: + verifyHostname(connection, self._hostnameASCII) + except verification_errors as e: + logger.warning( + 'Remote certificate is not valid for hostname "{}"; {}'.format( + self._hostnameASCII, e)) - except ValueError as e: - logger.warning( - 'Ignoring error while verifying certificate ' - 'from host "{}" (exception: {})'.format( - self._hostnameASCII, repr(e))) + except ValueError as e: + logger.warning( + 'Ignoring error while verifying certificate ' + 'from host "{}" (exception: {})'.format( + self._hostnameASCII, repr(e))) - DEFAULT_CIPHERS = AcceptableCiphers.fromOpenSSLCipherString('DEFAULT') +DEFAULT_CIPHERS = AcceptableCiphers.fromOpenSSLCipherString('DEFAULT') From a940a80f5886c631425c4790ac92928926b1cc92 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 8 Aug 2019 23:05:52 -0300 Subject: [PATCH 2/9] Remove check for pyOpenSSL>=0.16 16.2.0 is currently the minimum supported version --- scrapy/core/downloader/tls.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py index 2e218dfb4..d6b7967da 100644 --- a/scrapy/core/downloader/tls.py +++ b/scrapy/core/downloader/tls.py @@ -30,16 +30,6 @@ openssl_methods = { # Twisted versions because it is not used in the fallback # ScrapyClientContextFactory. -# taken from twisted/twisted/internet/_sslverify.py - -try: - # XXX: this try-except is not needed in Twisted 17.0.0+ because - # it requires pyOpenSSL 0.16+. - from OpenSSL.SSL import SSL_CB_HANDSHAKE_DONE, SSL_CB_HANDSHAKE_START -except ImportError: - SSL_CB_HANDSHAKE_START = 0x10 - SSL_CB_HANDSHAKE_DONE = 0x20 - try: # XXX: this import would fail on Debian jessie with system installed # service_identity library, due to lack of cryptography.x509 dependency @@ -73,9 +63,9 @@ class ScrapyClientTLSOptions(ClientTLSOptions): self.verbose_logging = verbose_logging def _identityVerifyingInfoCallback(self, connection, where, ret): - if where & SSL_CB_HANDSHAKE_START: + if where & SSL.SSL_CB_HANDSHAKE_START: set_tlsext_host_name(connection, self._hostnameBytes) - elif where & SSL_CB_HANDSHAKE_DONE: + elif where & SSL.SSL_CB_HANDSHAKE_DONE: if self.verbose_logging: if hasattr(connection, 'get_cipher_name'): # requires pyOPenSSL 0.15 if hasattr(connection, 'get_protocol_version_name'): # requires pyOPenSSL 16.0.0 From 3164543ed1659a11cea1f6a04d8545f9cfebe367 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 8 Aug 2019 23:15:03 -0300 Subject: [PATCH 3/9] Remove fallback ScrapyClientContextFactory class (used in Twisted < 14.0.0) 16.0.0 is currently the minimum supported version --- scrapy/core/downloader/contextfactory.py | 161 ++++++++++------------- scrapy/core/downloader/tls.py | 7 +- 2 files changed, 69 insertions(+), 99 deletions(-) diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index 5ac20c0bb..127a246f5 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -1,112 +1,85 @@ from OpenSSL import SSL -from twisted.internet.ssl import ClientContextFactory +from twisted.internet.ssl import optionsForClientTLS, CertificateOptions, platformTrust +from twisted.web.client import BrowserLikePolicyForHTTPS +from twisted.web.iweb import IPolicyForHTTPS +from zope.interface.declarations import implementer -from scrapy import twisted_version - -if twisted_version >= (14, 0, 0): - - from zope.interface.declarations import implementer - - from twisted.internet.ssl import (optionsForClientTLS, - CertificateOptions, - platformTrust) - from twisted.web.client import BrowserLikePolicyForHTTPS - from twisted.web.iweb import IPolicyForHTTPS - - from scrapy.core.downloader.tls import ScrapyClientTLSOptions, DEFAULT_CIPHERS +from scrapy.core.downloader.tls import ScrapyClientTLSOptions, DEFAULT_CIPHERS - @implementer(IPolicyForHTTPS) - class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): - """ - Non-peer-certificate verifying HTTPS context factory +@implementer(IPolicyForHTTPS) +class ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): + """ + Non-peer-certificate verifying HTTPS context factory - Default OpenSSL method is TLS_METHOD (also called SSLv23_METHOD) - which allows TLS protocol negotiation + Default OpenSSL method is TLS_METHOD (also called SSLv23_METHOD) + which allows TLS protocol negotiation - 'A TLS/SSL connection established with [this method] may - understand the SSLv3, TLSv1, TLSv1.1 and TLSv1.2 protocols.' - """ + 'A TLS/SSL connection established with [this method] may + understand the SSLv3, TLSv1, TLSv1.1 and TLSv1.2 protocols.' + """ - def __init__(self, method=SSL.SSLv23_METHOD, tls_verbose_logging=False, *args, **kwargs): - super(ScrapyClientContextFactory, self).__init__(*args, **kwargs) - self._ssl_method = method - self.tls_verbose_logging = tls_verbose_logging + def __init__(self, method=SSL.SSLv23_METHOD, tls_verbose_logging=False, *args, **kwargs): + super(ScrapyClientContextFactory, self).__init__(*args, **kwargs) + self._ssl_method = method + self.tls_verbose_logging = tls_verbose_logging - @classmethod - def from_settings(cls, settings, method=SSL.SSLv23_METHOD, *args, **kwargs): - tls_verbose_logging = settings.getbool('DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING') - return cls(method=method, tls_verbose_logging=tls_verbose_logging, *args, **kwargs) + @classmethod + def from_settings(cls, settings, method=SSL.SSLv23_METHOD, *args, **kwargs): + tls_verbose_logging = settings.getbool('DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING') + return cls(method=method, tls_verbose_logging=tls_verbose_logging, *args, **kwargs) - def getCertificateOptions(self): - # setting verify=True will require you to provide CAs - # to verify against; in other words: it's not that simple + def getCertificateOptions(self): + # setting verify=True will require you to provide CAs + # to verify against; in other words: it's not that simple - # backward-compatible SSL/TLS method: - # - # * this will respect `method` attribute in often recommended - # `ScrapyClientContextFactory` subclass - # (https://github.com/scrapy/scrapy/issues/1429#issuecomment-131782133) - # - # * getattr() for `_ssl_method` attribute for context factories - # not calling super(..., self).__init__ - return CertificateOptions(verify=False, - method=getattr(self, 'method', - getattr(self, '_ssl_method', None)), - fixBrokenPeers=True, - acceptableCiphers=DEFAULT_CIPHERS) + # backward-compatible SSL/TLS method: + # + # * this will respect `method` attribute in often recommended + # `ScrapyClientContextFactory` subclass + # (https://github.com/scrapy/scrapy/issues/1429#issuecomment-131782133) + # + # * getattr() for `_ssl_method` attribute for context factories + # not calling super(..., self).__init__ + return CertificateOptions(verify=False, + method=getattr(self, 'method', + getattr(self, '_ssl_method', None)), + fixBrokenPeers=True, + acceptableCiphers=DEFAULT_CIPHERS) - # kept for old-style HTTP/1.0 downloader context twisted calls, - # e.g. connectSSL() - def getContext(self, hostname=None, port=None): - return self.getCertificateOptions().getContext() + # kept for old-style HTTP/1.0 downloader context twisted calls, + # e.g. connectSSL() + def getContext(self, hostname=None, port=None): + return self.getCertificateOptions().getContext() - def creatorForNetloc(self, hostname, port): - return ScrapyClientTLSOptions(hostname.decode("ascii"), self.getContext(), - verbose_logging=self.tls_verbose_logging) + def creatorForNetloc(self, hostname, port): + return ScrapyClientTLSOptions(hostname.decode("ascii"), self.getContext(), + verbose_logging=self.tls_verbose_logging) - @implementer(IPolicyForHTTPS) - class BrowserLikeContextFactory(ScrapyClientContextFactory): - """ - Twisted-recommended context factory for web clients. +@implementer(IPolicyForHTTPS) +class BrowserLikeContextFactory(ScrapyClientContextFactory): + """ + Twisted-recommended context factory for web clients. - Quoting https://twistedmatrix.com/documents/current/api/twisted.web.client.Agent.html: - "The default is to use a BrowserLikePolicyForHTTPS, - so unless you have special requirements you can leave this as-is." + Quoting https://twistedmatrix.com/documents/current/api/twisted.web.client.Agent.html: + "The default is to use a BrowserLikePolicyForHTTPS, + so unless you have special requirements you can leave this as-is." - creatorForNetloc() is the same as BrowserLikePolicyForHTTPS - except this context factory allows setting the TLS/SSL method to use. + creatorForNetloc() is the same as BrowserLikePolicyForHTTPS + except this context factory allows setting the TLS/SSL method to use. - Default OpenSSL method is TLS_METHOD (also called SSLv23_METHOD) - which allows TLS protocol negotiation. - """ - def creatorForNetloc(self, hostname, port): + Default OpenSSL method is TLS_METHOD (also called SSLv23_METHOD) + which allows TLS protocol negotiation. + """ + def creatorForNetloc(self, hostname, port): - # trustRoot set to platformTrust() will use the platform's root CAs. - # - # This means that a website like https://www.cacert.org will be rejected - # by default, since CAcert.org CA certificate is seldom shipped. - return optionsForClientTLS(hostname.decode("ascii"), - trustRoot=platformTrust(), - extraCertificateOptions={ - 'method': self._ssl_method, - }) - -else: - - class ScrapyClientContextFactory(ClientContextFactory): - "A SSL context factory which is more permissive against SSL bugs." - # see https://github.com/scrapy/scrapy/issues/82 - # and https://github.com/scrapy/scrapy/issues/26 - # and https://github.com/scrapy/scrapy/issues/981 - - def __init__(self, method=SSL.SSLv23_METHOD): - self.method = method - - def getContext(self, hostname=None, port=None): - ctx = ClientContextFactory.getContext(self) - # Enable all workarounds to SSL bugs as documented by - # https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_options.html - ctx.set_options(SSL.OP_ALL) - return ctx + # trustRoot set to platformTrust() will use the platform's root CAs. + # + # This means that a website like https://www.cacert.org will be rejected + # by default, since CAcert.org CA certificate is seldom shipped. + return optionsForClientTLS(hostname.decode("ascii"), + trustRoot=platformTrust(), + extraCertificateOptions={ + 'method': self._ssl_method, + }) diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py index d6b7967da..995f8cbba 100644 --- a/scrapy/core/downloader/tls.py +++ b/scrapy/core/downloader/tls.py @@ -10,12 +10,14 @@ from scrapy.utils.ssl import x509name_to_string, get_temp_key_info logger = logging.getLogger(__name__) + METHOD_SSLv3 = 'SSLv3' METHOD_TLS = 'TLS' METHOD_TLSv10 = 'TLSv1.0' METHOD_TLSv11 = 'TLSv1.1' METHOD_TLSv12 = 'TLSv1.2' + openssl_methods = { METHOD_TLS: SSL.SSLv23_METHOD, # protocol negotiation (recommended) METHOD_SSLv3: SSL.SSLv3_METHOD, # SSL 3 (NOT recommended) @@ -25,11 +27,6 @@ openssl_methods = { } -# ClientTLSOptions requires a recent-enough version of Twisted (14.0.0+) -# Not having ScrapyClientTLSOptions should not matter for older -# Twisted versions because it is not used in the fallback -# ScrapyClientContextFactory. - try: # XXX: this import would fail on Debian jessie with system installed # service_identity library, due to lack of cryptography.x509 dependency From b404941e0de86ddf2e108c42a4847f0341d53d14 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 8 Aug 2019 23:18:47 -0300 Subject: [PATCH 4/9] Remove import check for service_identity service_identity.exceptions.CertificateError is available in the current minimum version (16.0.0) --- scrapy/core/downloader/tls.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py index 995f8cbba..1c3d94b29 100644 --- a/scrapy/core/downloader/tls.py +++ b/scrapy/core/downloader/tls.py @@ -1,8 +1,9 @@ import logging from OpenSSL import SSL -from twisted.internet.ssl import AcceptableCiphers +from service_identity.exceptions import CertificateError from twisted.internet._sslverify import ClientTLSOptions, verifyHostname, VerificationError +from twisted.internet.ssl import AcceptableCiphers from scrapy import twisted_version from scrapy.utils.ssl import x509name_to_string, get_temp_key_info @@ -27,15 +28,6 @@ openssl_methods = { } -try: - # XXX: this import would fail on Debian jessie with system installed - # service_identity library, due to lack of cryptography.x509 dependency - # See https://github.com/pyca/service_identity/issues/21 - from service_identity.exceptions import CertificateError - verification_errors = (CertificateError, VerificationError) -except ImportError: - verification_errors = VerificationError - if twisted_version < (17, 0, 0): from twisted.internet._sslverify import _maybeSetHostNameIndication set_tlsext_host_name = _maybeSetHostNameIndication @@ -87,7 +79,7 @@ class ScrapyClientTLSOptions(ClientTLSOptions): try: verifyHostname(connection, self._hostnameASCII) - except verification_errors as e: + except (CertificateError, VerificationError) as e: logger.warning( 'Remote certificate is not valid for hostname "{}"; {}'.format( self._hostnameASCII, e)) From d92f1b18580940c27f303f1dfcf47bb66e508ce4 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 8 Aug 2019 23:53:35 -0300 Subject: [PATCH 5/9] Simplify import + assignment --- scrapy/core/downloader/tls.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scrapy/core/downloader/tls.py b/scrapy/core/downloader/tls.py index 1c3d94b29..4ed482058 100644 --- a/scrapy/core/downloader/tls.py +++ b/scrapy/core/downloader/tls.py @@ -29,8 +29,7 @@ openssl_methods = { if twisted_version < (17, 0, 0): - from twisted.internet._sslverify import _maybeSetHostNameIndication - set_tlsext_host_name = _maybeSetHostNameIndication + from twisted.internet._sslverify import _maybeSetHostNameIndication as set_tlsext_host_name else: def set_tlsext_host_name(connection, hostNameBytes): connection.set_tlsext_host_name(hostNameBytes) From e17c9a48fdd41e838c235be75038fab1ab9bb8e2 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Thu, 8 Aug 2019 23:59:17 -0300 Subject: [PATCH 6/9] Remove check for Twisted>=15.0.0 16.0.0 is currently the minimum supported version --- scrapy/core/downloader/handlers/http11.py | 28 +++++++---------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 2ccab2614..8b4ae6b24 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -27,7 +27,7 @@ from scrapy.core.downloader.webclient import _parse from scrapy.core.downloader.tls import openssl_methods from scrapy.utils.misc import load_object, create_instance from scrapy.utils.python import to_bytes, to_unicode -from scrapy import twisted_version + logger = logging.getLogger(__name__) @@ -211,21 +211,14 @@ class TunnelingAgent(Agent): self._proxyConf = proxyConf self._contextFactory = contextFactory - if twisted_version >= (15, 0, 0): - def _getEndpoint(self, uri): - return TunnelingTCP4ClientEndpoint( - self._reactor, uri.host, uri.port, self._proxyConf, - self._contextFactory, self._endpointFactory._connectTimeout, - self._endpointFactory._bindAddress) - else: - def _getEndpoint(self, scheme, host, port): - return TunnelingTCP4ClientEndpoint( - self._reactor, host, port, self._proxyConf, - self._contextFactory, self._connectTimeout, - self._bindAddress) + def _getEndpoint(self, uri): + return TunnelingTCP4ClientEndpoint( + self._reactor, uri.host, uri.port, self._proxyConf, + self._contextFactory, self._endpointFactory._connectTimeout, + self._endpointFactory._bindAddress) def _requestWithEndpoint(self, key, endpoint, method, parsedURI, - headers, bodyProducer, requestPath): + headers, bodyProducer, requestPath): # proxy host and port are required for HTTP pool `key` # otherwise, same remote host connection request could reuse # a cached tunneled connection to a different proxy @@ -250,12 +243,7 @@ class ScrapyProxyAgent(Agent): """ # Cache *all* connections under the same key, since we are only # connecting to a single destination, the proxy: - if twisted_version >= (15, 0, 0): - proxyEndpoint = self._getEndpoint(self._proxyURI) - else: - proxyEndpoint = self._getEndpoint(self._proxyURI.scheme, - self._proxyURI.host, - self._proxyURI.port) + proxyEndpoint = self._getEndpoint(self._proxyURI) key = ("http-proxy", self._proxyURI.host, self._proxyURI.port) return self._requestWithEndpoint(key, proxyEndpoint, method, URI.fromBytes(uri), headers, From d3737d869b1b49497f53376283b02ac604543e2c Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 9 Aug 2019 00:21:43 -0300 Subject: [PATCH 7/9] Remove check for Twisted>=14.0 --- scrapy/core/downloader/handlers/http11.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 8b4ae6b24..f0ed1a4af 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -141,14 +141,8 @@ class TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): self._protocol.dataReceived = self._protocolDataReceived respm = TunnelingTCP4ClientEndpoint._responseMatcher.match(self._connectBuffer) if respm and int(respm.group('status')) == 200: - try: - # this sets proper Server Name Indication extension - # but is only available for Twisted>=14.0 - sslOptions = self._contextFactory.creatorForNetloc( - self._tunneledHost, self._tunneledPort) - except AttributeError: - # fall back to non-SNI SSL context factory - sslOptions = self._contextFactory + # set proper Server Name Indication extension + sslOptions = self._contextFactory.creatorForNetloc(self._tunneledHost, self._tunneledPort) self._protocol.transport.startTLS(sslOptions, self._protocolFactory) self._tunnelReadyDeferred.callback(self._protocol) From d5dcc5eaef80ef383dff90f19349c0e06f1836a6 Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 9 Aug 2019 00:30:58 -0300 Subject: [PATCH 8/9] Import twisted.web.client.URI directly --- scrapy/core/downloader/handlers/http11.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index f0ed1a4af..9da20e032 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -13,12 +13,8 @@ from twisted.web.http_headers import Headers as TxHeaders from twisted.web.iweb import IBodyProducer, UNKNOWN_LENGTH from twisted.internet.error import TimeoutError from twisted.web.http import _DataLoss, PotentialDataLoss -from twisted.web.client import Agent, ProxyAgent, ResponseDone, \ - HTTPConnectionPool, ResponseFailed -try: - from twisted.web.client import URI -except ImportError: - from twisted.web.client import _URI as URI +from twisted.web.client import (Agent, ProxyAgent, ResponseDone, + HTTPConnectionPool, ResponseFailed, URI) from twisted.internet.endpoints import TCP4ClientEndpoint from scrapy.http import Headers From 26fb28b20f661e6d7cfd02c904f4b1dc2a79e1dc Mon Sep 17 00:00:00 2001 From: Eugenio Lacuesta Date: Fri, 9 Aug 2019 00:49:46 -0300 Subject: [PATCH 9/9] PEP8-ify HTTP/1.1 downloader handler Signed-off-by: Eugenio Lacuesta --- scrapy/core/downloader/handlers/http11.py | 150 +++++++++++++--------- 1 file changed, 91 insertions(+), 59 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 9da20e032..e72052afc 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -13,8 +13,7 @@ from twisted.web.http_headers import Headers as TxHeaders from twisted.web.iweb import IBodyProducer, UNKNOWN_LENGTH from twisted.internet.error import TimeoutError from twisted.web.http import _DataLoss, PotentialDataLoss -from twisted.web.client import (Agent, ProxyAgent, ResponseDone, - HTTPConnectionPool, ResponseFailed, URI) +from twisted.web.client import Agent, ResponseDone, HTTPConnectionPool, ResponseFailed, URI from twisted.internet.endpoints import TCP4ClientEndpoint from scrapy.http import Headers @@ -40,11 +39,19 @@ class HTTP11DownloadHandler(object): self._contextFactoryClass = load_object(settings['DOWNLOADER_CLIENTCONTEXTFACTORY']) # try method-aware context factory try: - self._contextFactory = create_instance(self._contextFactoryClass, settings=settings, crawler=None, - method=self._sslMethod) + self._contextFactory = create_instance( + self._contextFactoryClass, + settings=settings, + crawler=None, + method=self._sslMethod, + ) except TypeError: # use context factory defaults - self._contextFactory = create_instance(self._contextFactoryClass, settings=settings, crawler=None) + self._contextFactory = create_instance( + self._contextFactoryClass, + settings=settings, + crawler=None, + ) msg = """ '%s' does not accept `method` argument (type OpenSSL.SSL method,\ e.g. OpenSSL.SSL.SSLv23_METHOD) and/or `tls_verbose_logging` argument.\ @@ -58,10 +65,13 @@ class HTTP11DownloadHandler(object): def download_request(self, request, spider): """Return a deferred for the HTTP download""" - agent = ScrapyAgent(contextFactory=self._contextFactory, pool=self._pool, + agent = ScrapyAgent( + contextFactory=self._contextFactory, + pool=self._pool, maxsize=getattr(spider, 'download_maxsize', self._default_maxsize), warnsize=getattr(spider, 'download_warnsize', self._default_warnsize), - fail_on_dataloss=self._fail_on_dataloss) + fail_on_dataloss=self._fail_on_dataloss, + ) return agent.download_request(request) def close(self): @@ -100,11 +110,9 @@ class TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): _responseMatcher = re.compile(br'HTTP/1\.. (?P\d{3})(?P.{,32})') - def __init__(self, reactor, host, port, proxyConf, contextFactory, - timeout=30, bindAddress=None): + def __init__(self, reactor, host, port, proxyConf, contextFactory, timeout=30, bindAddress=None): proxyHost, proxyPort, self._proxyAuthHeader = proxyConf - super(TunnelingTCP4ClientEndpoint, self).__init__(reactor, proxyHost, - proxyPort, timeout, bindAddress) + super(TunnelingTCP4ClientEndpoint, self).__init__(reactor, proxyHost, proxyPort, timeout, bindAddress) self._tunnelReadyDeferred = defer.Deferred() self._tunneledHost = host self._tunneledPort = port @@ -113,8 +121,7 @@ class TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): def requestTunnel(self, protocol): """Asks the proxy to open a tunnel.""" - tunnelReq = tunnel_request_data(self._tunneledHost, self._tunneledPort, - self._proxyAuthHeader) + tunnelReq = tunnel_request_data(self._tunneledHost, self._tunneledPort, self._proxyAuthHeader) protocol.transport.write(tunnelReq) self._protocolDataReceived = protocol.dataReceived protocol.dataReceived = self.processProxyResponse @@ -139,8 +146,7 @@ class TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): if respm and int(respm.group('status')) == 200: # set proper Server Name Indication extension sslOptions = self._contextFactory.creatorForNetloc(self._tunneledHost, self._tunneledPort) - self._protocol.transport.startTLS(sslOptions, - self._protocolFactory) + self._protocol.transport.startTLS(sslOptions, self._protocolFactory) self._tunnelReadyDeferred.callback(self._protocol) else: if respm: @@ -158,8 +164,7 @@ class TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): def connect(self, protocolFactory): self._protocolFactory = protocolFactory - connectDeferred = super(TunnelingTCP4ClientEndpoint, - self).connect(protocolFactory) + connectDeferred = super(TunnelingTCP4ClientEndpoint, self).connect(protocolFactory) connectDeferred.addCallback(self.requestTunnel) connectDeferred.addErrback(self.connectFailed) return self._tunnelReadyDeferred @@ -196,35 +201,46 @@ class TunnelingAgent(Agent): def __init__(self, reactor, proxyConf, contextFactory=None, connectTimeout=None, bindAddress=None, pool=None): - super(TunnelingAgent, self).__init__(reactor, contextFactory, - connectTimeout, bindAddress, pool) + super(TunnelingAgent, self).__init__(reactor, contextFactory, connectTimeout, bindAddress, pool) self._proxyConf = proxyConf self._contextFactory = contextFactory def _getEndpoint(self, uri): return TunnelingTCP4ClientEndpoint( - self._reactor, uri.host, uri.port, self._proxyConf, - self._contextFactory, self._endpointFactory._connectTimeout, - self._endpointFactory._bindAddress) + reactor=self._reactor, + host=uri.host, + port=uri.port, + proxyConf=self._proxyConf, + contextFactory=self._contextFactory, + timeout=self._endpointFactory._connectTimeout, + bindAddress=self._endpointFactory._bindAddress, + ) - def _requestWithEndpoint(self, key, endpoint, method, parsedURI, - headers, bodyProducer, requestPath): + def _requestWithEndpoint(self, key, endpoint, method, parsedURI, headers, bodyProducer, requestPath): # proxy host and port are required for HTTP pool `key` # otherwise, same remote host connection request could reuse # a cached tunneled connection to a different proxy key = key + self._proxyConf - return super(TunnelingAgent, self)._requestWithEndpoint(key, endpoint, method, parsedURI, - headers, bodyProducer, requestPath) + return super(TunnelingAgent, self)._requestWithEndpoint( + key=key, + endpoint=endpoint, + method=method, + parsedURI=parsedURI, + headers=headers, + bodyProducer=bodyProducer, + requestPath=requestPath, + ) class ScrapyProxyAgent(Agent): - def __init__(self, reactor, proxyURI, - connectTimeout=None, bindAddress=None, pool=None): - super(ScrapyProxyAgent, self).__init__(reactor, - connectTimeout=connectTimeout, - bindAddress=bindAddress, - pool=pool) + def __init__(self, reactor, proxyURI, connectTimeout=None, bindAddress=None, pool=None): + super(ScrapyProxyAgent, self).__init__( + reactor=reactor, + connectTimeout=connectTimeout, + bindAddress=bindAddress, + pool=pool, + ) self._proxyURI = URI.fromBytes(proxyURI) def request(self, method, uri, headers=None, bodyProducer=None): @@ -233,11 +249,15 @@ class ScrapyProxyAgent(Agent): """ # Cache *all* connections under the same key, since we are only # connecting to a single destination, the proxy: - proxyEndpoint = self._getEndpoint(self._proxyURI) - key = ("http-proxy", self._proxyURI.host, self._proxyURI.port) - return self._requestWithEndpoint(key, proxyEndpoint, method, - URI.fromBytes(uri), headers, - bodyProducer, uri) + return self._requestWithEndpoint( + key=("http-proxy", self._proxyURI.host, self._proxyURI.port), + endpoint=self._getEndpoint(self._proxyURI), + method=method, + parsedURI=URI.fromBytes(uri), + headers=headers, + bodyProducer=bodyProducer, + requestPath=uri, + ) class ScrapyAgent(object): @@ -265,18 +285,33 @@ class ScrapyAgent(object): scheme = _parse(request.url)[0] proxyHost = to_unicode(proxyHost) omitConnectTunnel = b'noconnect' in proxyParams - if scheme == b'https' and not omitConnectTunnel: - proxyConf = (proxyHost, proxyPort, - request.headers.get(b'Proxy-Authorization', None)) - return self._TunnelingAgent(reactor, proxyConf, - contextFactory=self._contextFactory, connectTimeout=timeout, - bindAddress=bindaddress, pool=self._pool) + if scheme == b'https' and not omitConnectTunnel: + proxyAuth = request.headers.get(b'Proxy-Authorization', None) + proxyConf = (proxyHost, proxyPort, proxyAuth) + return self._TunnelingAgent( + reactor=reactor, + proxyConf=proxyConf, + contextFactory=self._contextFactory, + connectTimeout=timeout, + bindAddress=bindaddress, + pool=self._pool, + ) else: - return self._ProxyAgent(reactor, proxyURI=to_bytes(proxy, encoding='ascii'), - connectTimeout=timeout, bindAddress=bindaddress, pool=self._pool) + return self._ProxyAgent( + reactor=reactor, + proxyURI=to_bytes(proxy, encoding='ascii'), + connectTimeout=timeout, + bindAddress=bindaddress, + pool=self._pool, + ) - return self._Agent(reactor, contextFactory=self._contextFactory, - connectTimeout=timeout, bindAddress=bindaddress, pool=self._pool) + return self._Agent( + reactor=reactor, + contextFactory=self._contextFactory, + connectTimeout=timeout, + bindAddress=bindaddress, + pool=self._pool, + ) def download_request(self, request): timeout = request.meta.get('download_timeout') or self._connectTimeout @@ -307,8 +342,7 @@ class ScrapyAgent(object): else: bodyproducer = None start_time = time() - d = agent.request( - method, to_bytes(url, encoding='ascii'), headers, bodyproducer) + d = agent.request(method, to_bytes(url, encoding='ascii'), headers, bodyproducer) # set download latency d.addCallback(self._cb_latency, request, start_time) # response body is ready to be consumed @@ -363,8 +397,9 @@ class ScrapyAgent(object): txresponse._transport._producer.abortConnection() d = defer.Deferred(_cancel) - txresponse.deliverBody(_ResponseReader( - d, txresponse, request, maxsize, warnsize, fail_on_dataloss)) + txresponse.deliverBody( + _ResponseReader(d, txresponse, request, maxsize, warnsize, fail_on_dataloss) + ) # save response for timeouts self._txresponse = txresponse @@ -399,22 +434,20 @@ class _RequestBodyProducer(object): class _ResponseReader(protocol.Protocol): - def __init__(self, finished, txresponse, request, maxsize, warnsize, - fail_on_dataloss): + def __init__(self, finished, txresponse, request, maxsize, warnsize, fail_on_dataloss): self._finished = finished self._txresponse = txresponse self._request = request self._bodybuf = BytesIO() - self._maxsize = maxsize - self._warnsize = warnsize + self._maxsize = maxsize + self._warnsize = warnsize self._fail_on_dataloss = fail_on_dataloss self._fail_on_dataloss_warned = False self._reached_warnsize = False self._bytes_received = 0 def dataReceived(self, bodyBytes): - # This maybe called several times after cancel was called with buffered - # data. + # This maybe called several times after cancel was called with buffered data. if self._finished.called: return @@ -427,8 +460,7 @@ class _ResponseReader(protocol.Protocol): {'bytes': self._bytes_received, 'maxsize': self._maxsize, 'request': self._request}) - # Clear buffer earlier to avoid keeping data in memory for a long - # time. + # Clear buffer earlier to avoid keeping data in memory for a long time. self._bodybuf.truncate(0) self._finished.cancel()