From d079e15fe2fcf269d040ac435e2eb414d2b4c334 Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Wed, 8 Feb 2017 17:03:11 +0500 Subject: [PATCH] Strip leading/trailing whitespaces in link extractors. Fixes GH-838. --- docs/topics/link-extractors.rst | 10 +++++++++- scrapy/linkextractors/htmlparser.py | 8 ++++++-- scrapy/linkextractors/lxmlhtml.py | 16 ++++++++++----- scrapy/linkextractors/sgml.py | 13 ++++++++---- scrapy/utils/url.py | 13 ++++++++++++ .../link_extractor/sgml_linkextractor.html | 1 + tests/test_linkextractors.py | 3 +++ tests/test_linkextractors_deprecated.py | 20 +++++++++++++------ 8 files changed, 66 insertions(+), 18 deletions(-) diff --git a/docs/topics/link-extractors.rst b/docs/topics/link-extractors.rst index 4636ddb18..2486e0982 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) +.. 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) LxmlLinkExtractor is the recommended link extractor with handy filtering options. It is implemented using lxml's robust HTMLParser. @@ -132,4 +132,12 @@ LxmlLinkExtractor :type process_value: callable + :param strip: whether to strip whitespaces from extracted attributes. + According to HTML5 standard, leading and trailing whitespaces + must be stripped from ``href`` attributes of ```` and ```` + elements, so LinkExtractor strips them by default. Set ``strip=False`` + to turn it off (e.g. if you're extracting urls from elements or + attributes which allow leading/trailing whitespaces). + :type strip: boolean + .. _scrapy.linkextractors: https://github.com/scrapy/scrapy/blob/master/scrapy/linkextractors/__init__.py diff --git a/scrapy/linkextractors/htmlparser.py b/scrapy/linkextractors/htmlparser.py index 9867e1179..4841e4a54 100644 --- a/scrapy/linkextractors/htmlparser.py +++ b/scrapy/linkextractors/htmlparser.py @@ -1,7 +1,6 @@ """ HTMLParser-based link extractor """ - import warnings import six from six.moves.html_parser import HTMLParser @@ -11,12 +10,14 @@ from w3lib.url import safe_url_string from scrapy.link import Link from scrapy.utils.python import unique as unique_list +from scrapy.utils.url import trim_href_attribute from scrapy.exceptions import ScrapyDeprecationWarning class HtmlParserLinkExtractor(HTMLParser): - def __init__(self, tag="a", attr="href", process=None, unique=False): + def __init__(self, tag="a", attr="href", process=None, unique=False, + strip=True): HTMLParser.__init__(self) warnings.warn( @@ -29,6 +30,7 @@ class HtmlParserLinkExtractor(HTMLParser): 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 def _extract_links(self, response_text, response_url, response_encoding): self.reset() @@ -70,6 +72,8 @@ class HtmlParserLinkExtractor(HTMLParser): for attr, value in attrs: if self.scan_attr(attr): url = self.process_attr(value) + if self.strip: + url = trim_href_attribute(url) link = Link(url=url) self.links.append(link) self.current_link = link diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index 71d57b392..f753033ab 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -9,8 +9,9 @@ import lxml.etree as etree from scrapy.link import Link from scrapy.utils.misc import arg_to_iter, rel_has_nofollow from scrapy.utils.python import unique as unique_list, to_native_str -from scrapy.linkextractors import FilteringLinkExtractor from scrapy.utils.response import get_base_url +from scrapy.utils.url import trim_href_attribute +from scrapy.linkextractors import FilteringLinkExtractor # from lxml/src/lxml/html/__init__.py @@ -27,11 +28,13 @@ def _nons(tag): class LxmlParserLinkExtractor(object): - def __init__(self, tag="a", attr="href", process=None, unique=False): + def __init__(self, tag="a", attr="href", process=None, unique=False, + strip=True): 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 def _iter_links(self, document): for el in document.iter(etree.Element): @@ -49,9 +52,11 @@ class LxmlParserLinkExtractor(object): for el, attr, attr_val in self._iter_links(selector.root): # pseudo lxml.html.HtmlElement.make_links_absolute(base_url) try: + if self.strip: + attr_val = trim_href_attribute(attr_val) attr_val = urljoin(base_url, attr_val) except ValueError: - continue # skipping bogus links + continue # skipping bogus links else: url = self.process_attr(attr_val) if url is None: @@ -85,12 +90,13 @@ class LxmlLinkExtractor(FilteringLinkExtractor): def __init__(self, allow=(), deny=(), allow_domains=(), deny_domains=(), restrict_xpaths=(), tags=('a', 'area'), attrs=('href',), canonicalize=True, - unique=True, process_value=None, deny_extensions=None, restrict_css=()): + 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) + unique=unique, process=process_value, strip=strip) 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 c68dae4c8..6ecfd52aa 100644 --- a/scrapy/linkextractors/sgml.py +++ b/scrapy/linkextractors/sgml.py @@ -7,18 +7,19 @@ import warnings from sgmllib import SGMLParser from w3lib.url import safe_url_string -from scrapy.selector import Selector from scrapy.link import Link from scrapy.linkextractors import FilteringLinkExtractor from scrapy.utils.misc import arg_to_iter, rel_has_nofollow from scrapy.utils.python import unique as unique_list, to_unicode from scrapy.utils.response import get_base_url +from scrapy.utils.url import trim_href_attribute from scrapy.exceptions import ScrapyDeprecationWarning class BaseSgmlLinkExtractor(SGMLParser): - def __init__(self, tag="a", attr="href", unique=False, process_value=None): + def __init__(self, tag="a", attr="href", unique=False, process_value=None, + strip=True): warnings.warn( "BaseSgmlLinkExtractor is deprecated and will be removed in future releases. " "Please use scrapy.linkextractors.LinkExtractor", @@ -30,6 +31,7 @@ class BaseSgmlLinkExtractor(SGMLParser): self.process_value = (lambda v: v) if process_value is None else process_value self.current_link = None self.unique = unique + self.strip = strip def _extract_links(self, response_text, response_url, response_encoding, base_url=None): """ Do the real extraction work """ @@ -81,6 +83,8 @@ class BaseSgmlLinkExtractor(SGMLParser): if self.scan_attr(attr): url = self.process_value(value) if url is not None: + if self.strip: + url = trim_href_attribute(url) link = Link(url=url, nofollow=rel_has_nofollow(dict(attrs).get('rel'))) self.links.append(link) self.current_link = link @@ -103,7 +107,8 @@ class SgmlLinkExtractor(FilteringLinkExtractor): def __init__(self, allow=(), deny=(), allow_domains=(), deny_domains=(), restrict_xpaths=(), tags=('a', 'area'), attrs=('href',), canonicalize=True, unique=True, - process_value=None, deny_extensions=None, restrict_css=()): + process_value=None, deny_extensions=None, restrict_css=(), + strip=True): warnings.warn( "SgmlLinkExtractor is deprecated and will be removed in future releases. " @@ -118,7 +123,7 @@ 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) + unique=unique, process_value=process_value, strip=strip) super(SgmlLinkExtractor, self).__init__(lx, allow=allow, deny=deny, allow_domains=allow_domains, deny_domains=deny_domains, diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index dc1cce4ac..090f65f80 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -103,3 +103,16 @@ def guess_scheme(url): return any_to_uri(url) else: return add_http_if_no_scheme(url) + + +def trim_href_attribute(href): + """ + Process href attribute of ``a`` or ``area`` elements according to HTML5 + standards (strip all leading and trailing whitespaces). References: + + * https://www.w3.org/TR/html5/links.html#links-created-by-a-and-area-elements + * https://www.w3.org/TR/html5/infrastructure.html#valid-url-potentially-surrounded-by-spaces + * https://www.w3.org/TR/html5/infrastructure.html#strip-leading-and-trailing-whitespace + * https://www.w3.org/TR/html5/infrastructure.html#space-character + """ + return href.strip(' \t\n\r\x0c') diff --git a/tests/sample_data/link_extractor/sgml_linkextractor.html b/tests/sample_data/link_extractor/sgml_linkextractor.html index 35aa457ee..fbb803f2d 100644 --- a/tests/sample_data/link_extractor/sgml_linkextractor.html +++ b/tests/sample_data/link_extractor/sgml_linkextractor.html @@ -13,6 +13,7 @@ sample 3 repetition inner tag +href with whitespaces diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 129336d14..340c64f35 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -32,6 +32,7 @@ class Base: Link(url='http://example.com/sample3.html', text=u'sample 3 text'), 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'), ]) def test_extract_filter_allow(self): @@ -281,6 +282,7 @@ class Base: Link(url='http://example.com/sample3.html', text=u'sample 3 text'), 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'), ]) lx = self.extractor_cls(attrs=("href","src"), tags=("a","area","img"), deny_extensions=()) @@ -291,6 +293,7 @@ class Base: Link(url='http://example.com/sample3.html', text=u'sample 3 text'), 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'), ]) lx = self.extractor_cls(attrs=None) diff --git a/tests/test_linkextractors_deprecated.py b/tests/test_linkextractors_deprecated.py index 36dfe174f..fef227aa1 100644 --- a/tests/test_linkextractors_deprecated.py +++ b/tests/test_linkextractors_deprecated.py @@ -117,12 +117,14 @@ class HtmlParserLinkExtractorTestCase(unittest.TestCase): def test_extraction(self): # Default arguments lx = HtmlParserLinkExtractor() - 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', text=u'sample 3 repetition'), - Link(url='http://www.google.com/something', text=u''), - Link(url='http://example.com/innertag.html', text=u'inner tag'),]) + 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', text=u'sample 3 repetition'), + 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'), + ]) def test_link_wrong_href(self): html = """ @@ -220,3 +222,9 @@ class RegexLinkExtractorTestCase(unittest.TestCase): self.assertEqual([link for link in lx.extract_links(response)], [ Link(url='http://b.com/test.html', text=u'', nofollow=False), ]) + + @unittest.expectedFailure + def test_extraction(self): + # RegexLinkExtractor doesn't parse URLs with leading/trailing + # whitespaces correctly. + super(RegexLinkExtractorTestCase, self).test_extraction()