Repository: scrapy/scrapy
Pull request #4410 (2020-03-07 19:35:34)

Fix handling of None in allowed_domains
`allowed_domains = [None]` leads to the following exception:

```
2020-03-07 20:40:02 [scrapy.utils.signal] ERROR: Error caught on signal handler: <bound method OffsiteMiddleware.spider_opened of <scrapy.spidermiddlewares.offsite.OffsiteMiddleware object at 0x7f57c2a22d60>>
Traceback (most recent call last):
  File "/home/lukas/Projects/3rd/scrapy/scrapy/utils/defer.py", line 161, in maybeDeferred_coro
    result = f(*args, **kw)
  File "/home/lukas/.pyenv/versions/3.8.2/envs/scrapy/lib/python3.8/site-packages/pydispatch/robustapply.py", line 55, in robustApply
    return receiver(*arguments, **named)
  File "/home/lukas/Projects/3rd/scrapy/scrapy/spidermiddlewares/offsite.py", line 67, in spider_opened
    self.host_regex = self.get_host_regex(spider)
  File "/home/lukas/Projects/3rd/scrapy/scrapy/spidermiddlewares/offsite.py", line 58, in get_host_regex
    if url_pattern.match(domain):
TypeError: expected string or bytes-like object
```

However, Nones in allowed_domains ought to be ignored and there are also tests for that scenario. This commit fixes the handling of None and also the accompanying tests which are now executed again.


Patch:
diff --git a/scrapy/spidermiddlewares/offsite.py b/scrapy/spidermiddlewares/offsite.py
index 232e96cbb..36f809699 100644
--- a/scrapy/spidermiddlewares/offsite.py
+++ b/scrapy/spidermiddlewares/offsite.py
@@ -54,12 +54,16 @@ class OffsiteMiddleware(object):
         if not allowed_domains:
             return re.compile('')  # allow all by default
         url_pattern = re.compile("^https?://.*$")
+        domains = []
         for domain in allowed_domains:
-            if url_pattern.match(domain):
+            if domain is None:
+                continue
+            elif url_pattern.match(domain):
                 message = ("allowed_domains accepts only domains, not URLs. "
                            "Ignoring URL entry %s in allowed_domains." % domain)
                 warnings.warn(message, URLWarning)
-        domains = [re.escape(d) for d in allowed_domains if d is not None]
+            else:
+                domains.append(re.escape(domain))
         regex = r'^(.*\.)?(%s)$' % '|'.join(domains)
         return re.compile(regex)
 


Test patch:
diff --git a/tests/test_spidermiddleware_offsite.py b/tests/test_spidermiddleware_offsite.py
index 7511aa568..51c328943 100644
--- a/tests/test_spidermiddleware_offsite.py
+++ b/tests/test_spidermiddleware_offsite.py
@@ -55,21 +55,21 @@ class TestOffsiteMiddleware2(TestOffsiteMiddleware):
 
 class TestOffsiteMiddleware3(TestOffsiteMiddleware2):
 
-    def _get_spider(self):
-        return Spider('foo')
+    def _get_spiderargs(self):
+        return dict(name='foo')
 
 
 class TestOffsiteMiddleware4(TestOffsiteMiddleware3):
 
-    def _get_spider(self):
-      bad_hostname = urlparse('http:////scrapytest.org').hostname
-      return dict(name='foo', allowed_domains=['scrapytest.org', None, bad_hostname])
+    def _get_spiderargs(self):
+        bad_hostname = urlparse('http:////scrapytest.org').hostname
+        return dict(name='foo', allowed_domains=['scrapytest.org', None, bad_hostname])
 
     def test_process_spider_output(self):
-      res = Response('http://scrapytest.org')
-      reqs = [Request('http://scrapytest.org/1')]
-      out = list(self.mw.process_spider_output(res, reqs, self.spider))
-      self.assertEqual(out, reqs)
+        res = Response('http://scrapytest.org')
+        reqs = [Request('http://scrapytest.org/1')]
+        out = list(self.mw.process_spider_output(res, reqs, self.spider))
+        self.assertEqual(out, reqs)
 
 
 class TestOffsiteMiddleware5(TestOffsiteMiddleware4):