redirectmw: allow dont_filter requests to continue redirecting but limit max redirections by global setting and per request

--HG--
extra : convert_revision : svn%3Ab85faa78-f9eb-468e-a121-7cced6da292c%401004

scrapy/trunk/scrapy/conf/default_settings.py

@@ -135,6 +135,8 @@ NEWSPIDER_MODULE = ''
 
 PRIORITIZER = 'scrapy.core.prioritizers.RandomPrioritizer'
 
+REDIRECTMIDDLEWARE_MAX_TIMES = 20 # uses Firefox default setting
+
 REQUEST_HEADER_ACCEPT = 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8'
 REQUEST_HEADER_ACCEPT_LANGUAGE = 'en'
 
@@ -145,6 +147,7 @@ REQUESTS_PER_DOMAIN = 8     # max simultaneous requests per domain
 RETRY_TIMES = 2 # initial response + 2 retries = 3 requests
 RETRY_HTTP_CODES = ['500', '503', '504', '400', '408', '200']
 
+
 ROBOTSTXT_OBEY = False
 
 SCHEDULER = 'scrapy.core.scheduler.Scheduler'

scrapy/trunk/scrapy/contrib/downloadermiddleware/redirect.py

@@ -2,9 +2,7 @@ from scrapy import log
 from scrapy.core.exceptions import HttpException
 from scrapy.utils.url import urljoin_rfc as urljoin
 from scrapy.utils.response import get_meta_refresh
-
-class RedirectLoop(Exception):
-    pass
+from scrapy.conf import settings
 
 # some sites use meta-refresh for redirecting to a session expired page, so we
 # restrict automatic redirection to a maximum delay (in number of seconds)
@@ -12,6 +10,9 @@ META_REFRESH_MAXSEC = 100
 MAX_REDIRECT_LOOP = 10
 
 class RedirectMiddleware(object):
+    def __init__(self):
+        self.max_redirect_times = settings.getint('REDIRECTMIDDLEWARE_MAX_TIMES')
+
     def process_exception(self, request, exception, spider):
         if not isinstance(exception, HttpException):
             return
@@ -39,7 +40,18 @@ class RedirectMiddleware(object):
         return response
 
     def _redirect(self, redirected, request, spider, reason):
-        domain = spider.domain_name
-        log.msg("Redirecting (%s) to %s from %s" % (reason, redirected, request), level=log.DEBUG, domain=domain)
-        return redirected
+        ttl = request.meta.setdefault('redirect_ttl', self.max_redirect_times)
+        redirects = request.meta.get('redirect_times', 0) + 1
+
+        if ttl and redirects <= self.max_redirect_times:
+            redirected.meta['redirect_times'] = redirects
+            redirected.meta['redirect_ttl'] = ttl - 1
+            redirected.dont_filter = request.dont_filter
+            log.msg("Redirecting (%s) to %s from %s" % (reason, redirected, request),
+                    domain=spider.domain_name, level=log.DEBUG)
+            return redirected
+        else:
+            log.msg("Discarding %s: max redirections reached" % request,
+                    domain=spider.domain_name, level=log.DEBUG)
+
 

scrapy/trunk/scrapy/tests/test_downloadermiddleware_redirect.py

@@ -13,14 +13,12 @@ class RedirectMiddlewareTest(unittest.TestCase):
         spiders.reload()
         self.spider = spiders.fromdomain('scrapytest.org')
         dupefilter.open('scrapytest.org')
+        self.mw = RedirectMiddleware()
 
     def tearDown(self):
         dupefilter.close('scrapytest.org')
 
     def test_process_exception(self):
-
-        mw = RedirectMiddleware()
-
         url = 'http://www.example.com/301'
         url2 = 'http://www.example.com/redirected'
         req = Request(url)
@@ -28,7 +26,7 @@ class RedirectMiddlewareTest(unittest.TestCase):
         rsp = Response(url, headers=hdr)
         exc = HttpException('301', None, rsp)
 
-        req2 = mw.process_exception(req, exc, self.spider)
+        req2 = self.mw.process_exception(req, exc, self.spider)
         assert isinstance(req2, Request)
         self.assertEqual(req2.url, url2)
 
@@ -39,22 +37,19 @@ class RedirectMiddlewareTest(unittest.TestCase):
         rsp = Response(url, headers=hdr)
         exc = HttpException('302', None, rsp)
 
-        req2 = mw.process_exception(req, exc, self.spider)
+        req2 = self.mw.process_exception(req, exc, self.spider)
         assert isinstance(req2, Request)
         self.assertEqual(req2.url, url2)
         self.assertEqual(req2.method, 'GET')
         assert not req2.body
 
     def test_process_response(self):
-
-        mw = RedirectMiddleware()
-
         body = """<html>
             <head><meta http-equiv="refresh" content="5;url=http://example.org/newpage" /></head>
             </html>"""
         req = Request(url='http://example.org')
         rsp = Response(url='http://example.org', body=body)
-        req2 = mw.process_response(req, rsp, self.spider)
+        req2 = self.mw.process_response(req, rsp, self.spider)
 
         assert isinstance(req2, Request)
         self.assertEqual(req2.url, 'http://example.org/newpage')
@@ -65,9 +60,32 @@ class RedirectMiddlewareTest(unittest.TestCase):
             </html>"""
         req = Request(url='http://example.org')
         rsp = Response(url='http://example.org', body=body)
-        rsp2 = mw.process_response(req, rsp, self.spider)
+        rsp2 = self.mw.process_response(req, rsp, self.spider)
 
         assert rsp is rsp2
 
+    def test_max_redirect_times(self):
+        self.mw.max_redirect_times = 1
+        req = Request('http://scrapytest.org/302')
+        exc = HttpException('302', None, Response('http://www.scrapytest.org/302', headers={'Location': '/redirected'}))
+
+        req = self.mw.process_exception(req, exc, self.spider)
+        assert isinstance(req, Request)
+        assert 'redirect_times' in req.meta
+        self.assertEqual(req.meta['redirect_times'], 1)
+
+        req = self.mw.process_exception(req, exc, self.spider)
+        self.assertEqual(req, None)
+
+    def test_ttl(self):
+        self.mw.max_redirect_times = 100
+        req = Request('http://scrapytest.org/302', meta={'redirect_ttl': 1})
+        exc = HttpException('302', None, Response('http://www.scrapytest.org/302', headers={'Location': '/redirected'}))
+
+        req = self.mw.process_exception(req, exc, self.spider)
+        assert isinstance(req, Request)
+        req = self.mw.process_exception(req, exc, self.spider)
+        self.assertEqual(req, None)
+
 if __name__ == "__main__":
     unittest.main()