diff --git a/docs/topics/link-extractors.rst b/docs/topics/link-extractors.rst index 75bdb4142..01d7f0b97 100644 --- a/docs/topics/link-extractors.rst +++ b/docs/topics/link-extractors.rst @@ -51,7 +51,7 @@ LxmlLinkExtractor :synopsis: lxml's HTMLParser-based link extractors -.. class:: LxmlLinkExtractor(allow=(), deny=(), allow_domains=(), deny_domains=(), deny_extensions=None, restrict_xpaths=(), restrict_css=(), tags=('a', 'area'), attrs=('href',), canonicalize=True, unique=True, process_value=None, strip=True) +.. class:: LxmlLinkExtractor(allow=(), deny=(), allow_domains=(), deny_domains=(), deny_extensions=None, restrict_xpaths=(), restrict_css=(), tags=('a', 'area'), attrs=('href',), canonicalize=False, unique=True, process_value=None, strip=True) LxmlLinkExtractor is the recommended link extractor with handy filtering options. It is implemented using lxml's robust HTMLParser. @@ -103,7 +103,12 @@ LxmlLinkExtractor :type attrs: list :param canonicalize: canonicalize each extracted url (using - w3lib.url.canonicalize_url). Defaults to ``True``. + w3lib.url.canonicalize_url). Defaults to ``False``. + Note that canonicalize_url is meant for duplicate checking; + it can change the URL visible at server side, so the response can be + different for requests with canonicalized and raw URLs. If you're + using LinkExtractor to follow links it is more robust to + keep the default ``canonicalize=False``. :type canonicalize: boolean :param unique: whether duplicate filtering should be applied to extracted diff --git a/scrapy/linkextractors/__init__.py b/scrapy/linkextractors/__init__.py index e5d21e174..8676c3b92 100644 --- a/scrapy/linkextractors/__init__.py +++ b/scrapy/linkextractors/__init__.py @@ -101,7 +101,7 @@ class FilteringLinkExtractor(object): links = [x for x in links if self._link_allowed(x)] if self.canonicalize: for link in links: - link.url = canonicalize_url(urlparse(link.url)) + link.url = canonicalize_url(link.url) links = self.link_extractor._process_links(links) return links diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index c284f1905..a7092f9b8 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -6,6 +6,7 @@ from six.moves.urllib.parse import urljoin import lxml.etree as etree from w3lib.html import strip_html5_whitespace +from w3lib.url import canonicalize_url from scrapy.link import Link from scrapy.utils.misc import arg_to_iter, rel_has_nofollow @@ -29,12 +30,17 @@ def _nons(tag): class LxmlParserLinkExtractor(object): def __init__(self, tag="a", attr="href", process=None, unique=False, - strip=True): + strip=True, canonicalized=False): self.scan_tag = tag if callable(tag) else lambda t: t == tag self.scan_attr = attr if callable(attr) else lambda a: a == attr self.process_attr = process if callable(process) else lambda v: v self.unique = unique self.strip = strip + if canonicalized: + self.link_key = lambda link: link.url + else: + self.link_key = lambda link: canonicalize_url(link.url, + keep_fragments=True) def _iter_links(self, document): for el in document.iter(etree.Element): @@ -82,21 +88,27 @@ class LxmlParserLinkExtractor(object): def _deduplicate_if_needed(self, links): if self.unique: - return unique_list(links, key=lambda link: link.url) + return unique_list(links, key=self.link_key) return links class LxmlLinkExtractor(FilteringLinkExtractor): def __init__(self, allow=(), deny=(), allow_domains=(), deny_domains=(), restrict_xpaths=(), - tags=('a', 'area'), attrs=('href',), canonicalize=True, + tags=('a', 'area'), attrs=('href',), canonicalize=False, unique=True, process_value=None, deny_extensions=None, restrict_css=(), strip=True): tags, attrs = set(arg_to_iter(tags)), set(arg_to_iter(attrs)) tag_func = lambda x: x in tags attr_func = lambda x: x in attrs - lx = LxmlParserLinkExtractor(tag=tag_func, attr=attr_func, - unique=unique, process=process_value, strip=strip) + lx = LxmlParserLinkExtractor( + tag=tag_func, + attr=attr_func, + unique=unique, + process=process_value, + strip=strip, + canonicalized=canonicalize + ) super(LxmlLinkExtractor, self).__init__(lx, allow=allow, deny=deny, allow_domains=allow_domains, deny_domains=deny_domains, diff --git a/scrapy/linkextractors/sgml.py b/scrapy/linkextractors/sgml.py index 11ff7a261..f4ca4262a 100644 --- a/scrapy/linkextractors/sgml.py +++ b/scrapy/linkextractors/sgml.py @@ -6,7 +6,7 @@ from six.moves.urllib.parse import urljoin import warnings from sgmllib import SGMLParser -from w3lib.url import safe_url_string +from w3lib.url import safe_url_string, canonicalize_url from w3lib.html import strip_html5_whitespace from scrapy.link import Link @@ -20,7 +20,7 @@ from scrapy.exceptions import ScrapyDeprecationWarning class BaseSgmlLinkExtractor(SGMLParser): def __init__(self, tag="a", attr="href", unique=False, process_value=None, - strip=True): + strip=True, canonicalized=False): warnings.warn( "BaseSgmlLinkExtractor is deprecated and will be removed in future releases. " "Please use scrapy.linkextractors.LinkExtractor", @@ -33,6 +33,11 @@ class BaseSgmlLinkExtractor(SGMLParser): self.current_link = None self.unique = unique self.strip = strip + if canonicalized: + self.link_key = lambda link: link.url + else: + self.link_key = lambda link: canonicalize_url(link.url, + keep_fragments=True) def _extract_links(self, response_text, response_url, response_encoding, base_url=None): """ Do the real extraction work """ @@ -61,8 +66,7 @@ class BaseSgmlLinkExtractor(SGMLParser): The subclass should override it if necessary """ - links = unique_list(links, key=lambda link: link.url) if self.unique else links - return links + return unique_list(links, key=self.link_key) if self.unique else links def extract_links(self, response): # wrapper needed to allow to work directly with text @@ -107,10 +111,9 @@ class BaseSgmlLinkExtractor(SGMLParser): class SgmlLinkExtractor(FilteringLinkExtractor): def __init__(self, allow=(), deny=(), allow_domains=(), deny_domains=(), restrict_xpaths=(), - tags=('a', 'area'), attrs=('href',), canonicalize=True, unique=True, + tags=('a', 'area'), attrs=('href',), canonicalize=False, unique=True, process_value=None, deny_extensions=None, restrict_css=(), strip=True): - warnings.warn( "SgmlLinkExtractor is deprecated and will be removed in future releases. " "Please use scrapy.linkextractors.LinkExtractor", @@ -124,7 +127,8 @@ class SgmlLinkExtractor(FilteringLinkExtractor): with warnings.catch_warnings(): warnings.simplefilter('ignore', ScrapyDeprecationWarning) lx = BaseSgmlLinkExtractor(tag=tag_func, attr=attr_func, - unique=unique, process_value=process_value, strip=strip) + unique=unique, process_value=process_value, strip=strip, + canonicalized=canonicalize) super(SgmlLinkExtractor, self).__init__(lx, allow=allow, deny=deny, allow_domains=allow_domains, deny_domains=deny_domains, diff --git a/tests/sample_data/link_extractor/sgml_linkextractor.html b/tests/sample_data/link_extractor/sgml_linkextractor.html index fbb803f2d..7d5db368a 100644 --- a/tests/sample_data/link_extractor/sgml_linkextractor.html +++ b/tests/sample_data/link_extractor/sgml_linkextractor.html @@ -11,6 +11,7 @@ sample 3 text sample 3 repetition +sample 3 repetition with fragment inner tag href with whitespaces diff --git a/tests/test_http_response.py b/tests/test_http_response.py index 924bb7979..779f5a71c 100644 --- a/tests/test_http_response.py +++ b/tests/test_http_response.py @@ -392,6 +392,7 @@ class TextResponseTest(BaseResponseTest): 'http://example.com/sample2.html', 'http://example.com/sample3.html', 'http://example.com/sample3.html', + 'http://example.com/sample3.html#foo', 'http://www.google.com/something', 'http://example.com/innertag.html' ] diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 340c64f35..1d7c4f311 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -13,6 +13,7 @@ from tests import get_testdata class Base: class LinkExtractorTestCase(unittest.TestCase): extractor_cls = None + escapes_whitespace = False def setUp(self): body = get_testdata('link_extractor', 'sgml_linkextractor.html') @@ -26,13 +27,19 @@ class Base: def test_extract_all_links(self): lx = self.extractor_cls() + if self.escapes_whitespace: + page4_url = 'http://example.com/page%204.html' + else: + page4_url = 'http://example.com/page 4.html' + self.assertEqual([link for link in lx.extract_links(self.response)], [ Link(url='http://example.com/sample1.html', text=u''), Link(url='http://example.com/sample2.html', text=u'sample 2'), Link(url='http://example.com/sample3.html', text=u'sample 3 text'), + Link(url='http://example.com/sample3.html#foo', text='sample 3 repetition with fragment'), Link(url='http://www.google.com/something', text=u''), Link(url='http://example.com/innertag.html', text=u'inner tag'), - Link(url='http://example.com/page%204.html', text=u'href with whitespaces'), + Link(url=page4_url, text=u'href with whitespaces'), ]) def test_extract_filter_allow(self): @@ -41,6 +48,7 @@ class Base: Link(url='http://example.com/sample1.html', text=u''), Link(url='http://example.com/sample2.html', text=u'sample 2'), Link(url='http://example.com/sample3.html', text=u'sample 3 text'), + Link(url='http://example.com/sample3.html#foo', text='sample 3 repetition with fragment') ]) def test_extract_filter_allow_with_duplicates(self): @@ -50,6 +58,27 @@ class Base: Link(url='http://example.com/sample2.html', text=u'sample 2'), Link(url='http://example.com/sample3.html', text=u'sample 3 text'), Link(url='http://example.com/sample3.html', text=u'sample 3 repetition'), + Link(url='http://example.com/sample3.html#foo', text='sample 3 repetition with fragment') + ]) + + def test_extract_filter_allow_with_duplicates_canonicalize(self): + lx = self.extractor_cls(allow=('sample', ), unique=False, + canonicalize=True) + self.assertEqual([link for link in lx.extract_links(self.response)], [ + Link(url='http://example.com/sample1.html', text=u''), + Link(url='http://example.com/sample2.html', text=u'sample 2'), + Link(url='http://example.com/sample3.html', text=u'sample 3 text'), + Link(url='http://example.com/sample3.html', text=u'sample 3 repetition'), + Link(url='http://example.com/sample3.html', text='sample 3 repetition with fragment') + ]) + + def test_extract_filter_allow_no_duplicates_canonicalize(self): + lx = self.extractor_cls(allow=('sample',), unique=True, + canonicalize=True) + self.assertEqual([link for link in lx.extract_links(self.response)], [ + Link(url='http://example.com/sample1.html', text=u''), + Link(url='http://example.com/sample2.html', text=u'sample 2'), + Link(url='http://example.com/sample3.html', text=u'sample 3 text'), ]) def test_extract_filter_allow_and_deny(self): @@ -73,6 +102,8 @@ class Base: Link(url='http://example.com/sample1.html', text=u''), Link(url='http://example.com/sample2.html', text=u'sample 2'), Link(url='http://example.com/sample3.html', text=u'sample 3 text'), + Link(url='http://example.com/sample3.html#foo', + text='sample 3 repetition with fragment') ]) lx = self.extractor_cls(allow='sample', deny='3') @@ -276,13 +307,19 @@ class Base: def test_attrs(self): lx = self.extractor_cls(attrs="href") + if self.escapes_whitespace: + page4_url = 'http://example.com/page%204.html' + else: + page4_url = 'http://example.com/page 4.html' + self.assertEqual(lx.extract_links(self.response), [ Link(url='http://example.com/sample1.html', text=u''), Link(url='http://example.com/sample2.html', text=u'sample 2'), Link(url='http://example.com/sample3.html', text=u'sample 3 text'), + Link(url='http://example.com/sample3.html#foo', text='sample 3 repetition with fragment'), Link(url='http://www.google.com/something', text=u''), Link(url='http://example.com/innertag.html', text=u'inner tag'), - Link(url='http://example.com/page%204.html', text=u'href with whitespaces'), + Link(url=page4_url, text=u'href with whitespaces'), ]) lx = self.extractor_cls(attrs=("href","src"), tags=("a","area","img"), deny_extensions=()) @@ -291,9 +328,10 @@ class Base: Link(url='http://example.com/sample2.html', text=u'sample 2'), Link(url='http://example.com/sample2.jpg', text=u''), Link(url='http://example.com/sample3.html', text=u'sample 3 text'), + Link(url='http://example.com/sample3.html#foo', text='sample 3 repetition with fragment'), Link(url='http://www.google.com/something', text=u''), Link(url='http://example.com/innertag.html', text=u'inner tag'), - Link(url='http://example.com/page%204.html', text=u'href with whitespaces'), + Link(url=page4_url, text=u'href with whitespaces'), ]) lx = self.extractor_cls(attrs=None) diff --git a/tests/test_linkextractors_deprecated.py b/tests/test_linkextractors_deprecated.py index fef227aa1..1366971be 100644 --- a/tests/test_linkextractors_deprecated.py +++ b/tests/test_linkextractors_deprecated.py @@ -121,6 +121,7 @@ class HtmlParserLinkExtractorTestCase(unittest.TestCase): Link(url='http://example.com/sample2.html', text=u'sample 2'), Link(url='http://example.com/sample3.html', text=u'sample 3 text'), Link(url='http://example.com/sample3.html', text=u'sample 3 repetition'), + Link(url='http://example.com/sample3.html#foo', text=u'sample 3 repetition with fragment'), Link(url='http://www.google.com/something', text=u''), Link(url='http://example.com/innertag.html', text=u'inner tag'), Link(url='http://example.com/page%204.html', text=u'href with whitespaces'), @@ -142,6 +143,7 @@ class HtmlParserLinkExtractorTestCase(unittest.TestCase): class SgmlLinkExtractorTestCase(Base.LinkExtractorTestCase): extractor_cls = SgmlLinkExtractor + escapes_whitespace = True def test_deny_extensions(self): html = """asd and """ @@ -190,6 +192,7 @@ class RegexLinkExtractorTestCase(unittest.TestCase): self.assertEqual(lx.extract_links(self.response), [Link(url='http://example.com/sample2.html', text=u'sample 2'), Link(url='http://example.com/sample3.html', text=u'sample 3 text'), + Link(url='http://example.com/sample3.html#foo', text=u'sample 3 repetition with fragment'), Link(url='http://www.google.com/something', text=u''), Link(url='http://example.com/innertag.html', text=u'inner tag'),])