Merge pull request #1391 from nyov/py3-robots

PY3 port scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware

scrapy/downloadermiddlewares/robotstxt.py

@@ -65,5 +65,17 @@ class RobotsTxtMiddleware(object):
 
     def _parse_robots(self, response):
         rp = robotparser.RobotFileParser(response.url)
-        rp.parse(response.body.splitlines())
+        body = ''
+        if hasattr(response, 'body_as_unicode'):
+            body = response.body_as_unicode()
+        else: # last effort try
+            try:
+                body = response.body.decode('utf-8')
+            except UnicodeDecodeError:
+                # If we found garbage, disregard it:,
+                # but keep the lookup cached (in self._parsers)
+                # Running rp.parse() will set rp state from
+                # 'disallow all' to 'allow any'.
+                pass
+        rp.parse(body.splitlines())
         self._parsers[urlparse_cached(response).netloc] = rp

tests/test_downloadermiddleware_robotstxt.py

@@ -6,15 +6,44 @@ from twisted.python import failure
 from twisted.trial import unittest
 from scrapy.downloadermiddlewares.robotstxt import RobotsTxtMiddleware
 from scrapy.exceptions import IgnoreRequest, NotConfigured
-from scrapy.http import Request, Response
+from scrapy.http import Request, Response, TextResponse
 from scrapy.settings import Settings
 from tests import mock
 
 
 class RobotsTxtMiddlewareTest(unittest.TestCase):
 
+    def setUp(self):
+        self.crawler = mock.MagicMock()
+        self.crawler.settings = Settings()
+        self.crawler.engine.download = mock.MagicMock()
+
+    def tearDown(self):
+        del self.crawler
+
+    def test_robotstxt_settings(self):
+        self.crawler.settings = Settings()
+        self.crawler.settings.set('USER_AGENT', 'CustomAgent')
+        self.assertRaises(NotConfigured, RobotsTxtMiddleware, self.crawler)
+
+    def _get_successful_crawler(self):
+        crawler = self.crawler
+        crawler.settings.set('ROBOTSTXT_OBEY', True)
+        ROBOTS = re.sub(b'^\s+(?m)', b'', b'''
+        User-Agent: *
+        Disallow: /admin/
+        Disallow: /static/
+        ''')
+        response = TextResponse('http://site.local/robots.txt', body=ROBOTS)
+        def return_response(request, spider):
+            deferred = Deferred()
+            reactor.callFromThread(deferred.callback, response)
+            return deferred
+        crawler.engine.download.side_effect = return_response
+        return crawler
+
     def test_robotstxt(self):
-        middleware = self._get_middleware()
+        middleware = RobotsTxtMiddleware(self._get_successful_crawler())
         # There is a bit of neglect in robotstxt.py: robots.txt is fetched asynchronously,
         # and it is actually fetched only *after* first process_request completes.
         # So, first process_request will always succeed.
@@ -30,8 +59,8 @@ class RobotsTxtMiddlewareTest(unittest.TestCase):
         return deferred
 
     def test_robotstxt_meta(self):
+        middleware = RobotsTxtMiddleware(self._get_successful_crawler())
         meta = {'dont_obey_robotstxt': True}
-        middleware = self._get_middleware()
         self.assertNotIgnored(Request('http://site.local', meta=meta), middleware)
         def test(r):
             self.assertNotIgnored(Request('http://site.local/allowed', meta=meta), middleware)
@@ -42,19 +71,67 @@ class RobotsTxtMiddlewareTest(unittest.TestCase):
         reactor.callFromThread(deferred.callback, None)
         return deferred
 
-    def test_robotstxt_error(self):
-        crawler = mock.MagicMock()
-        crawler.settings = Settings()
+    def _get_garbage_crawler(self):
+        crawler = self.crawler
         crawler.settings.set('ROBOTSTXT_OBEY', True)
