fix(sitemap): join element text once and reject entity declarations

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)
 

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 = (
+        '<?xml version="1.0"?>\n'
+        '<!DOCTYPE urlset [<!ENTITY a "XXXXXXXXXX"><!ENTITY b "&a;&a;&a;&a;&a;&a;&a;&a;&a;&a;">]>\n'
+        '<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">'
+        f'<url><loc>{DEFAULT_URL}&b;</loc></url>'
+        '</urlset>'
+    )
+
+    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: