diff --git a/src/crawlee/_utils/sitemap.py b/src/crawlee/_utils/sitemap.py index a78ecb1e21..0b83b2484b 100644 --- a/src/crawlee/_utils/sitemap.py +++ b/src/crawlee/_utils/sitemap.py @@ -118,7 +118,7 @@ def __init__(self) -> None: self._root_tag_name: str | None = None self._current_tag: str | None = None self._current_url: _SitemapItem = {} - self._buffer: str = '' + self._buffer: list[str] = [] self._items: list[_SitemapItem] = [] @property @@ -132,16 +132,17 @@ def startElement(self, name: str, attrs: AttributesImpl) -> None: if name in ('loc', 'lastmod', 'changefreq', 'priority'): self._current_tag = name - self._buffer = '' + self._buffer = [] def characters(self, content: str) -> None: + # Expat splits one text node into many callbacks, so the pieces are joined once in `endElement`. if self._current_tag: - self._buffer += content + self._buffer.append(content) @override def endElement(self, name: str) -> None: if name == self._current_tag: - text = self._buffer.strip() + text = ''.join(self._buffer).strip() if name == 'loc': if self._root_tag_name == 'sitemapindex': @@ -168,6 +169,21 @@ def endElement(self, name: str) -> None: self._current_url = {} +class _SitemapExpatParser(ExpatParser): + """Expat SAX parser that rejects entity declarations. + + Sitemaps never declare entities, and an internal entity lets a few kilobytes of input expand into megabytes of text. + """ + + @override + def reset(self) -> None: + super().reset() + self._parser.EntityDeclHandler = self._reject_entity_declaration # ty: ignore[unresolved-attribute] + + def _reject_entity_declaration(self, name: str, *_args: object) -> None: + raise ValueError(f'Entity declarations are not allowed in sitemaps, found {name!r}') + + class _TxtSitemapParser: """Parser for plaintext sitemaps that processes data as a stream.""" @@ -206,7 +222,7 @@ class _XmlSitemapParser: """Parser for XML sitemaps using SAX to process data as a stream.""" def __init__(self) -> None: - self._parser = ExpatParser() + self._parser = _SitemapExpatParser() self._handler = _XMLSaxSitemapHandler() self._parser.setContentHandler(self._handler) diff --git a/tests/unit/_utils/test_sitemap.py b/tests/unit/_utils/test_sitemap.py index fed4654484..40fcb30bed 100644 --- a/tests/unit/_utils/test_sitemap.py +++ b/tests/unit/_utils/test_sitemap.py @@ -759,7 +759,40 @@ def test_xml_handler_resets_current_tag_on_end_element() -> None: # Stray text between elements must not be buffered. handler.characters(' stray text ') - assert handler._buffer == 'https://example.com/' + assert handler._buffer == ['https://example.com/'] + + +def test_xml_handler_joins_text_split_across_callbacks() -> None: + """A text node delivered in many `characters()` callbacks is joined into one value.""" + handler = _XMLSaxSitemapHandler() + handler.startElement('urlset', MagicMock()) + handler.startElement('url', MagicMock()) + handler.startElement('loc', MagicMock()) + handler.characters(' https://example.com/') + for _ in range(1000): + handler.characters('a') + handler.characters(' ') + handler.endElement('loc') + handler.endElement('url') + + assert handler.items == [{'type': 'url', 'loc': 'https://example.com/' + 'a' * 1000}] + + +async def test_sitemap_with_entity_declaration_is_rejected(caplog: pytest.LogCaptureFixture) -> None: + """A sitemap declaring an internal entity yields no URLs, so the entity is never expanded.""" + sitemap_xml = ( + '\n' + ']>\n' + '' + f'{DEFAULT_URL}&b;' + '' + ) + + with caplog.at_level(logging.WARNING): + sitemap = await Sitemap.from_xml_string(sitemap_xml) + + assert sitemap.urls == [] + assert 'Entity declarations are not allowed in sitemaps' in caplog.text def test_xml_handler_discards_metadata_from_url_without_loc() -> None: