From 5f02ef82e8560242eb34b336f385addfdef3211d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Fri, 31 Jul 2015 17:27:53 -0300 Subject: [PATCH 1/4] PY3 port http cookies handling --- scrapy/http/cookies.py | 26 +++++++++++++----- tests/py3-ignores.txt | 3 --- tests/test_downloadermiddleware_cookies.py | 31 +++++++++++----------- tests/test_http_cookies.py | 20 +++++++++----- 4 files changed, 49 insertions(+), 31 deletions(-) diff --git a/scrapy/http/cookies.py b/scrapy/http/cookies.py index b1eb767cc..740f21d24 100644 --- a/scrapy/http/cookies.py +++ b/scrapy/http/cookies.py @@ -1,6 +1,9 @@ import time -from cookielib import CookieJar as _CookieJar, DefaultCookiePolicy, IPV4_RE +from six.moves.http_cookiejar import ( + CookieJar as _CookieJar, DefaultCookiePolicy, IPV4_RE +) from scrapy.utils.httpobj import urlparse_cached +from scrapy.utils.python import to_native_str class CookieJar(object): @@ -97,6 +100,7 @@ def potential_domain_matches(domain): pass return matches + ['.' + d for d in matches] + class _DummyLock(object): def acquire(self): pass @@ -133,6 +137,11 @@ class WrappedRequest(object): """ return self.request.meta.get('is_unverifiable', False) + # python3 uses request.unverifiable + @property + def unverifiable(self): + return self.is_unverifiable() + def get_origin_req_host(self): return urlparse_cached(self.request).hostname @@ -140,14 +149,16 @@ class WrappedRequest(object): return name in self.request.headers def get_header(self, name, default=None): - return self.request.headers.get(name, default) + return to_native_str(self.request.headers.get(name, default)) def header_items(self): - return self.request.headers.items() + return [ + (to_native_str(k), [to_native_str(x) for x in v]) + for k, v in self.request.headers.items() + ] def add_unredirected_header(self, name, value): self.request.headers.appendlist(name, value) - #print 'add_unredirected_header', self.request.headers class WrappedResponse(object): @@ -158,5 +169,8 @@ class WrappedResponse(object): def info(self): return self - def getheaders(self, name): - return self.response.headers.getlist(name) + # python3 cookiejars calls get_all + def get_all(self, name, default=None): + return [to_native_str(v) for v in self.response.headers.getlist(name)] + # python2 cookiejars calls getheaders + getheaders = get_all diff --git a/tests/py3-ignores.txt b/tests/py3-ignores.txt index 0d4d397a3..469d2c5e1 100644 --- a/tests/py3-ignores.txt +++ b/tests/py3-ignores.txt @@ -11,7 +11,6 @@ tests/test_crawl.py tests/test_crawler.py tests/test_downloader_handlers.py tests/test_downloadermiddleware_ajaxcrawlable.py -tests/test_downloadermiddleware_cookies.py tests/test_downloadermiddleware_defaultheaders.py tests/test_downloadermiddleware_downloadtimeout.py tests/test_downloadermiddleware_httpauth.py @@ -24,7 +23,6 @@ tests/test_downloadermiddleware_retry.py tests/test_downloadermiddleware_stats.py tests/test_downloadermiddleware_useragent.py tests/test_engine.py -tests/test_http_cookies.py tests/test_logformatter.py tests/test_mail.py tests/test_pipeline_files.py @@ -51,7 +49,6 @@ scrapy/xlib/tx/endpoints.py scrapy/xlib/tx/client.py scrapy/xlib/tx/_newclient.py scrapy/xlib/tx/__init__.py -scrapy/http/cookies.py scrapy/core/downloader/handlers/s3.py scrapy/core/downloader/handlers/http11.py scrapy/core/downloader/handlers/http.py diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 996b8c388..6174f8c3f 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -9,7 +9,7 @@ from scrapy.downloadermiddlewares.cookies import CookiesMiddleware class CookiesMiddlewareTest(TestCase): def assertCookieValEqual(self, first, second, msg=None): - cookievaleq = lambda cv: re.split(';\s*', cv) + cookievaleq = lambda cv: re.split(';\s*', cv.decode('latin1')) return self.assertEqual( sorted(cookievaleq(first)), sorted(cookievaleq(second)), msg) @@ -34,7 +34,7 @@ class CookiesMiddlewareTest(TestCase): req2 = Request('http://scrapytest.org/sub1/') assert self.mw.process_request(req2, self.spider) is None - self.assertEquals(req2.headers.get('Cookie'), "C1=value1") + self.assertEquals(req2.headers.get('Cookie'), b"C1=value1") def test_dont_merge_cookies(self): # merge some cookies into jar @@ -55,12 +55,12 @@ class CookiesMiddlewareTest(TestCase): # check that cookies are merged back req = Request('http://scrapytest.org/mergeme') assert self.mw.process_request(req, self.spider) is None - self.assertEquals(req.headers.get('Cookie'), 'C1=value1') + self.assertEquals(req.headers.get('Cookie'), b'C1=value1') # check that cookies are merged when dont_merge_cookies is passed as 0 req = Request('http://scrapytest.org/mergeme', meta={'dont_merge_cookies': 0}) assert self.mw.process_request(req, self.spider) is None - self.assertEquals(req.headers.get('Cookie'), 'C1=value1') + self.assertEquals(req.headers.get('Cookie'), b'C1=value1') def test_complex_cookies(self): # merge some cookies into jar @@ -76,12 +76,12 @@ class CookiesMiddlewareTest(TestCase): # embed C1 and C3 for scrapytest.org/foo req = Request('http://scrapytest.org/foo') self.mw.process_request(req, self.spider) - assert req.headers.get('Cookie') in ('C1=value1; C3=value3', 'C3=value3; C1=value1') + assert req.headers.get('Cookie') in (b'C1=value1; C3=value3', b'C3=value3; C1=value1') # embed C2 for scrapytest.org/bar req = Request('http://scrapytest.org/bar') self.mw.process_request(req, self.spider) - self.assertEquals(req.headers.get('Cookie'), 'C2=value2') + self.assertEquals(req.headers.get('Cookie'), b'C2=value2') # embed nothing for scrapytest.org/baz req = Request('http://scrapytest.org/baz') @@ -91,7 +91,7 @@ class CookiesMiddlewareTest(TestCase): def test_merge_request_cookies(self): req = Request('http://scrapytest.org/', cookies={'galleta': 'salada'}) assert self.mw.process_request(req, self.spider) is None - self.assertEquals(req.headers.get('Cookie'), 'galleta=salada') + self.assertEquals(req.headers.get('Cookie'), b'galleta=salada') headers = {'Set-Cookie': 'C1=value1; path=/'} res = Response('http://scrapytest.org/', headers=headers) @@ -100,12 +100,12 @@ class CookiesMiddlewareTest(TestCase): req2 = Request('http://scrapytest.org/sub1/') assert self.mw.process_request(req2, self.spider) is None - self.assertCookieValEqual(req2.headers.get('Cookie'), "C1=value1; galleta=salada") + self.assertCookieValEqual(req2.headers.get('Cookie'), b"C1=value1; galleta=salada") def test_cookiejar_key(self): req = Request('http://scrapytest.org/', cookies={'galleta': 'salada'}, meta={'cookiejar': "store1"}) assert self.mw.process_request(req, self.spider) is None - self.assertEquals(req.headers.get('Cookie'), 'galleta=salada') + self.assertEquals(req.headers.get('Cookie'), b'galleta=salada') headers = {'Set-Cookie': 'C1=value1; path=/'} res = Response('http://scrapytest.org/', headers=headers, request=req) @@ -113,11 +113,11 @@ class CookiesMiddlewareTest(TestCase): req2 = Request('http://scrapytest.org/', meta=res.meta) assert self.mw.process_request(req2, self.spider) is None - self.assertCookieValEqual(req2.headers.get('Cookie'),'C1=value1; galleta=salada') + self.assertCookieValEqual(req2.headers.get('Cookie'), b'C1=value1; galleta=salada') req3 = Request('http://scrapytest.org/', cookies={'galleta': 'dulce'}, meta={'cookiejar': "store2"}) assert self.mw.process_request(req3, self.spider) is None - self.assertEquals(req3.headers.get('Cookie'), 'galleta=dulce') + self.assertEquals(req3.headers.get('Cookie'), b'galleta=dulce') headers = {'Set-Cookie': 'C2=value2; path=/'} res2 = Response('http://scrapytest.org/', headers=headers, request=req3) @@ -125,7 +125,7 @@ class CookiesMiddlewareTest(TestCase): req4 = Request('http://scrapytest.org/', meta=res2.meta) assert self.mw.process_request(req4, self.spider) is None - self.assertCookieValEqual(req4.headers.get('Cookie'), 'C2=value2; galleta=dulce') + self.assertCookieValEqual(req4.headers.get('Cookie'), b'C2=value2; galleta=dulce') #cookies from hosts with port req5_1 = Request('http://scrapytest.org:1104/') @@ -137,11 +137,11 @@ class CookiesMiddlewareTest(TestCase): req5_2 = Request('http://scrapytest.org:1104/some-redirected-path') assert self.mw.process_request(req5_2, self.spider) is None - self.assertEquals(req5_2.headers.get('Cookie'), 'C1=value1') + self.assertEquals(req5_2.headers.get('Cookie'), b'C1=value1') req5_3 = Request('http://scrapytest.org/some-redirected-path') assert self.mw.process_request(req5_3, self.spider) is None - self.assertEquals(req5_3.headers.get('Cookie'), 'C1=value1') + self.assertEquals(req5_3.headers.get('Cookie'), b'C1=value1') #skip cookie retrieval for not http request req6 = Request('file:///scrapy/sometempfile') @@ -152,5 +152,4 @@ class CookiesMiddlewareTest(TestCase): request = Request("http://example-host/", cookies={'currencyCookie': 'USD'}) assert self.mw.process_request(request, self.spider) is None self.assertIn('Cookie', request.headers) - self.assertIn('currencyCookie', request.headers['Cookie']) - + self.assertEqual(b'currencyCookie=USD', request.headers['Cookie']) diff --git a/tests/test_http_cookies.py b/tests/test_http_cookies.py index 3d6993491..d529f609b 100644 --- a/tests/test_http_cookies.py +++ b/tests/test_http_cookies.py @@ -8,8 +8,8 @@ from scrapy.http.cookies import WrappedRequest, WrappedResponse class WrappedRequestTest(TestCase): def setUp(self): - self.request = Request("http://www.example.com/page.html", \ - headers={"Content-Type": "text/html"}) + self.request = Request("http://www.example.com/page.html", + headers={"Content-Type": "text/html"}) self.wrapped = WrappedRequest(self.request) def test_get_full_url(self): @@ -23,10 +23,12 @@ class WrappedRequestTest(TestCase): def test_is_unverifiable(self): self.assertFalse(self.wrapped.is_unverifiable()) + self.assertFalse(self.wrapped.unverifiable) def test_is_unverifiable2(self): self.request.meta['is_unverifiable'] = True self.assertTrue(self.wrapped.is_unverifiable()) + self.assertTrue(self.wrapped.unverifiable) def test_get_origin_req_host(self): self.assertEqual(self.wrapped.get_origin_req_host(), 'www.example.com') @@ -40,17 +42,19 @@ class WrappedRequestTest(TestCase): self.assertEqual(self.wrapped.get_header('xxxxx', 'def'), 'def') def test_header_items(self): - self.assertEqual(self.wrapped.header_items(), [('Content-Type', ['text/html'])]) + self.assertEqual(self.wrapped.header_items(), + [('Content-Type', ['text/html'])]) def test_add_unredirected_header(self): self.wrapped.add_unredirected_header('hello', 'world') - self.assertEqual(self.request.headers['hello'], 'world') + self.assertEqual(self.request.headers['hello'], b'world') + class WrappedResponseTest(TestCase): def setUp(self): - self.response = Response("http://www.example.com/page.html", - headers={"Content-TYpe": "text/html"}) + self.response = Response("http://www.example.com/page.html", + headers={"Content-TYpe": "text/html"}) self.wrapped = WrappedResponse(self.response) def test_info(self): @@ -58,3 +62,7 @@ class WrappedResponseTest(TestCase): def test_getheaders(self): self.assertEqual(self.wrapped.getheaders('content-type'), ['text/html']) + + def test_get_all(self): + # get_all result must be native string + self.assertEqual(self.wrapped.get_all('content-type'), ['text/html']) From dba7e39f61cbe2c22d3c9064f32f6e36d74f14b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Mon, 3 Aug 2015 10:53:40 -0300 Subject: [PATCH 2/4] Do not break cookie parsing on non-utf8 headers --- scrapy/http/cookies.py | 9 ++++++--- tests/test_downloadermiddleware_cookies.py | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/scrapy/http/cookies.py b/scrapy/http/cookies.py index 740f21d24..e92c3fe73 100644 --- a/scrapy/http/cookies.py +++ b/scrapy/http/cookies.py @@ -149,11 +149,13 @@ class WrappedRequest(object): return name in self.request.headers def get_header(self, name, default=None): - return to_native_str(self.request.headers.get(name, default)) + return to_native_str(self.request.headers.get(name, default), + errors='replace') def header_items(self): return [ - (to_native_str(k), [to_native_str(x) for x in v]) + (to_native_str(k, errors='replace'), + [to_native_str(x, errors='replace') for x in v]) for k, v in self.request.headers.items() ] @@ -171,6 +173,7 @@ class WrappedResponse(object): # python3 cookiejars calls get_all def get_all(self, name, default=None): - return [to_native_str(v) for v in self.response.headers.getlist(name)] + return [to_native_str(v, errors='replace') + for v in self.response.headers.getlist(name)] # python2 cookiejars calls getheaders getheaders = get_all diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 6174f8c3f..63be0beb8 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -22,20 +22,32 @@ class CookiesMiddlewareTest(TestCase): del self.mw def test_basic(self): - headers = {'Set-Cookie': 'C1=value1; path=/'} req = Request('http://scrapytest.org/') assert self.mw.process_request(req, self.spider) is None assert 'Cookie' not in req.headers + headers = {'Set-Cookie': 'C1=value1; path=/'} res = Response('http://scrapytest.org/', headers=headers) assert self.mw.process_response(req, res, self.spider) is res - #assert res.cookies - req2 = Request('http://scrapytest.org/sub1/') assert self.mw.process_request(req2, self.spider) is None self.assertEquals(req2.headers.get('Cookie'), b"C1=value1") + def test_do_not_break_on_non_utf8_header(self): + req = Request('http://scrapytest.org/') + assert self.mw.process_request(req, self.spider) is None + assert 'Cookie' not in req.headers + + headers = {'Set-Cookie': b'C1=in\xa3valid; path=/', + 'Other': b'ignore\xa3me'} + res = Response('http://scrapytest.org/', headers=headers) + assert self.mw.process_response(req, res, self.spider) is res + + req2 = Request('http://scrapytest.org/sub1/') + assert self.mw.process_request(req2, self.spider) is None + self.assertIn('Cookie', req2.headers) + def test_dont_merge_cookies(self): # merge some cookies into jar headers = {'Set-Cookie': 'C1=value1; path=/'} From c6adf648dcfe59b52c0e4663e701a232e78d7bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Mon, 3 Aug 2015 16:28:29 -0300 Subject: [PATCH 3/4] PY3 port COOKIES_DEBUG and add tests --- scrapy/downloadermiddlewares/cookies.py | 15 +++-- scrapy/mail.py | 3 +- tests/test_downloadermiddleware_cookies.py | 64 +++++++++++++++++++++- 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/scrapy/downloadermiddlewares/cookies.py b/scrapy/downloadermiddlewares/cookies.py index 270d621cd..321c0171b 100644 --- a/scrapy/downloadermiddlewares/cookies.py +++ b/scrapy/downloadermiddlewares/cookies.py @@ -6,6 +6,7 @@ from collections import defaultdict from scrapy.exceptions import NotConfigured from scrapy.http import Response from scrapy.http.cookies import CookieJar +from scrapy.utils.python import to_native_str logger = logging.getLogger(__name__) @@ -52,18 +53,20 @@ class CookiesMiddleware(object): def _debug_cookie(self, request, spider): if self.debug: - cl = request.headers.getlist('Cookie') + cl = [to_native_str(c, errors='replace') + for c in request.headers.getlist('Cookie')] if cl: - msg = "Sending cookies to: %s" % request + os.linesep - msg += os.linesep.join("Cookie: %s" % c for c in cl) + cookies = "\n".join("Cookie: {}\n".format(c) for c in cl) + msg = "Sending cookies to: {}\n{}".format(request, cookies) logger.debug(msg, extra={'spider': spider}) def _debug_set_cookie(self, response, spider): if self.debug: - cl = response.headers.getlist('Set-Cookie') + cl = [to_native_str(c, errors='replace') + for c in response.headers.getlist('Set-Cookie')] if cl: - msg = "Received cookies from: %s" % response + os.linesep - msg += os.linesep.join("Set-Cookie: %s" % c for c in cl) + cookies = "\n".join("Set-Cookie: {}\n".format(c) for c in cl) + msg = "Received cookies from: {}\n{}".format(response, cookies) logger.debug(msg, extra={'spider': spider}) def _format_cookie(self, cookie): diff --git a/scrapy/mail.py b/scrapy/mail.py index 2b4c57980..ad8ecbe13 100644 --- a/scrapy/mail.py +++ b/scrapy/mail.py @@ -20,7 +20,6 @@ else: from email import encoders as Encoders from twisted.internet import defer, reactor, ssl -from twisted.mail.smtp import ESMTPSenderFactory logger = logging.getLogger(__name__) @@ -102,6 +101,8 @@ class MailSender(object): 'mailattachs': nattachs, 'mailerr': errstr}) def _sendmail(self, to_addrs, msg): + # Import twisted.mail here because it is not available in python3 + from twisted.mail.smtp import ESMTPSenderFactory msg = StringIO(msg) d = defer.Deferred() factory = ESMTPSenderFactory(self.smtpuser, self.smtppass, self.mailfrom, \ diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 63be0beb8..66d9faa79 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -1,8 +1,12 @@ -from unittest import TestCase import re +import logging +from unittest import TestCase +from testfixtures import LogCapture from scrapy.http import Response, Request from scrapy.spiders import Spider +from scrapy.utils.test import get_crawler +from scrapy.exceptions import NotConfigured from scrapy.downloadermiddlewares.cookies import CookiesMiddleware @@ -34,6 +38,64 @@ class CookiesMiddlewareTest(TestCase): assert self.mw.process_request(req2, self.spider) is None self.assertEquals(req2.headers.get('Cookie'), b"C1=value1") + def test_setting_false_cookies_enabled(self): + self.assertRaises( + NotConfigured, + CookiesMiddleware.from_crawler, + get_crawler(settings_dict={'COOKIES_ENABLED': False}) + ) + + def test_setting_default_cookies_enabled(self): + self.assertIsInstance( + CookiesMiddleware.from_crawler(get_crawler()), + CookiesMiddleware + ) + + def test_setting_true_cookies_enabled(self): + self.assertIsInstance( + CookiesMiddleware.from_crawler( + get_crawler(settings_dict={'COOKIES_ENABLED': True}) + ), + CookiesMiddleware + ) + + def test_setting_enabled_cookies_debug(self): + crawler = get_crawler(settings_dict={'COOKIES_DEBUG': True}) + mw = CookiesMiddleware.from_crawler(crawler) + with LogCapture('scrapy.downloadermiddlewares.cookies', + level=logging.DEBUG) as l: + req = Request('http://scrapytest.org/') + res = Response('http://scrapytest.org/', + headers={'Set-Cookie': 'C1=value1; path=/'}) + mw.process_response(req, res, crawler.spider) + req2 = Request('http://scrapytest.org/sub1/') + mw.process_request(req2, crawler.spider) + + l.check( + ('scrapy.downloadermiddlewares.cookies', + 'DEBUG', + 'Received cookies from: <200 http://scrapytest.org/>\n' + 'Set-Cookie: C1=value1; path=/\n'), + ('scrapy.downloadermiddlewares.cookies', + 'DEBUG', + 'Sending cookies to: \n' + 'Cookie: C1=value1\n'), + ) + + def test_setting_disabled_cookies_debug(self): + crawler = get_crawler(settings_dict={'COOKIES_DEBUG': False}) + mw = CookiesMiddleware.from_crawler(crawler) + with LogCapture('scrapy.downloadermiddlewares.cookies', + level=logging.DEBUG) as l: + req = Request('http://scrapytest.org/') + res = Response('http://scrapytest.org/', + headers={'Set-Cookie': 'C1=value1; path=/'}) + mw.process_response(req, res, crawler.spider) + req2 = Request('http://scrapytest.org/sub1/') + mw.process_request(req2, crawler.spider) + + l.check() + def test_do_not_break_on_non_utf8_header(self): req = Request('http://scrapytest.org/') assert self.mw.process_request(req, self.spider) is None From f4fc05c2a1a676f711650d1661f7f5f78a27b501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gra=C3=B1a?= Date: Mon, 3 Aug 2015 17:18:08 -0300 Subject: [PATCH 4/4] Do not propagate cookie log messages in tests so TopLevelFormatter does not rewrite them --- tests/test_downloadermiddleware_cookies.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 66d9faa79..26d9794b6 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -63,6 +63,7 @@ class CookiesMiddlewareTest(TestCase): crawler = get_crawler(settings_dict={'COOKIES_DEBUG': True}) mw = CookiesMiddleware.from_crawler(crawler) with LogCapture('scrapy.downloadermiddlewares.cookies', + propagate=False, level=logging.DEBUG) as l: req = Request('http://scrapytest.org/') res = Response('http://scrapytest.org/', @@ -86,6 +87,7 @@ class CookiesMiddlewareTest(TestCase): crawler = get_crawler(settings_dict={'COOKIES_DEBUG': False}) mw = CookiesMiddleware.from_crawler(crawler) with LogCapture('scrapy.downloadermiddlewares.cookies', + propagate=False, level=logging.DEBUG) as l: req = Request('http://scrapytest.org/') res = Response('http://scrapytest.org/',