Merge pull request #1488 from scrapy/py3-redirect-downloader-mware

[MRG+1] PY3 redirect downloader mware

scrapy/downloadermiddlewares/redirect.py

@@ -3,6 +3,7 @@ from six.moves.urllib.parse import urljoin
 
 from scrapy.http import HtmlResponse
 from scrapy.utils.response import get_meta_refresh
+from scrapy.utils.python import to_native_str
 from scrapy.exceptions import IgnoreRequest, NotConfigured
 
 logger = logging.getLogger(__name__)
@@ -55,30 +56,26 @@ class RedirectMiddleware(BaseRedirectMiddleware):
 
     def process_response(self, request, response, spider):
         if (request.meta.get('dont_redirect', False) or
-               response.status in getattr(spider, 'handle_httpstatus_list', []) or
-               response.status in request.meta.get('handle_httpstatus_list', []) or
-               request.meta.get('handle_httpstatus_all', False)):
+                response.status in getattr(spider, 'handle_httpstatus_list', []) or
+                response.status in request.meta.get('handle_httpstatus_list', []) or
+                request.meta.get('handle_httpstatus_all', False)):
             return response
 
-        if request.method == 'HEAD':
-            if response.status in [301, 302, 303, 307] and 'Location' in response.headers:
-                redirected_url = urljoin(request.url, response.headers['location'])
-                redirected = request.replace(url=redirected_url)
-                return self._redirect(redirected, request, spider, response.status)
-            else:
-                return response
+        allowed_status = (301, 302, 303, 307)
+        if 'Location' not in response.headers or response.status not in allowed_status:
+            return response
 
-        if response.status in [302, 303] and 'Location' in response.headers:
-            redirected_url = urljoin(request.url, response.headers['location'])
-            redirected = self._redirect_request_using_get(request, redirected_url)
-            return self._redirect(redirected, request, spider, response.status)
+        # HTTP header is ascii or latin1, redirected url will be percent-encoded utf-8
+        location = to_native_str(response.headers['location'].decode('latin1'))
 
-        if response.status in [301, 307] and 'Location' in response.headers:
-            redirected_url = urljoin(request.url, response.headers['location'])
+        redirected_url = urljoin(request.url, location)
+
+        if response.status in (301, 307) or request.method == 'HEAD':
             redirected = request.replace(url=redirected_url)
             return self._redirect(redirected, request, spider, response.status)
 
-        return response
+        redirected = self._redirect_request_using_get(request, redirected_url)
+        return self._redirect(redirected, request, spider, response.status)
 
 
 class MetaRefreshMiddleware(BaseRedirectMiddleware):

tests/py3-ignores.txt

@@ -10,7 +10,6 @@ tests/test_downloadermiddleware_httpcache.py
 tests/test_downloadermiddleware_httpcompression.py
 tests/test_downloadermiddleware_httpproxy.py
 tests/test_downloadermiddleware.py
-tests/test_downloadermiddleware_redirect.py
 tests/test_downloadermiddleware_retry.py
 tests/test_engine.py
 tests/test_mail.py

tests/test_downloadermiddleware_redirect.py

@@ -1,3 +1,5 @@
+# -*- coding: utf-8 -*-
+
 import unittest
 
 from scrapy.downloadermiddlewares.redirect import RedirectMiddleware, MetaRefreshMiddleware
@@ -150,6 +152,22 @@ class RedirectMiddlewareTest(unittest.TestCase):
                                                            [404, 301, 302]}))
         _test_passthrough(Request(url, meta={'handle_httpstatus_all': True}))
 
+    def test_latin1_location(self):
+        req = Request('http://scrapytest.org/first')
+        latin1_location = u'/ação'.encode('latin1')  # HTTP historically supports latin1
+        resp = Response('http://scrapytest.org/first', headers={'Location': latin1_location}, status=302)
+        req_result = self.mw.process_response(req, resp, self.spider)
+        perc_encoded_utf8_url = 'http://scrapytest.org/a%C3%A7%C3%A3o'
+        self.assertEquals(perc_encoded_utf8_url, req_result.url)
+
+    def test_location_with_wrong_encoding(self):
+        req = Request('http://scrapytest.org/first')
+        utf8_location = u'/ação'  # header with wrong encoding (utf-8)
+        resp = Response('http://scrapytest.org/first', headers={'Location': utf8_location}, status=302)
+        req_result = self.mw.process_response(req, resp, self.spider)
+        perc_encoded_utf8_url = 'http://scrapytest.org/a%C3%83%C2%A7%C3%83%C2%A3o'
+        self.assertEquals(perc_encoded_utf8_url, req_result.url)
+
 
 class MetaRefreshMiddlewareTest(unittest.TestCase):
 
@@ -159,8 +177,8 @@ class MetaRefreshMiddlewareTest(unittest.TestCase):
         self.mw = MetaRefreshMiddleware.from_crawler(crawler)
 
     def _body(self, interval=5, url='http://example.org/newpage'):
-        return """<html><head><meta http-equiv="refresh" content="{0};url={1}"/></head></html>"""\
-                .format(interval, url)
+        html = u"""<html><head><meta http-equiv="refresh" content="{0};url={1}"/></head></html>"""
+        return html.format(interval, url).encode('utf-8')
 
     def test_priority_adjust(self):
         req = Request('http://a.com')
@@ -178,7 +196,9 @@ class MetaRefreshMiddlewareTest(unittest.TestCase):
     def test_meta_refresh_with_high_interval(self):
         # meta-refresh with high intervals don't trigger redirects
         req = Request(url='http://example.org')
-        rsp = HtmlResponse(url='http://example.org', body=self._body(interval=1000))
+        rsp = HtmlResponse(url='http://example.org',
+                           body=self._body(interval=1000),
+                           encoding='utf-8')
         rsp2 = self.mw.process_response(req, rsp, self.spider)
         assert rsp is rsp2
 
@@ -231,5 +251,6 @@ class MetaRefreshMiddlewareTest(unittest.TestCase):
         self.assertEqual(req3.url, 'http://scrapytest.org/redirected2')
         self.assertEqual(req3.meta['redirect_urls'], ['http://scrapytest.org/first', 'http://scrapytest.org/redirected'])
 
+
 if __name__ == "__main__":
     unittest.main()