Merge pull request scrapy#1398 from scrapy/py3-cookies

PY3 port http cookies handling

Author: kmike

scrapy/downloadermiddlewares/cookies.py

@@ -6,6 +6,7 @@
 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 @@ def process_response(self, request, response, spider):
 
     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):

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,21 +137,30 @@ def is_unverifiable(self):
         """
         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
 
     def has_header(self, name):
         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),
+                             errors='replace')
 
     def header_items(self):
-        return self.request.headers.items()
+        return [
+            (to_native_str(k, errors='replace'),
+             [to_native_str(x, errors='replace') 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 +171,9 @@ def __init__(self, response):
     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, errors='replace')
+                for v in self.response.headers.getlist(name)]
+    # python2 cookiejars calls getheaders
+    getheaders = get_all

scrapy/mail.py

@@ -20,7 +20,6 @@
     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 @@ def _sent_failed(self, failure, to, cc, subject, nattachs):
                       '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, \

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

tests/test_downloadermiddleware_cookies.py

@@ -1,15 +1,19 @@
-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
 
 
 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)
@@ -22,19 +26,91 @@ def tearDown(self):
         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_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',
+                        propagate=False,
+                        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: <GET http://scrapytest.org/sub1/>\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',
+                        propagate=False,
+                        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
+        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.assertEquals(req2.headers.get('Cookie'), "C1=value1")
+        self.assertIn('Cookie', req2.headers)
 
     def test_dont_merge_cookies(self):
         # merge some cookies into jar
@@ -55,12 +131,12 @@ def test_dont_merge_cookies(self):
         # 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 +152,12 @@ def test_complex_cookies(self):
         # 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 +167,7 @@ def test_complex_cookies(self):
     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,32 +176,32 @@ def test_merge_request_cookies(self):
         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)
         assert self.mw.process_response(req, res, self.spider) is res
 
         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)
         assert self.mw.process_response(req3, res2, self.spider) is res2
 
         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 +213,11 @@ def test_cookiejar_key(self):
 
         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 +228,4 @@ def test_local_domain(self):
         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'])