From e1ceaf3b5fa29326f032c4ed3f50943384b9e63d Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Mon, 13 Feb 2017 21:06:05 +0500 Subject: [PATCH 1/4] require w3lib 1.17+ --- requirements-py3.txt | 2 +- requirements.txt | 2 +- setup.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements-py3.txt b/requirements-py3.txt index 08ccf1958..cc0a7f644 100644 --- a/requirements-py3.txt +++ b/requirements-py3.txt @@ -3,5 +3,5 @@ lxml>=3.2.4 pyOpenSSL>=0.13.1 cssselect>=0.9 queuelib>=1.1.1 -w3lib>=1.14.2 +w3lib>=1.17.0 service_identity diff --git a/requirements.txt b/requirements.txt index f92603d3d..392f83dd6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ Twisted>=13.1.0 lxml pyOpenSSL cssselect>=0.9 -w3lib>=1.15.0 +w3lib>=1.17.0 queuelib six>=1.5.2 PyDispatcher>=2.0.5 diff --git a/setup.py b/setup.py index a6e6f9615..086ab8142 100644 --- a/setup.py +++ b/setup.py @@ -43,7 +43,7 @@ setup( ], install_requires=[ 'Twisted>=13.1.0', - 'w3lib>=1.15.0', + 'w3lib>=1.17.0', 'queuelib', 'lxml', 'pyOpenSSL', From ad36a4a6ae8376a779f9feb08adfb2ca4a59dbb4 Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Wed, 8 Feb 2017 16:58:38 +0500 Subject: [PATCH 2/4] RegexLinkExtractor: add \x0c to whitespace characters, as per html5 standard --- scrapy/linkextractors/regex.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scrapy/linkextractors/regex.py b/scrapy/linkextractors/regex.py index 0fc7b079f..e689b4727 100644 --- a/scrapy/linkextractors/regex.py +++ b/scrapy/linkextractors/regex.py @@ -10,9 +10,10 @@ linkre = re.compile( "|\s.*?>)(.*?)<[/ ]?a>", re.DOTALL | re.IGNORECASE) + def clean_link(link_text): """Remove leading and trailing whitespace and punctuation""" - return link_text.strip("\t\r\n '\"") + return link_text.strip("\t\r\n '\"\x0c") class RegexLinkExtractor(SgmlLinkExtractor): From d079e15fe2fcf269d040ac435e2eb414d2b4c334 Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Wed, 8 Feb 2017 17:03:11 +0500 Subject: [PATCH 3/4] 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() From d09eed7674b5df2a1883a7c7abad40fdfd062c74 Mon Sep 17 00:00:00 2001 From: Mikhail Korobov Date: Wed, 8 Feb 2017 23:44:55 +0500 Subject: [PATCH 4/4] use w3lib.html.strip_html5_whitespace function; expand docs; strip consistently before calling process_value --- docs/topics/link-extractors.rst | 9 +++++---- scrapy/linkextractors/htmlparser.py | 6 +++--- scrapy/linkextractors/lxmlhtml.py | 6 +++--- scrapy/linkextractors/sgml.py | 7 ++++--- scrapy/utils/url.py | 13 ------------- 5 files changed, 15 insertions(+), 26 deletions(-) diff --git a/docs/topics/link-extractors.rst b/docs/topics/link-extractors.rst index 2486e0982..75bdb4142 100644 --- a/docs/topics/link-extractors.rst +++ b/docs/topics/link-extractors.rst @@ -134,10 +134,11 @@ LxmlLinkExtractor :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). + must be stripped from ``href`` attributes of ````, ```` + and many other elements, ``src`` attribute of ````, ``