From 62a626102877de4998538717f34e61d2f7d2622c Mon Sep 17 00:00:00 2001 From: Jana Cavojska Date: Sat, 18 Nov 2017 20:03:59 +0100 Subject: [PATCH 1/5] Issues a warning when user puts a URL into allowed_domains (#2250) --- scrapy/spidermiddlewares/offsite.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scrapy/spidermiddlewares/offsite.py b/scrapy/spidermiddlewares/offsite.py index ea1c9270f..f51b0a2b0 100644 --- a/scrapy/spidermiddlewares/offsite.py +++ b/scrapy/spidermiddlewares/offsite.py @@ -52,6 +52,10 @@ class OffsiteMiddleware(object): allowed_domains = getattr(spider, 'allowed_domains', None) if not allowed_domains: return re.compile('') # allow all by default + for domainIndex in range(0, len(allowed_domains)): + url_pattern = re.compile("^https?://.*$") + if url_pattern.match(allowed_domains[domainIndex]): + logger.warn("allowed_domains accepts only domains, not URLs. Ignoring URL entry %s in allowed_domains." % allowed_domains[domainIndex]) regex = r'^(.*\.)?(%s)$' % '|'.join(re.escape(d) for d in allowed_domains if d is not None) return re.compile(regex) From 91ff194d1e9477d2196817ea1dc8beb220c3e058 Mon Sep 17 00:00:00 2001 From: Jana Cavojska Date: Mon, 20 Nov 2017 21:23:31 +0100 Subject: [PATCH 2/5] looping over allowed_domains directly instead of via index --- scrapy/spidermiddlewares/offsite.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scrapy/spidermiddlewares/offsite.py b/scrapy/spidermiddlewares/offsite.py index f51b0a2b0..8ff35e29f 100644 --- a/scrapy/spidermiddlewares/offsite.py +++ b/scrapy/spidermiddlewares/offsite.py @@ -52,10 +52,10 @@ class OffsiteMiddleware(object): allowed_domains = getattr(spider, 'allowed_domains', None) if not allowed_domains: return re.compile('') # allow all by default - for domainIndex in range(0, len(allowed_domains)): + for domain in allowed_domains: url_pattern = re.compile("^https?://.*$") - if url_pattern.match(allowed_domains[domainIndex]): - logger.warn("allowed_domains accepts only domains, not URLs. Ignoring URL entry %s in allowed_domains." % allowed_domains[domainIndex]) + if url_pattern.match(domain): + logger.warn("allowed_domains accepts only domains, not URLs. Ignoring URL entry %s in allowed_domains." % domain) regex = r'^(.*\.)?(%s)$' % '|'.join(re.escape(d) for d in allowed_domains if d is not None) return re.compile(regex) From 8ec3b476b03d6b8424f6dfc556758392e7a5a61f Mon Sep 17 00:00:00 2001 From: Jana Cavojska Date: Sun, 26 Nov 2017 16:36:15 +0100 Subject: [PATCH 3/5] triggering a warning when user puts URL in allowed_domains now covered by test --- scrapy/spidermiddlewares/offsite.py | 3 ++- tests/test_spidermiddleware_offsite.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/scrapy/spidermiddlewares/offsite.py b/scrapy/spidermiddlewares/offsite.py index 8ff35e29f..647792e5d 100644 --- a/scrapy/spidermiddlewares/offsite.py +++ b/scrapy/spidermiddlewares/offsite.py @@ -6,6 +6,7 @@ See documentation in docs/topics/spider-middleware.rst import re import logging +import warnings from scrapy import signals from scrapy.http import Request @@ -55,7 +56,7 @@ class OffsiteMiddleware(object): for domain in allowed_domains: url_pattern = re.compile("^https?://.*$") if url_pattern.match(domain): - logger.warn("allowed_domains accepts only domains, not URLs. Ignoring URL entry %s in allowed_domains." % domain) + warnings.warn("allowed_domains accepts only domains, not URLs. Ignoring URL entry %s in allowed_domains." % domain, Warning) regex = r'^(.*\.)?(%s)$' % '|'.join(re.escape(d) for d in allowed_domains if d is not None) return re.compile(regex) diff --git a/tests/test_spidermiddleware_offsite.py b/tests/test_spidermiddleware_offsite.py index 9ad86313c..b532cc2ec 100644 --- a/tests/test_spidermiddleware_offsite.py +++ b/tests/test_spidermiddleware_offsite.py @@ -6,6 +6,7 @@ from scrapy.http import Response, Request from scrapy.spiders import Spider from scrapy.spidermiddlewares.offsite import OffsiteMiddleware from scrapy.utils.test import get_crawler +import warnings class TestOffsiteMiddleware(TestCase): @@ -68,3 +69,13 @@ class TestOffsiteMiddleware4(TestOffsiteMiddleware3): reqs = [Request('http://scrapytest.org/1')] out = list(self.mw.process_spider_output(res, reqs, self.spider)) self.assertEqual(out, reqs) + + +class TestOffsiteMiddleware5(TestOffsiteMiddleware4): + + def test_get_host_regex(self): + self.spider.allowed_domains = ['http://scrapytest.org', 'scrapy.org', 'scrapy.test.org'] + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + self.mw.get_host_regex(self.spider) + assert "allowed_domains accepts only domains, not URLs." in str(w[-1].message) From 454d5e57333e9f33c8d684e4e21f8f7e9493f310 Mon Sep 17 00:00:00 2001 From: Jana Cavojska Date: Sun, 26 Nov 2017 20:07:04 +0100 Subject: [PATCH 4/5] checking for subclass of URLWarning instead of checking error message text when URL in allowed_domains --- scrapy/spidermiddlewares/offsite.py | 7 ++++++- tests/test_spidermiddleware_offsite.py | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/scrapy/spidermiddlewares/offsite.py b/scrapy/spidermiddlewares/offsite.py index 647792e5d..f595eef42 100644 --- a/scrapy/spidermiddlewares/offsite.py +++ b/scrapy/spidermiddlewares/offsite.py @@ -56,10 +56,15 @@ class OffsiteMiddleware(object): for domain in allowed_domains: url_pattern = re.compile("^https?://.*$") if url_pattern.match(domain): - warnings.warn("allowed_domains accepts only domains, not URLs. Ignoring URL entry %s in allowed_domains." % domain, Warning) + warnings.warn("allowed_domains accepts only domains, not URLs. Ignoring URL entry %s in allowed_domains." % domain, URLWarning) + regex = r'^(.*\.)?(%s)$' % '|'.join(re.escape(d) for d in allowed_domains if d is not None) return re.compile(regex) def spider_opened(self, spider): self.host_regex = self.get_host_regex(spider) self.domains_seen = set() + + +class URLWarning(Warning): + pass \ No newline at end of file diff --git a/tests/test_spidermiddleware_offsite.py b/tests/test_spidermiddleware_offsite.py index b532cc2ec..7e4af0d4c 100644 --- a/tests/test_spidermiddleware_offsite.py +++ b/tests/test_spidermiddleware_offsite.py @@ -5,6 +5,7 @@ from six.moves.urllib.parse import urlparse from scrapy.http import Response, Request from scrapy.spiders import Spider from scrapy.spidermiddlewares.offsite import OffsiteMiddleware +from scrapy.spidermiddlewares.offsite import URLWarning from scrapy.utils.test import get_crawler import warnings @@ -78,4 +79,4 @@ class TestOffsiteMiddleware5(TestOffsiteMiddleware4): with warnings.catch_warnings(record=True) as w: warnings.simplefilter("always") self.mw.get_host_regex(self.spider) - assert "allowed_domains accepts only domains, not URLs." in str(w[-1].message) + assert issubclass(w[-1].category, URLWarning) From 22c68baf990f15d249f38c481f24a984977be3e5 Mon Sep 17 00:00:00 2001 From: Jana Cavojska Date: Thu, 7 Dec 2017 18:38:29 +0100 Subject: [PATCH 5/5] url_pattern is now being compiled before entering the loop --- scrapy/spidermiddlewares/offsite.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scrapy/spidermiddlewares/offsite.py b/scrapy/spidermiddlewares/offsite.py index f595eef42..310166cad 100644 --- a/scrapy/spidermiddlewares/offsite.py +++ b/scrapy/spidermiddlewares/offsite.py @@ -53,8 +53,8 @@ class OffsiteMiddleware(object): allowed_domains = getattr(spider, 'allowed_domains', None) if not allowed_domains: return re.compile('') # allow all by default + url_pattern = re.compile("^https?://.*$") for domain in allowed_domains: - url_pattern = re.compile("^https?://.*$") if url_pattern.match(domain): warnings.warn("allowed_domains accepts only domains, not URLs. Ignoring URL entry %s in allowed_domains." % domain, URLWarning) @@ -67,4 +67,4 @@ class OffsiteMiddleware(object): class URLWarning(Warning): - pass \ No newline at end of file + pass