diff --git a/docs/faq.rst b/docs/faq.rst index 9733471bf..8ec501a1a 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -280,9 +280,13 @@ build the DOM of the entire feed in memory, and this can be quite slow and consume a lot of memory. In order to avoid parsing all the entire feed at once in memory, you can use -the functions ``xmliter`` and ``csviter`` from ``scrapy.utils.iterators`` -module. In fact, this is what the feed spiders (see :ref:`topics-spiders`) use -under the cover. +the :func:`~scrapy.utils.iterators.xmliter_lxml` and +:func:`~scrapy.utils.iterators.csviter` functions. In fact, this is what +:class:`~scrapy.spiders.XMLFeedSpider` uses. + +.. autofunction:: scrapy.utils.iterators.xmliter_lxml + +.. autofunction:: scrapy.utils.iterators.csviter Does Scrapy manage cookies automatically? ----------------------------------------- diff --git a/docs/news.rst b/docs/news.rst index aba35aa46..2bdd63c18 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -13,11 +13,18 @@ Scrapy 1.8.4 (unreleased) **Security bug fix:** -- Fixed regular expressions susceptible to a `ReDoS attack`_ affecting the - ``iternodes`` node iterator of :class:`~scrapy.spiders.XMLFeedSpider`. +- Due to its `ReDoS vulnerabilities`_, ``scrapy.utils.iterators.xmliter`` is + now deprecated in favor of :func:`~scrapy.utils.iterators.xmliter_lxml`, + which :class:`~scrapy.spiders.XMLFeedSpider` now uses. + + To minimize the impact of this change on existing code, + :func:`~scrapy.utils.iterators.xmliter_lxml` now supports indicating + the node namespace as a prefix in the node name, and big files with highly + nested trees when using libxml2 2.7+. + Please, see the `cc65-xxvf-f7r9 security advisory`_ for more information. - .. _ReDoS attack: https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS + .. _ReDoS vulnerability: https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS .. _cc65-xxvf-f7r9 security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-cc65-xxvf-f7r9 .. _release-1.8.3: diff --git a/scrapy/spiders/feed.py b/scrapy/spiders/feed.py index 06e212e1c..d793a991f 100644 --- a/scrapy/spiders/feed.py +++ b/scrapy/spiders/feed.py @@ -5,7 +5,7 @@ for scraping from an XML feed. See documentation in docs/topics/spiders.rst """ from scrapy.spiders import Spider -from scrapy.utils.iterators import xmliter, csviter +from scrapy.utils.iterators import csviter, xmliter_lxml from scrapy.utils.spider import iterate_spider_output from scrapy.selector import Selector from scrapy.exceptions import NotConfigured, NotSupported @@ -82,7 +82,7 @@ class XMLFeedSpider(Spider): return self.parse_nodes(response, nodes) def _iternodes(self, response): - for node in xmliter(response, self.itertag): + for node in xmliter_lxml(response, self.itertag): self._register_namespaces(node) yield node diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index feaf4812e..884e48ee2 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -6,8 +6,12 @@ try: except ImportError: from io import BytesIO from io import StringIO -import six +from warnings import warn +import six +from lxml import etree + +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import TextResponse, Response from scrapy.selector import Selector from scrapy.utils.python import re_rsearch, to_unicode @@ -24,6 +28,15 @@ def xmliter(obj, nodename): - a unicode string - a string encoded as utf-8 """ + warn( + ( + "xmliter is deprecated and its use strongly discouraged because " + "it is vulnerable to ReDoS attacks. Use xmliter_lxml instead. See " + "https://github.com/scrapy/scrapy/security/advisories/GHSA-cc65-xxvf-f7r9" + ), + ScrapyDeprecationWarning, + stacklevel=2, + ) nodename_patt = re.escape(nodename) HEADER_START_RE = re.compile(r'^(.{,1024}?)<\s*%s(?:\s|>)' % nodename_patt, re.S) @@ -42,12 +55,34 @@ def xmliter(obj, nodename): def xmliter_lxml(obj, nodename, namespace=None, prefix='x'): - from lxml import etree reader = _StreamReader(obj) tag = '{%s}%s' % (namespace, nodename) if namespace else nodename - iterable = etree.iterparse(reader, tag=tag, encoding=reader.encoding) + iterable = etree.iterparse( + reader, + encoding=reader.encoding, + events=("end", "start-ns"), + huge_tree=True, + ) selxpath = '//' + ('%s:%s' % (prefix, nodename) if namespace else nodename) - for _, node in iterable: + needs_namespace_resolution = not namespace and ":" in nodename + if needs_namespace_resolution: + prefix, nodename = nodename.split(":", maxsplit=1) + for event, data in iterable: + if event == "start-ns": + assert isinstance(data, tuple) + if needs_namespace_resolution: + _prefix, _namespace = data + if _prefix != prefix: + continue + namespace = _namespace + needs_namespace_resolution = False + selxpath = "//{prefix}:{nodename}".format(prefix=prefix, nodename=nodename) + tag = "{{{namespace}}}{nodename}".format(namespace=namespace, nodename=nodename) + continue + assert isinstance(data, etree._Element) + node = data + if node.tag != tag: + continue nodetext = etree.tostring(node, encoding='unicode') node.clear() xs = Selector(text=nodetext, type='xml') diff --git a/tests/test_spider.py b/tests/test_spider.py index 2220b8ffc..195326f1a 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -122,8 +122,8 @@ class XMLFeedSpiderTest(SpiderTest): body = b""" - http://www.example.com/Special-Offers.html2009-08-16 - http://www.example.com/2009-08-16 + http://www.example.com/Special-Offers.html2009-08-16 + http://www.example.com/2009-08-16 """ response = XmlResponse(url='http://example.com/sitemap.xml', body=body) diff --git a/tests/test_utils_iterators.py b/tests/test_utils_iterators.py index 2d845697e..129607d8d 100644 --- a/tests/test_utils_iterators.py +++ b/tests/test_utils_iterators.py @@ -3,6 +3,9 @@ import os import six from twisted.trial import unittest +import pytest + +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.utils.iterators import csviter, xmliter, _body_or_str, xmliter_lxml from scrapy.http import XmlResponse, TextResponse, Response from tests import get_testdata @@ -10,10 +13,8 @@ from tests import get_testdata FOOBAR_NL = u"foo\nbar" -class XmliterTestCase(unittest.TestCase): - - xmliter = staticmethod(xmliter) - +class XmliterBaseTestCase: + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter(self): body = b"""\ \ @@ -38,6 +39,7 @@ class XmliterTestCase(unittest.TestCase): self.assertEqual(attrs, [('001', ['Name 1'], ['Type 1']), ('002', ['Name 2'], ['Type 2'])]) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_unusual_node(self): body = b""" @@ -50,6 +52,7 @@ class XmliterTestCase(unittest.TestCase): for e in self.xmliter(response, 'matchme...')] self.assertEqual(nodenames, [['matchme...']]) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_unicode(self): # example taken from https://github.com/scrapy/scrapy/issues/1665 body = u""" @@ -105,12 +108,14 @@ class XmliterTestCase(unittest.TestCase): (u'21', [u'Ab'], [u'76']), (u'27', [u'A'], [u'27'])]) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_text(self): body = u"""onetwo""" self.assertEqual([x.xpath("text()").getall() for x in self.xmliter(body, 'product')], [[u'one'], [u'two']]) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_namespaces(self): body = b"""\ @@ -145,6 +150,7 @@ class XmliterTestCase(unittest.TestCase): self.assertEqual(node.xpath('id/text()').getall(), []) self.assertEqual(node.xpath('price/text()').getall(), []) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_exception(self): body = u"""onetwo""" @@ -154,10 +160,12 @@ class XmliterTestCase(unittest.TestCase): self.assertRaises(StopIteration, next, iter) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_objtype_exception(self): i = self.xmliter(42, 'product') self.assertRaises(AssertionError, next, i) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_encoding(self): body = b'\n\n Some Turkish Characters \xd6\xc7\xde\xdd\xd0\xdc \xfc\xf0\xfd\xfe\xe7\xf6\n\n\n' response = XmlResponse('http://www.example.com', body=body) @@ -166,8 +174,24 @@ class XmliterTestCase(unittest.TestCase): u'Some Turkish Characters \xd6\xc7\u015e\u0130\u011e\xdc \xfc\u011f\u0131\u015f\xe7\xf6' ) +class XmliterTestCase(XmliterBaseTestCase, unittest.TestCase): + xmliter = staticmethod(xmliter) -class LxmlXmliterTestCase(XmliterTestCase): + def test_deprecation(self): + body = b""" + + + + + """ + with pytest.warns( + ScrapyDeprecationWarning, + match="xmliter", + ): + next(self.xmliter(body, "product")) + + +class LxmlXmliterTestCase(XmliterBaseTestCase, unittest.TestCase): xmliter = staticmethod(xmliter_lxml) def test_xmliter_iterate_namespace(self):