-        crawler.engine.download = mock.MagicMock()
+        response = Response('http://site.local/robots.txt', body=b'GIF89a\xd3\x00\xfe\x00\xa2')
+        def return_response(request, spider):
+            deferred = Deferred()
+            reactor.callFromThread(deferred.callback, response)
+            return deferred
+        crawler.engine.download.side_effect = return_response
+        return crawler
+
+    def test_robotstxt_garbage(self):
+        # garbage response should be discarded, equal 'allow all'
+        middleware = RobotsTxtMiddleware(self._get_garbage_crawler())
+        middleware._logerror = mock.MagicMock()
+        middleware.process_request(Request('http://site.local'), None)
+        self.assertNotIgnored(Request('http://site.local'), middleware)
+        def test(r):
+            self.assertNotIgnored(Request('http://site.local/allowed'), middleware)
+            self.assertNotIgnored(Request('http://site.local/admin/main'), middleware)
+            self.assertNotIgnored(Request('http://site.local/static/'), middleware)
+        deferred = Deferred()
+        deferred.addCallback(test)
+        deferred.addErrback(lambda _: self.assertIsNone(middleware._logerror.assert_any_call()))
+        reactor.callFromThread(deferred.callback, None)
+        return deferred
+
+    def _get_emptybody_crawler(self):
+        crawler = self.crawler
+        crawler.settings.set('ROBOTSTXT_OBEY', True)
+        response = Response('http://site.local/robots.txt')
+        def return_response(request, spider):
+            deferred = Deferred()
+            reactor.callFromThread(deferred.callback, response)
+            return deferred
+        crawler.engine.download.side_effect = return_response
+        return crawler
+
+    def test_robotstxt_empty_response(self):
+        # empty response should equal 'allow all'
+        middleware = RobotsTxtMiddleware(self._get_emptybody_crawler())
+        self.assertNotIgnored(Request('http://site.local'), middleware)
+        def test(r):
+            self.assertNotIgnored(Request('http://site.local/allowed'), middleware)
+            self.assertNotIgnored(Request('http://site.local/admin/main'), middleware)
+            self.assertNotIgnored(Request('http://site.local/static/'), middleware)
+        deferred = Deferred()
+        deferred.addCallback(test)
+        reactor.callFromThread(deferred.callback, None)
+        return deferred
+
+    def test_robotstxt_error(self):
+        self.crawler.settings.set('ROBOTSTXT_OBEY', True)
         err = error.DNSLookupError('Robotstxt address not found')
         def return_failure(request, spider):
             deferred = Deferred()
             reactor.callFromThread(deferred.errback, failure.Failure(err))
             return deferred
-        crawler.engine.download.side_effect = return_failure
+        self.crawler.engine.download.side_effect = return_failure
 
-        middleware = RobotsTxtMiddleware(crawler)
+        middleware = RobotsTxtMiddleware(self.crawler)
         middleware._logerror = mock.MagicMock()
         middleware.process_request(Request('http://site.local'), None)
         deferred = Deferred()
@@ -69,27 +146,3 @@ class RobotsTxtMiddlewareTest(unittest.TestCase):
     def assertIgnored(self, request, middleware):
         spider = None  # not actually used
         self.assertRaises(IgnoreRequest, middleware.process_request, request, spider)
-
-    def _get_crawler(self):
-        crawler = mock.MagicMock()
-        crawler.settings = Settings()
-        crawler.settings.set('USER_AGENT', 'CustomAgent')
-        self.assertRaises(NotConfigured, RobotsTxtMiddleware, crawler)
-        crawler.settings.set('ROBOTSTXT_OBEY', True)
-        crawler.engine.download = mock.MagicMock()
-        ROBOTS = re.sub(r'^\s+(?m)', '', '''
-        User-Agent: *
-        Disallow: /admin/
-        Disallow: /static/
-        ''')
-        response = Response('http://site.local/robots.txt', body=ROBOTS)
-        def return_response(request, spider):
-            deferred = Deferred()
-            reactor.callFromThread(deferred.callback, response)
-            return deferred
-        crawler.engine.download.side_effect = return_response
-        return crawler
-
-    def _get_middleware(self):
-        crawler = self._get_crawler()
-        return RobotsTxtMiddleware(crawler)