From 90ce6589eee58e8aca9c368a71907b30250df68d Mon Sep 17 00:00:00 2001 From: Samuel Bartlett Date: Thu, 30 Mar 2023 13:07:51 +0000 Subject: [PATCH 01/10] Add try/except to safe_url_string() Added a try catch condition to the safe_url_string() processing in the LxmlParserLinkExtractor class to avoid scrapers crashing unneccessarily --- scrapy/linkextractors/lxmlhtml.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index dd8dcdf7c..1ee81427c 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -88,7 +88,11 @@ class LxmlParserLinkExtractor: url = self.process_attr(attr_val) if url is None: continue - url = safe_url_string(url, encoding=response_encoding) + try: + url = safe_url_string(url, encoding=response_encoding) + except ValueError: + continue # Disregard badly formatted urls + # to fix relative links after process_value url = urljoin(response_url, url) link = Link( From 9ef00c5c0bc16c9b44c878952a2b54310dc25638 Mon Sep 17 00:00:00 2001 From: Samuel Bartlett Date: Fri, 31 Mar 2023 08:01:54 +0000 Subject: [PATCH 02/10] Add logging Lines Adds an error loggign line to the LinkExtractor to detail encountered bad links --- scrapy/linkextractors/lxmlhtml.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index 1ee81427c..f7c6937b0 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -2,6 +2,7 @@ Link extractor based on lxml.html """ import operator +import logging from functools import partial from urllib.parse import urljoin, urlparse @@ -23,6 +24,8 @@ from scrapy.utils.python import unique as unique_list from scrapy.utils.response import get_base_url from scrapy.utils.url import url_has_any_extension, url_is_from_any_domain +logger = logging.getLogger(__name__) + # from lxml/src/lxml/html/__init__.py XHTML_NAMESPACE = "http://www.w3.org/1999/xhtml" @@ -91,6 +94,7 @@ class LxmlParserLinkExtractor: try: url = safe_url_string(url, encoding=response_encoding) except ValueError: + logger.error(f"Skipping extraction of bad link {url}") continue # Disregard badly formatted urls # to fix relative links after process_value From 9cbcf7724df7de9a449659fbf68bc5d532c33499 Mon Sep 17 00:00:00 2001 From: Samuel Bartlett Date: Fri, 31 Mar 2023 08:07:43 +0000 Subject: [PATCH 03/10] Add test to make sure spider doesn't crash on bad --- tests/test_linkextractors.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index f663013ba..d992a5eae 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -815,3 +815,26 @@ class LxmlLinkExtractorTestCase(Base.LinkExtractorTestCase): def test_restrict_xpaths_with_html_entities(self): super().test_restrict_xpaths_with_html_entities() + + def test_skip_bad_links(self): + html = b""" + Why would you do this? + Good Link + Good Link 2 + """ + response = HtmlResponse("http://example.org/index.html", body=html) + self.assertEqual( + [link for link in lx.extract_links(response)], + [ + Link( + url="http://example.org/item2.html", + text="Good Link", + nofollow=False, + ), + Link( + url="http://example.org/item3.html", + text="Good Link 2", + nofollow=False, + ), + ], + ) From 7cb7cf1ad1aa3d75b494a5b069e5b76b60328daa Mon Sep 17 00:00:00 2001 From: Samuel Bartlett Date: Fri, 31 Mar 2023 08:09:02 +0000 Subject: [PATCH 04/10] Add link extractor back to test --- tests/test_linkextractors.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index d992a5eae..3ad1abea5 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -823,6 +823,7 @@ class LxmlLinkExtractorTestCase(Base.LinkExtractorTestCase): Good Link 2 """ response = HtmlResponse("http://example.org/index.html", body=html) + lx = self.extractor_cls() self.assertEqual( [link for link in lx.extract_links(response)], [ From 00d93026c8b078d75e4fe43c344f6524f9b45f28 Mon Sep 17 00:00:00 2001 From: Samuel Bartlett Date: Fri, 31 Mar 2023 08:30:19 +0000 Subject: [PATCH 05/10] Fix bad test case --- tests/test_linkextractors.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 3ad1abea5..1ea364d80 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -818,11 +818,11 @@ class LxmlLinkExtractorTestCase(Base.LinkExtractorTestCase): def test_skip_bad_links(self): html = b""" - Why would you do this? + Why would you do this? Good Link Good Link 2 """ - response = HtmlResponse("http://example.org/index.html", body=html) + response = HtmlResponse("http://example.org/index.html", body=html, encoding='utf-8') lx = self.extractor_cls() self.assertEqual( [link for link in lx.extract_links(response)], From 4043560547faac0ee4cfadba4d0f02b4be1f72de Mon Sep 17 00:00:00 2001 From: Samuel Bartlett Date: Fri, 31 Mar 2023 12:29:22 +0000 Subject: [PATCH 06/10] remove utf-8 encoding flag from test --- tests/test_linkextractors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 1ea364d80..3673e82cd 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -822,7 +822,7 @@ class LxmlLinkExtractorTestCase(Base.LinkExtractorTestCase): Good Link Good Link 2 """ - response = HtmlResponse("http://example.org/index.html", body=html, encoding='utf-8') + response = HtmlResponse("http://example.org/index.html", body=html) lx = self.extractor_cls() self.assertEqual( [link for link in lx.extract_links(response)], From c9a5934494cbb3fe0aa572b536ee222a3e212487 Mon Sep 17 00:00:00 2001 From: Samuel Bartlett Date: Fri, 31 Mar 2023 12:29:49 +0000 Subject: [PATCH 07/10] Reduce logging level of bad URL --- scrapy/linkextractors/lxmlhtml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index f7c6937b0..0d1b76531 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -94,7 +94,7 @@ class LxmlParserLinkExtractor: try: url = safe_url_string(url, encoding=response_encoding) except ValueError: - logger.error(f"Skipping extraction of bad link {url}") + logger.debug(f"Skipping extraction of bad link {url}") continue # Disregard badly formatted urls # to fix relative links after process_value From 608b7de582af891a37a3fab60423c847af648db8 Mon Sep 17 00:00:00 2001 From: Samuel Bartlett Date: Fri, 31 Mar 2023 14:38:06 +0000 Subject: [PATCH 08/10] Skip new test if python version less than 3.8 --- tests/test_linkextractors.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 3673e82cd..78219f642 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -1,6 +1,7 @@ import pickle import re import unittest +import sys from scrapy.http import HtmlResponse, XmlResponse from scrapy.link import Link @@ -816,6 +817,10 @@ class LxmlLinkExtractorTestCase(Base.LinkExtractorTestCase): def test_restrict_xpaths_with_html_entities(self): super().test_restrict_xpaths_with_html_entities() + @unittest.skipIf( + sys.version_info < (3, 8), + reason="Urllib3 is less strict in versions for python 3.7 so does not cause spider to crash", + ) def test_skip_bad_links(self): html = b""" Why would you do this? From 618e82dbe104c4b97cc4f7b37bf9130a76093734 Mon Sep 17 00:00:00 2001 From: Samuel Bartlett Date: Fri, 31 Mar 2023 15:12:47 +0000 Subject: [PATCH 09/10] Exclude test for python versionbs less than 3.8 --- tests/test_linkextractors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 78219f642..784fdb658 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -819,7 +819,7 @@ class LxmlLinkExtractorTestCase(Base.LinkExtractorTestCase): @unittest.skipIf( sys.version_info < (3, 8), - reason="Urllib3 is less strict in versions for python 3.7 so does not cause spider to crash", + reason="some library for python 3.7 so is less strict so bad links like htis don't crash scrapy", ) def test_skip_bad_links(self): html = b""" From 3f0c2fae5e18c448bd1791920500c976d44fc321 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 12 Apr 2023 09:28:28 +0200 Subject: [PATCH 10/10] Skip test_skip_bad_links based on the w3lib version --- scrapy/linkextractors/lxmlhtml.py | 6 +++--- tests/test_linkextractors.py | 16 +++++++++++----- tox.ini | 1 + 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/scrapy/linkextractors/lxmlhtml.py b/scrapy/linkextractors/lxmlhtml.py index 0d1b76531..23cbd0116 100644 --- a/scrapy/linkextractors/lxmlhtml.py +++ b/scrapy/linkextractors/lxmlhtml.py @@ -1,8 +1,8 @@ """ Link extractor based on lxml.html """ -import operator import logging +import operator from functools import partial from urllib.parse import urljoin, urlparse @@ -94,8 +94,8 @@ class LxmlParserLinkExtractor: try: url = safe_url_string(url, encoding=response_encoding) except ValueError: - logger.debug(f"Skipping extraction of bad link {url}") - continue # Disregard badly formatted urls + logger.debug(f"Skipping extraction of link with bad URL {url!r}") + continue # to fix relative links after process_value url = urljoin(response_url, url) diff --git a/tests/test_linkextractors.py b/tests/test_linkextractors.py index 784fdb658..e1ec19601 100644 --- a/tests/test_linkextractors.py +++ b/tests/test_linkextractors.py @@ -1,7 +1,10 @@ import pickle import re import unittest -import sys + +from packaging.version import Version +from pytest import mark +from w3lib import __version__ as w3lib_version from scrapy.http import HtmlResponse, XmlResponse from scrapy.link import Link @@ -817,13 +820,16 @@ class LxmlLinkExtractorTestCase(Base.LinkExtractorTestCase): def test_restrict_xpaths_with_html_entities(self): super().test_restrict_xpaths_with_html_entities() - @unittest.skipIf( - sys.version_info < (3, 8), - reason="some library for python 3.7 so is less strict so bad links like htis don't crash scrapy", + @mark.skipif( + Version(w3lib_version) < Version("2.0.0"), + reason=( + "Before w3lib 2.0.0, w3lib.url.safe_url_string would not complain " + "about an invalid port value." + ), ) def test_skip_bad_links(self): html = b""" - Why would you do this? + Why would you do this? Good Link Good Link 2 """ diff --git a/tox.ini b/tox.ini index 5a9d9cf29..873e7662b 100644 --- a/tox.ini +++ b/tox.ini @@ -101,6 +101,7 @@ install_command = python -I -m pip install {opts} {packages} [testenv:pinned] +basepython = python3.7 deps = {[pinned]deps} PyDispatcher==2.0.5