From 32bb5b682a7cf4f2baca78914f18297c120eef2d Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 14 Jan 2016 16:11:16 +0300 Subject: [PATCH 01/21] fix import of test_downloader_handlers.py: use @implementer, move failing on py3 imports into corresponding tests --- scrapy/core/downloader/handlers/http11.py | 4 ++-- tests/test_downloader_handlers.py | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 31412a0f4..7c937a036 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -6,7 +6,7 @@ from io import BytesIO from time import time from six.moves.urllib.parse import urldefrag -from zope.interface import implements +from zope.interface import implementer from twisted.internet import defer, reactor, protocol from twisted.web.http_headers import Headers as TxHeaders from twisted.web.iweb import IBodyProducer, UNKNOWN_LENGTH @@ -265,8 +265,8 @@ class ScrapyAgent(object): return respcls(url=url, status=status, headers=headers, body=body, flags=flags) +@implementer(IBodyProducer) class _RequestBodyProducer(object): - implements(IBodyProducer) def __init__(self, body): self.body = body diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index d2a349b40..5f1703c5c 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -10,9 +10,7 @@ from twisted.web import server, static, util, resource from twisted.web.test.test_webclient import ForeverTakingResource, \ NoLengthResource, HostHeaderResource, \ PayloadResource, BrokenDownloadResource -from twisted.protocols.ftp import FTPRealm, FTPFactory from twisted.cred import portal, checkers, credentials -from twisted.protocols.ftp import FTPClient, ConnectionLost from w3lib.url import path_to_file_uri from scrapy import twisted_version @@ -22,7 +20,6 @@ from scrapy.core.downloader.handlers.http import HTTPDownloadHandler, HttpDownlo from scrapy.core.downloader.handlers.http10 import HTTP10DownloadHandler from scrapy.core.downloader.handlers.http11 import HTTP11DownloadHandler from scrapy.core.downloader.handlers.s3 import S3DownloadHandler -from scrapy.core.downloader.handlers.ftp import FTPDownloadHandler from scrapy.spiders import Spider from scrapy.http import Request @@ -520,6 +517,9 @@ class FTPTestCase(unittest.TestCase): skip = "Twisted pre 10.2.0 doesn't allow to set home path other than /home" def setUp(self): + from twisted.protocols.ftp import FTPRealm, FTPFactory + from scrapy.core.downloader.handlers.ftp import FTPDownloadHandler + # setup dirs and test file self.directory = self.mktemp() os.mkdir(self.directory) @@ -601,6 +601,8 @@ class FTPTestCase(unittest.TestCase): return self._add_test_callbacks(d, _test) def test_invalid_credentials(self): + from twisted.protocols.ftp import ConnectionLost + request = Request(url="ftp://127.0.0.1:%s/file.txt" % self.portNum, meta={"ftp_user": self.username, "ftp_password": 'invalid'}) d = self.download_handler.download_request(request, None) From 3509378b8be0df7cb38d3823068288b5daa37612 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 14 Jan 2016 16:29:19 +0300 Subject: [PATCH 02/21] py3: pass first http downloader test, simple crawler works now, yay! --- scrapy/core/downloader/handlers/http11.py | 9 ++++++--- scrapy/http/response/__init__.py | 3 ++- tests/test_downloader_handlers.py | 20 ++++++++++---------- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 7c937a036..34070ebc6 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -19,6 +19,7 @@ from scrapy.http import Headers from scrapy.responsetypes import responsetypes from scrapy.core.downloader.webclient import _parse from scrapy.utils.misc import load_object +from scrapy.utils.python import to_bytes, to_unicode from scrapy import twisted_version logger = logging.getLogger(__name__) @@ -200,8 +201,8 @@ class ScrapyAgent(object): agent = self._get_agent(request, timeout) # request details - url = urldefrag(request.url)[0] - method = request.method + url = to_bytes(urldefrag(request.url)[0]) + method = to_bytes(request.method) headers = TxHeaders(request.headers) if isinstance(agent, self._TunnelingAgent): headers.removeHeader('Proxy-Authorization') @@ -261,8 +262,10 @@ class ScrapyAgent(object): txresponse, body, flags = result status = int(txresponse.code) headers = Headers(txresponse.headers.getAllRawHeaders()) + url = to_unicode(url) respcls = responsetypes.from_args(headers=headers, url=url) - return respcls(url=url, status=status, headers=headers, body=body, flags=flags) + return respcls( + url=url, status=status, headers=headers, body=body, flags=flags) @implementer(IBodyProducer) diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index 983154001..59ef15682 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -4,6 +4,7 @@ responses in Scrapy. See documentation in docs/topics/request-response.rst """ +import six from six.moves.urllib.parse import urljoin from scrapy.http.headers import Headers @@ -34,7 +35,7 @@ class Response(object_ref): return self._url def _set_url(self, url): - if isinstance(url, str): + if isinstance(url, six.string_types): self._url = url else: raise TypeError('%s url must be str, got %s:' % (type(self).__name__, diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 5f1703c5c..cdb1ad02d 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -88,7 +88,7 @@ class FileTestCase(unittest.TestCase): def _test(response): self.assertEquals(response.url, request.url) self.assertEquals(response.status, 200) - self.assertEquals(response.body, '0123456789') + self.assertEquals(response.body, b'0123456789') request = Request(path_to_file_uri(self.tmpname + '^')) assert request.url.upper().endswith('%5E') @@ -107,15 +107,15 @@ class HttpTestCase(unittest.TestCase): def setUp(self): name = self.mktemp() os.mkdir(name) - FilePath(name).child("file").setContent("0123456789") + FilePath(name).child("file").setContent(b"0123456789") r = static.File(name) - r.putChild("redirect", util.Redirect("/file")) - r.putChild("wait", ForeverTakingResource()) - r.putChild("hang-after-headers", ForeverTakingResource(write=True)) - r.putChild("nolength", NoLengthResource()) - r.putChild("host", HostHeaderResource()) - r.putChild("payload", PayloadResource()) - r.putChild("broken", BrokenDownloadResource()) + r.putChild(b"redirect", util.Redirect(b"/file")) + r.putChild(b"wait", ForeverTakingResource()) + r.putChild(b"hang-after-headers", ForeverTakingResource(write=True)) + r.putChild(b"nolength", NoLengthResource()) + r.putChild(b"host", HostHeaderResource()) + r.putChild(b"payload", PayloadResource()) + r.putChild(b"broken", BrokenDownloadResource()) self.site = server.Site(r, timeout=None) self.wrapper = WrappingFactory(self.site) self.port = reactor.listenTCP(0, self.wrapper, interface='127.0.0.1') @@ -136,7 +136,7 @@ class HttpTestCase(unittest.TestCase): request = Request(self.getURL('file')) d = self.download_request(request, Spider('foo')) d.addCallback(lambda r: r.body) - d.addCallback(self.assertEquals, "0123456789") + d.addCallback(self.assertEquals, b"0123456789") return d def test_download_head(self): From 6b79fffa9a4c53cb6a6af2e9d9251b95c28496b4 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 14 Jan 2016 16:37:18 +0300 Subject: [PATCH 03/21] py3: pass all of HttpTestCase --- scrapy/core/downloader/handlers/http11.py | 4 ++-- tests/test_downloader_handlers.py | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 34070ebc6..dbb002710 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -182,7 +182,7 @@ class ScrapyAgent(object): _, _, proxyHost, proxyPort, proxyParams = _parse(proxy) scheme = _parse(request.url)[0] omitConnectTunnel = proxyParams.find('noconnect') >= 0 - if scheme == 'https' and not omitConnectTunnel: + if scheme == b'https' and not omitConnectTunnel: proxyConf = (proxyHost, proxyPort, request.headers.get('Proxy-Authorization', None)) return self._TunnelingAgent(reactor, proxyConf, @@ -233,7 +233,7 @@ class ScrapyAgent(object): def _cb_bodyready(self, txresponse, request): # deliverBody hangs for responses without body if txresponse.length == 0: - return txresponse, '', None + return txresponse, b'', None maxsize = request.meta.get('download_maxsize', self._maxsize) warnsize = request.meta.get('download_warnsize', self._warnsize) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index cdb1ad02d..c017a9eb2 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -1,5 +1,4 @@ import os -import twisted import six from twisted.trial import unittest @@ -25,6 +24,7 @@ from scrapy.spiders import Spider from scrapy.http import Request from scrapy.settings import Settings from scrapy.utils.test import get_crawler +from scrapy.utils.python import to_bytes from scrapy.exceptions import NotConfigured from tests.mockserver import MockServer @@ -143,7 +143,7 @@ class HttpTestCase(unittest.TestCase): request = Request(self.getURL('file'), method='HEAD') d = self.download_request(request, Spider('foo')) d.addCallback(lambda r: r.body) - d.addCallback(self.assertEquals, '') + d.addCallback(self.assertEquals, b'') return d def test_redirect_status(self): @@ -175,7 +175,7 @@ class HttpTestCase(unittest.TestCase): def test_host_header_not_in_request_headers(self): def _test(response): - self.assertEquals(response.body, '127.0.0.1:%d' % self.portno) + self.assertEquals(response.body, to_bytes('127.0.0.1:%d' % self.portno)) self.assertEquals(request.headers, {}) request = Request(self.getURL('host')) @@ -183,19 +183,19 @@ class HttpTestCase(unittest.TestCase): def test_host_header_seted_in_request_headers(self): def _test(response): - self.assertEquals(response.body, 'example.com') - self.assertEquals(request.headers.get('Host'), 'example.com') + self.assertEquals(response.body, b'example.com') + self.assertEquals(request.headers.get('Host'), b'example.com') request = Request(self.getURL('host'), headers={'Host': 'example.com'}) return self.download_request(request, Spider('foo')).addCallback(_test) d = self.download_request(request, Spider('foo')) d.addCallback(lambda r: r.body) - d.addCallback(self.assertEquals, 'example.com') + d.addCallback(self.assertEquals, b'example.com') return d def test_payload(self): - body = '1'*100 # PayloadResource requires body length to be 100 + body = b'1'*100 # PayloadResource requires body length to be 100 request = Request(self.getURL('payload'), method='POST', body=body) d = self.download_request(request, Spider('foo')) d.addCallback(lambda r: r.body) From c6f14a39de14b15b78d3fd4a098ec246536e12c4 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 14 Jan 2016 16:50:16 +0300 Subject: [PATCH 04/21] py3: fix http10 downloader - unicode host expected here --- scrapy/core/downloader/handlers/http10.py | 5 +++-- tests/test_downloader_handlers.py | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/scrapy/core/downloader/handlers/http10.py b/scrapy/core/downloader/handlers/http10.py index 11b2acdae..0322bbe49 100644 --- a/scrapy/core/downloader/handlers/http10.py +++ b/scrapy/core/downloader/handlers/http10.py @@ -2,6 +2,7 @@ """ from twisted.internet import reactor from scrapy.utils.misc import load_object +from scrapy.utils.python import to_unicode class HTTP10DownloadHandler(object): @@ -17,8 +18,8 @@ class HTTP10DownloadHandler(object): return factory.deferred def _connect(self, factory): - host, port = factory.host, factory.port - if factory.scheme == 'https': + host, port = to_unicode(factory.host), factory.port + if factory.scheme == b'https': return reactor.connectSSL(host, port, factory, self.ClientContextFactory()) else: diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index c017a9eb2..780f08806 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -223,7 +223,7 @@ class Http11TestCase(HttpTestCase): request = Request(self.getURL('file')) d = self.download_request(request, Spider('foo')) d.addCallback(lambda r: r.body) - d.addCallback(self.assertEquals, "0123456789") + d.addCallback(self.assertEquals, b"0123456789") return d @defer.inlineCallbacks @@ -234,7 +234,7 @@ class Http11TestCase(HttpTestCase): # response body. (regardless of headers) d = self.download_request(request, Spider('foo', download_maxsize=10)) d.addCallback(lambda r: r.body) - d.addCallback(self.assertEquals, "0123456789") + d.addCallback(self.assertEquals, b"0123456789") yield d d = self.download_request(request, Spider('foo', download_maxsize=9)) @@ -257,7 +257,7 @@ class Http11TestCase(HttpTestCase): request = Request(self.getURL('file')) d = self.download_request(request, Spider('foo', download_maxsize=100)) d.addCallback(lambda r: r.body) - d.addCallback(self.assertEquals, "0123456789") + d.addCallback(self.assertEquals, b"0123456789") return d From 4950f5988ef1df5bc6b6ec2c4a70a7956f64b539 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 14 Jan 2016 17:24:08 +0300 Subject: [PATCH 05/21] py3: pass http proxy tests --- scrapy/core/downloader/handlers/http11.py | 7 ++++--- tests/test_downloader_handlers.py | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index dbb002710..d81093a9f 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -181,10 +181,11 @@ class ScrapyAgent(object): if proxy: _, _, proxyHost, proxyPort, proxyParams = _parse(proxy) scheme = _parse(request.url)[0] - omitConnectTunnel = proxyParams.find('noconnect') >= 0 + proxyHost = to_unicode(proxyHost) + omitConnectTunnel = proxyParams.find(b'noconnect') >= 0 if scheme == b'https' and not omitConnectTunnel: proxyConf = (proxyHost, proxyPort, - request.headers.get('Proxy-Authorization', None)) + request.headers.get(b'Proxy-Authorization', None)) return self._TunnelingAgent(reactor, proxyConf, contextFactory=self._contextFactory, connectTimeout=timeout, bindAddress=bindaddress, pool=self._pool) @@ -205,7 +206,7 @@ class ScrapyAgent(object): method = to_bytes(request.method) headers = TxHeaders(request.headers) if isinstance(agent, self._TunnelingAgent): - headers.removeHeader('Proxy-Authorization') + headers.removeHeader(b'Proxy-Authorization') bodyproducer = _RequestBodyProducer(request.body) if request.body else None start_time = time() diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 780f08806..ebf1d2f9c 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -351,7 +351,7 @@ class HttpProxyTestCase(unittest.TestCase): def _test(response): self.assertEquals(response.status, 200) self.assertEquals(response.url, request.url) - self.assertEquals(response.body, 'http://example.com') + self.assertEquals(response.body, b'http://example.com') http_proxy = self.getURL('') request = Request('http://example.com', meta={'proxy': http_proxy}) @@ -361,7 +361,7 @@ class HttpProxyTestCase(unittest.TestCase): def _test(response): self.assertEquals(response.status, 200) self.assertEquals(response.url, request.url) - self.assertEquals(response.body, 'https://example.com') + self.assertEquals(response.body, b'https://example.com') http_proxy = '%s?noconnect' % self.getURL('') request = Request('https://example.com', meta={'proxy': http_proxy}) @@ -371,7 +371,7 @@ class HttpProxyTestCase(unittest.TestCase): def _test(response): self.assertEquals(response.status, 200) self.assertEquals(response.url, request.url) - self.assertEquals(response.body, '/path/to/resource') + self.assertEquals(response.body, b'/path/to/resource') request = Request(self.getURL('path/to/resource')) return self.download_request(request, Spider('foo')).addCallback(_test) From f46a9d595dee801d0ea13d7cdaab8b8de952929f Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 14 Jan 2016 17:31:58 +0300 Subject: [PATCH 06/21] skip ftp tests on py3 for now --- tests/test_downloader_handlers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index ebf1d2f9c..b3c1c565f 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -515,6 +515,8 @@ class FTPTestCase(unittest.TestCase): if twisted_version < (10, 2, 0): skip = "Twisted pre 10.2.0 doesn't allow to set home path other than /home" + if six.PY3: + skip = "Twisted missing ftp support for PY3" def setUp(self): from twisted.protocols.ftp import FTPRealm, FTPFactory From 2aa6c92ffca50c8f6e5d057ac2808a99785eb88f Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 14 Jan 2016 17:52:50 +0300 Subject: [PATCH 07/21] py3 fixes in tests.mockserver --- tests/mockserver.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/mockserver.py b/tests/mockserver.py index 3e4f8c0e5..1ab8e4b8d 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -134,12 +134,12 @@ class Echo(LeafResource): class Partial(LeafResource): def render_GET(self, request): - request.setHeader("Content-Length", "1024") + request.setHeader(b"Content-Length", b"1024") self.deferRequest(request, 0, self._delayedRender, request) return NOT_DONE_YET def _delayedRender(self, request): - request.write("partial content\n") + request.write(b"partial content\n") request.finish() @@ -147,7 +147,7 @@ class Drop(Partial): def _delayedRender(self, request): abort = getarg(request, "abort", 0, type=int) - request.write("this connection will be dropped\n") + request.write(b"this connection will be dropped\n") tr = request.channel.transport try: if abort and hasattr(tr, 'abortConnection'): @@ -162,13 +162,13 @@ class Root(Resource): def __init__(self): Resource.__init__(self) - self.putChild("status", Status()) - self.putChild("follow", Follow()) - self.putChild("delay", Delay()) - self.putChild("partial", Partial()) - self.putChild("drop", Drop()) - self.putChild("raw", Raw()) - self.putChild("echo", Echo()) + self.putChild(b"status", Status()) + self.putChild(b"follow", Follow()) + self.putChild(b"delay", Delay()) + self.putChild(b"partial", Partial()) + self.putChild(b"drop", Drop()) + self.putChild(b"raw", Raw()) + self.putChild(b"echo", Echo()) if six.PY2 and twisted_version > (12, 3, 0): from twisted.web.test.test_webclient import PayloadResource @@ -181,7 +181,7 @@ class Root(Resource): return self def render(self, request): - return 'Scrapy mock HTTP server\n' + return b'Scrapy mock HTTP server\n' class MockServer(): From 81a90c3af65ce863c073e2d83a6b149a03e7d4cb Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 14 Jan 2016 18:47:06 +0300 Subject: [PATCH 08/21] unskip part of test_download_gzip_response on py3, file a twisted issue for the remaining part --- tests/mockserver.py | 6 +++--- tests/test_downloader_handlers.py | 21 ++++++++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/tests/mockserver.py b/tests/mockserver.py index 1ab8e4b8d..02bab0efd 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -170,12 +170,12 @@ class Root(Resource): self.putChild(b"raw", Raw()) self.putChild(b"echo", Echo()) - if six.PY2 and twisted_version > (12, 3, 0): + if twisted_version > (12, 3, 0): from twisted.web.test.test_webclient import PayloadResource from twisted.web.server import GzipEncoderFactory from twisted.web.resource import EncodingResourceWrapper - self.putChild('payload', PayloadResource()) - self.putChild("xpayload", EncodingResourceWrapper(PayloadResource(), [GzipEncoderFactory()])) + self.putChild(b"payload", PayloadResource()) + self.putChild(b"xpayload", EncodingResourceWrapper(PayloadResource(), [GzipEncoderFactory()])) def getChild(self, name, request): return self diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index b3c1c565f..a8de28d4b 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -294,27 +294,30 @@ class Http11MockServerTestCase(unittest.TestCase): @defer.inlineCallbacks def test_download_gzip_response(self): - if six.PY2 and twisted_version > (12, 3, 0): + if twisted_version > (12, 3, 0): crawler = get_crawler(SingleRequestSpider) - body = '1'*100 # PayloadResource requires body length to be 100 + body = b'1'*100 # PayloadResource requires body length to be 100 request = Request('http://localhost:8998/payload', method='POST', body=body, meta={'download_maxsize': 50}) yield crawler.crawl(seed=request) failure = crawler.spider.meta['failure'] # download_maxsize < 100, hence the CancelledError self.assertIsInstance(failure.value, defer.CancelledError) - request.headers.setdefault('Accept-Encoding', 'gzip,deflate') + request.headers.setdefault(b'Accept-Encoding', b'gzip,deflate') request = request.replace(url='http://localhost:8998/xpayload') yield crawler.crawl(seed=request) - # download_maxsize = 50 is enough for the gzipped response - failure = crawler.spider.meta.get('failure') - self.assertTrue(failure == None) - reason = crawler.spider.meta['close_reason'] - self.assertTrue(reason, 'finished') + if six.PY2: + # download_maxsize = 50 is enough for the gzipped response + # See issue https://twistedmatrix.com/trac/ticket/8175 + raise unittest.SkipTest("xpayload only enabled for PY2") + failure = crawler.spider.meta.get('failure') + self.assertTrue(failure == None) + reason = crawler.spider.meta['close_reason'] + self.assertTrue(reason, 'finished') else: - raise unittest.SkipTest("xpayload and payload endpoint only enabled for twisted > 12.3.0 and python 2.x") + raise unittest.SkipTest("xpayload and payload endpoint only enabled for twisted > 12.3.0") class UriResource(resource.Resource): From 99f1f2ad1dbff9ae2e97755b2d61e03ab2339a6d Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Thu, 14 Jan 2016 19:00:48 +0300 Subject: [PATCH 09/21] unskip tests and modules ported to py3 --- tests/py3-ignores.txt | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/py3-ignores.txt b/tests/py3-ignores.txt index 9e75ecf92..57e80f590 100644 --- a/tests/py3-ignores.txt +++ b/tests/py3-ignores.txt @@ -4,7 +4,6 @@ tests/test_command_shell.py tests/test_exporters.py tests/test_linkextractors_deprecated.py tests/test_crawl.py -tests/test_downloader_handlers.py tests/test_downloadermiddleware_httpcache.py tests/test_downloadermiddleware_httpcompression.py tests/test_downloadermiddleware_httpproxy.py @@ -25,8 +24,6 @@ scrapy/xlib/tx/client.py scrapy/xlib/tx/_newclient.py scrapy/xlib/tx/__init__.py scrapy/core/downloader/handlers/s3.py -scrapy/core/downloader/handlers/http11.py -scrapy/core/downloader/handlers/http.py scrapy/core/downloader/handlers/ftp.py scrapy/pipelines/images.py scrapy/pipelines/files.py From 96fcf4cea41a067b4feb4f8adaa5b9ae1d5d38dd Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 15 Jan 2016 13:27:28 +0300 Subject: [PATCH 10/21] add a check that byte url is not accepted in http.Response on py3 --- tests/test_http_response.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_http_response.py b/tests/test_http_response.py index b49d46ea8..710a5b29d 100644 --- a/tests/test_http_response.py +++ b/tests/test_http_response.py @@ -17,6 +17,8 @@ class BaseResponseTest(unittest.TestCase): # Response requires url in the consturctor self.assertRaises(Exception, self.response_class) self.assertTrue(isinstance(self.response_class('http://example.com/'), self.response_class)) + if not six.PY2: + self.assertRaises(TypeError, self.response_class, b"http://example.com") # body can be str or None self.assertTrue(isinstance(self.response_class('http://example.com/', body=b''), self.response_class)) self.assertTrue(isinstance(self.response_class('http://example.com/', body=b'body'), self.response_class)) From a4ca1668d894920e4a74c8f4204fa3ee53039a1d Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Fri, 15 Jan 2016 14:20:19 +0300 Subject: [PATCH 11/21] add https test for http10 handler (no luck with testing https with http11 so far) --- tests/mockserver.py | 12 ++++++++---- tests/test_downloader_handlers.py | 18 +++++++++++++++--- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/tests/mockserver.py b/tests/mockserver.py index 02bab0efd..e7953c4d4 100644 --- a/tests/mockserver.py +++ b/tests/mockserver.py @@ -199,14 +199,18 @@ class MockServer(): time.sleep(0.2) +def ssl_context_factory(): + return ssl.DefaultOpenSSLContextFactory( + os.path.join(os.path.dirname(__file__), 'keys/cert.pem'), + os.path.join(os.path.dirname(__file__), 'keys/cert.pem'), + ) + + if __name__ == "__main__": root = Root() factory = Site(root) httpPort = reactor.listenTCP(8998, factory) - contextFactory = ssl.DefaultOpenSSLContextFactory( - os.path.join(os.path.dirname(__file__), 'keys/cert.pem'), - os.path.join(os.path.dirname(__file__), 'keys/cert.pem'), - ) + contextFactory = ssl_context_factory() httpsPort = reactor.listenSSL(8999, factory, contextFactory) def print_listening(): diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index a8de28d4b..84d1aa191 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -27,7 +27,7 @@ from scrapy.utils.test import get_crawler from scrapy.utils.python import to_bytes from scrapy.exceptions import NotConfigured -from tests.mockserver import MockServer +from tests.mockserver import MockServer, ssl_context_factory from tests.spiders import SingleRequestSpider class DummyDH(object): @@ -102,6 +102,7 @@ class FileTestCase(unittest.TestCase): class HttpTestCase(unittest.TestCase): + scheme = 'http' download_handler_cls = HTTPDownloadHandler def setUp(self): @@ -118,7 +119,12 @@ class HttpTestCase(unittest.TestCase): r.putChild(b"broken", BrokenDownloadResource()) self.site = server.Site(r, timeout=None) self.wrapper = WrappingFactory(self.site) - self.port = reactor.listenTCP(0, self.wrapper, interface='127.0.0.1') + self.host = '127.0.0.1' + if self.scheme == 'https': + self.port = reactor.listenSSL( + 0, self.wrapper, ssl_context_factory(), interface=self.host) + else: + self.port = reactor.listenTCP(0, self.wrapper, interface=self.host) self.portno = self.port.getHost().port self.download_handler = self.download_handler_cls(Settings()) self.download_request = self.download_handler.download_request @@ -130,7 +136,7 @@ class HttpTestCase(unittest.TestCase): yield self.download_handler.close() def getURL(self, path): - return "http://127.0.0.1:%d/%s" % (self.portno, path) + return "%s://%s:%d/%s" % (self.scheme, self.host, self.portno, path) def test_download(self): request = Request(self.getURL('file')) @@ -213,6 +219,12 @@ class Http10TestCase(HttpTestCase): download_handler_cls = HTTP10DownloadHandler +class Https10TestCase(Http10TestCase): + scheme = 'https' + def test_timeout_download_from_spider(self): + raise unittest.SkipTest("test_timeout_download_from_spider skipped under https") + + class Http11TestCase(HttpTestCase): """HTTP 1.1 test case""" download_handler_cls = HTTP11DownloadHandler From 04f69fd18406a9361f3c13df32b5f6ea295ecdbf Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 18 Jan 2016 12:37:46 +0300 Subject: [PATCH 12/21] add https 1.1 downloader test - localhost is a valid DNS-ID --- tests/test_downloader_handlers.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 84d1aa191..80eed86f2 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -119,7 +119,7 @@ class HttpTestCase(unittest.TestCase): r.putChild(b"broken", BrokenDownloadResource()) self.site = server.Site(r, timeout=None) self.wrapper = WrappingFactory(self.site) - self.host = '127.0.0.1' + self.host = 'localhost' if self.scheme == 'https': self.port = reactor.listenSSL( 0, self.wrapper, ssl_context_factory(), interface=self.host) @@ -273,6 +273,10 @@ class Http11TestCase(HttpTestCase): return d +class Https11TestCase(Http11TestCase): + scheme = 'https' + + class Http11MockServerTestCase(unittest.TestCase): """HTTP 1.1 test case with MockServer""" if twisted_version < (11, 1, 0): From 98c060d0b2cc76934e16abc03a033f21850fd565 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 18 Jan 2016 12:42:21 +0300 Subject: [PATCH 13/21] py3: fix http 1.1 test with https, and use self.host everywhere --- tests/test_downloader_handlers.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 80eed86f2..999fa4c0a 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -168,6 +168,9 @@ class HttpTestCase(unittest.TestCase): @defer.inlineCallbacks def test_timeout_download_from_spider(self): + if self.scheme == 'https': + raise unittest.SkipTest( + 'test_timeout_download_from_spider skipped under https') spider = Spider('foo') meta = {'download_timeout': 0.2} # client connects but no data is received @@ -181,7 +184,8 @@ class HttpTestCase(unittest.TestCase): def test_host_header_not_in_request_headers(self): def _test(response): - self.assertEquals(response.body, to_bytes('127.0.0.1:%d' % self.portno)) + self.assertEquals( + response.body, to_bytes('%s:%d' % (self.host, self.portno))) self.assertEquals(request.headers, {}) request = Request(self.getURL('host')) @@ -221,8 +225,6 @@ class Http10TestCase(HttpTestCase): class Https10TestCase(Http10TestCase): scheme = 'https' - def test_timeout_download_from_spider(self): - raise unittest.SkipTest("test_timeout_download_from_spider skipped under https") class Http11TestCase(HttpTestCase): From 0f527849f2e8eddaf5d756b061699f2eca522a18 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 18 Jan 2016 14:44:04 +0300 Subject: [PATCH 14/21] https proxy tunneling - add a test (not perfect, but covers all impl) and fix for py3 --- scrapy/core/downloader/handlers/http11.py | 14 +++++++++----- tests/test_downloader_handlers.py | 10 ++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index d81093a9f..729b80b05 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -78,7 +78,7 @@ class TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): for it. """ - _responseMatcher = re.compile('HTTP/1\.. 200') + _responseMatcher = re.compile(b'HTTP/1\.. 200') def __init__(self, reactor, host, port, proxyConf, contextFactory, timeout=30, bindAddress=None): @@ -92,11 +92,15 @@ class TunnelingTCP4ClientEndpoint(TCP4ClientEndpoint): def requestTunnel(self, protocol): """Asks the proxy to open a tunnel.""" - tunnelReq = 'CONNECT %s:%s HTTP/1.1\r\n' % (self._tunneledHost, - self._tunneledPort) + tunnelReq = ( + b'CONNECT ' + + to_bytes(self._tunneledHost, encoding='ascii') + b':' + + to_bytes(str(self._tunneledPort)) + + b' HTTP/1.1\r\n') if self._proxyAuthHeader: - tunnelReq += 'Proxy-Authorization: %s\r\n' % self._proxyAuthHeader - tunnelReq += '\r\n' + tunnelReq += \ + b'Proxy-Authorization: ' + self._proxyAuthHeader + b'\r\n' + tunnelReq += b'\r\n' protocol.transport.write(tunnelReq) self._protocolDataReceived = protocol.dataReceived protocol.dataReceived = self.processProxyResponse diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 999fa4c0a..2d6c05741 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -388,6 +388,16 @@ class HttpProxyTestCase(unittest.TestCase): request = Request('https://example.com', meta={'proxy': http_proxy}) return self.download_request(request, Spider('foo')).addCallback(_test) + @defer.inlineCallbacks + def test_download_with_proxy_https_timeout(self): + http_proxy = self.getURL('') + domain = 'https://no-such-domain.nosuch' + request = Request( + domain, meta={'proxy': http_proxy, 'download_timeout': 0.2}) + d = self.download_request(request, Spider('foo')) + timeout = yield self.assertFailure(d, error.TimeoutError) + self.assertIn(domain, timeout.osError) + def test_download_without_proxy(self): def _test(response): self.assertEquals(response.status, 200) From 7af64e8fd2a90102d85ff0f453cd6ba1dde71caa Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 18 Jan 2016 15:00:43 +0300 Subject: [PATCH 15/21] py3: remove extra encoding/decoding of url: pass it as bytes only when required --- scrapy/core/downloader/handlers/http11.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 729b80b05..82cf507f7 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -206,7 +206,7 @@ class ScrapyAgent(object): agent = self._get_agent(request, timeout) # request details - url = to_bytes(urldefrag(request.url)[0]) + url = urldefrag(request.url)[0] method = to_bytes(request.method) headers = TxHeaders(request.headers) if isinstance(agent, self._TunnelingAgent): @@ -214,7 +214,8 @@ class ScrapyAgent(object): bodyproducer = _RequestBodyProducer(request.body) if request.body else None start_time = time() - d = agent.request(method, url, 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 @@ -267,10 +268,8 @@ class ScrapyAgent(object): txresponse, body, flags = result status = int(txresponse.code) headers = Headers(txresponse.headers.getAllRawHeaders()) - url = to_unicode(url) respcls = responsetypes.from_args(headers=headers, url=url) - return respcls( - url=url, status=status, headers=headers, body=body, flags=flags) + return respcls(url=url, status=status, headers=headers, body=body, flags=flags) @implementer(IBodyProducer) From b940606b7e3dacbf5d639965c54ccbb87a214c95 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 18 Jan 2016 15:06:15 +0300 Subject: [PATCH 16/21] this is a test for TunnelingTCP4ClientEndpoint - move into Http11ProxyTestCase --- tests/test_downloader_handlers.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 2d6c05741..59320597e 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -388,16 +388,6 @@ class HttpProxyTestCase(unittest.TestCase): request = Request('https://example.com', meta={'proxy': http_proxy}) return self.download_request(request, Spider('foo')).addCallback(_test) - @defer.inlineCallbacks - def test_download_with_proxy_https_timeout(self): - http_proxy = self.getURL('') - domain = 'https://no-such-domain.nosuch' - request = Request( - domain, meta={'proxy': http_proxy, 'download_timeout': 0.2}) - d = self.download_request(request, Spider('foo')) - timeout = yield self.assertFailure(d, error.TimeoutError) - self.assertIn(domain, timeout.osError) - def test_download_without_proxy(self): def _test(response): self.assertEquals(response.status, 200) @@ -422,6 +412,17 @@ class Http11ProxyTestCase(HttpProxyTestCase): if twisted_version < (11, 1, 0): skip = 'HTTP1.1 not supported in twisted < 11.1.0' + @defer.inlineCallbacks + def test_download_with_proxy_https_timeout(self): + """ Test TunnelingTCP4ClientEndpoint """ + http_proxy = self.getURL('') + domain = 'https://no-such-domain.nosuch' + request = Request( + domain, meta={'proxy': http_proxy, 'download_timeout': 0.2}) + d = self.download_request(request, Spider('foo')) + timeout = yield self.assertFailure(d, error.TimeoutError) + self.assertIn(domain, timeout.osError) + class HttpDownloadHandlerMock(object): def __init__(self, settings): From 120fb4adeb957a348f01bf60e821b2dcab2f9de1 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 18 Jan 2016 19:00:40 +0300 Subject: [PATCH 17/21] revert bogus change --- scrapy/http/response/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index 59ef15682..09c4e725a 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -35,7 +35,7 @@ class Response(object_ref): return self._url def _set_url(self, url): - if isinstance(url, six.string_types): + if isinstance(url, str): self._url = url else: raise TypeError('%s url must be str, got %s:' % (type(self).__name__, From 7fdd3225b293d4951ba079e1141683e7fc55f905 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 18 Jan 2016 19:09:09 +0300 Subject: [PATCH 18/21] fix test skipping logic - this is (temporary) py2-only part --- tests/test_downloader_handlers.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 59320597e..e6d219168 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -322,18 +322,18 @@ class Http11MockServerTestCase(unittest.TestCase): # download_maxsize < 100, hence the CancelledError self.assertIsInstance(failure.value, defer.CancelledError) - request.headers.setdefault(b'Accept-Encoding', b'gzip,deflate') - request = request.replace(url='http://localhost:8998/xpayload') - yield crawler.crawl(seed=request) - if six.PY2: + request.headers.setdefault(b'Accept-Encoding', b'gzip,deflate') + request = request.replace(url='http://localhost:8998/xpayload') + yield crawler.crawl(seed=request) # download_maxsize = 50 is enough for the gzipped response # See issue https://twistedmatrix.com/trac/ticket/8175 - raise unittest.SkipTest("xpayload only enabled for PY2") failure = crawler.spider.meta.get('failure') self.assertTrue(failure == None) reason = crawler.spider.meta['close_reason'] self.assertTrue(reason, 'finished') + else: + raise unittest.SkipTest("xpayload only enabled for PY2") else: raise unittest.SkipTest("xpayload and payload endpoint only enabled for twisted > 12.3.0") From de98d8d00658181ed46a96835e2ab95f2f6cd457 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Mon, 18 Jan 2016 19:27:31 +0300 Subject: [PATCH 19/21] move comment about test skipped on py3 into a proper place --- tests/test_downloader_handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index e6d219168..c936b72ed 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -327,12 +327,12 @@ class Http11MockServerTestCase(unittest.TestCase): request = request.replace(url='http://localhost:8998/xpayload') yield crawler.crawl(seed=request) # download_maxsize = 50 is enough for the gzipped response - # See issue https://twistedmatrix.com/trac/ticket/8175 failure = crawler.spider.meta.get('failure') self.assertTrue(failure == None) reason = crawler.spider.meta['close_reason'] self.assertTrue(reason, 'finished') else: + # See issue https://twistedmatrix.com/trac/ticket/8175 raise unittest.SkipTest("xpayload only enabled for PY2") else: raise unittest.SkipTest("xpayload and payload endpoint only enabled for twisted > 12.3.0") From bb50c0be2fd7e737f2e2bd772f333e68cd02db06 Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Tue, 19 Jan 2016 17:30:59 +0300 Subject: [PATCH 20/21] remove unused import --- scrapy/http/response/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/scrapy/http/response/__init__.py b/scrapy/http/response/__init__.py index 09c4e725a..983154001 100644 --- a/scrapy/http/response/__init__.py +++ b/scrapy/http/response/__init__.py @@ -4,7 +4,6 @@ responses in Scrapy. See documentation in docs/topics/request-response.rst """ -import six from six.moves.urllib.parse import urljoin from scrapy.http.headers import Headers From 9c3117a914c27b3acbb2b0300d8938d6e3b49b9e Mon Sep 17 00:00:00 2001 From: Konstantin Lopuhin Date: Tue, 19 Jan 2016 17:32:53 +0300 Subject: [PATCH 21/21] more pythonic check of noconnect in proxy params --- scrapy/core/downloader/handlers/http11.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 82cf507f7..bda72f5e6 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -186,7 +186,7 @@ class ScrapyAgent(object): _, _, proxyHost, proxyPort, proxyParams = _parse(proxy) scheme = _parse(request.url)[0] proxyHost = to_unicode(proxyHost) - omitConnectTunnel = proxyParams.find(b'noconnect') >= 0 + omitConnectTunnel = b'noconnect' in proxyParams if scheme == b'https' and not omitConnectTunnel: proxyConf = (proxyHost, proxyPort, request.headers.get(b'Proxy-Authorization', None))