Remove unreachable entity escaping passes that hang on large HTML pages

rssparser/src/commonMain/kotlin/com/prof18/rssparser/RssParser.kt

@@ -63,41 +63,30 @@ public class RssParser internal constructor(
      * - Escapes standalone ampersands that aren't part of valid entity references
      * - Fixes self-closing or unclosed tags that should be properly closed
      * - Fixes duplicate closing tags with content between them
-     * - Handles special cases for ampersands in URLs and text content
      */
-    private fun escapeInvalidXmlEntities(xml: String): String {
+    internal fun escapeInvalidXmlEntities(xml: String): String {
         return xml
             // Fix standalone ampersands in URLs and text
-            .replace(
-                Regex("&(?!(amp;|lt;|gt;|quot;|apos;|#[0-9]+;|#x[0-9a-fA-F]+;))"),
-                "&"
-            )
+            .replace(STANDALONE_AMPERSAND_REGEX, "&")
             // Fix duplicate closing tags with content between them
             // Example: <category></category><![CDATA[News]]></category> -> <category><![CDATA[News]]></category>
-            .replace(
-                Regex("<([^>]+)></\\1>([^<]+|<!\\[CDATA\\[.+?\\]\\]>)</\\1>"),
-                "<$1>$2</$1>"
-            )
+            .replace(DUPLICATE_CLOSING_TAG_REGEX, "<$1>$2</$1>")
             // Fix self-closing tags, but only if they don't already have content
             // This regex checks that there's no content between the opening and closing tags
-            .replace(
-                Regex("<(link|source|category|guid|enclosure|media:content|media:thumbnail)([^>]*?)>\\s*</\\1>"),
-                "<$1$2></$1>"
-            )
+            .replace(EMPTY_FEED_TAG_REGEX, "<$1$2></$1>")
             // Fix other common HTML tags that might be self-closing
-            .replace(
-                Regex("<(meta|img|br|hr|input|area|base|col|embed|keygen|param|track|wbr)([^>]*?)/?>(?!</\\1>)"),
-                "<$1$2></$1>"
-            )
-            // Additional pass to catch any ampersands in CDATA sections or attribute values that might have been missed
-            .replace(
-                Regex("(<!\\[CDATA\\[.*?)&(?!(amp;|lt;|gt;|quot;|apos;|#[0-9]+;|#x[0-9a-fA-F]+;))(.*?\\]\\]>)"),
-                "$1&amp;$3"
-            )
-            .replace(
-                Regex("=\"(.*?)&(?!(amp;|lt;|gt;|quot;|apos;|#[0-9]+;|#x[0-9a-fA-F]+;))(.*?)\""),
-                "=\"$1&amp;$3\""
-            )
+            .replace(VOID_HTML_TAG_REGEX, "<$1$2></$1>")
+    }
+
+    private companion object {
+        private val STANDALONE_AMPERSAND_REGEX =
+            Regex("&(?!(amp;|lt;|gt;|quot;|apos;|#[0-9]+;|#x[0-9a-fA-F]+;))")
+        private val DUPLICATE_CLOSING_TAG_REGEX =
+            Regex("<([^>]+)></\\1>([^<]+|<!\\[CDATA\\[.+?\\]\\]>)</\\1>")
+        private val EMPTY_FEED_TAG_REGEX =
+            Regex("<(link|source|category|guid|enclosure|media:content|media:thumbnail)([^>]*?)>\\s*</\\1>")
+        private val VOID_HTML_TAG_REGEX =
+            Regex("<(meta|img|br|hr|input|area|base|col|embed|keygen|param|track|wbr)([^>]*?)/?>(?!</\\1>)")
     }
 }
 

rssparser/src/commonTest/kotlin/com/prof18/rssparser/EscapeInvalidXmlEntitiesTest.kt

@@ -0,0 +1,87 @@
+package com.prof18.rssparser
+
+import com.prof18.rssparser.internal.ParserInput
+import com.prof18.rssparser.internal.XmlFetcher
+import kotlinx.coroutines.test.runTest
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
+
+class EscapeInvalidXmlEntitiesTest : XmlParserTestExecutor() {
+
+    /**
+     * A feed URL pointing at a website hands the escaping fallback a full HTML page, which is one
+     * very long line. The test passes or it times out, there is nothing in between.
+     */
+    @Test
+    fun escapingALargeMinifiedHtmlPageTerminates() = runTest {
+        val html = buildMinifiedHtmlPage()
+        assertTrue(html.length > 800_000, "expected a page big enough to be representative")
+        assertEquals(0, html.count { it == '\n' }, "the page has to be one single line")
+
+        val escaped = rssParser().escapeInvalidXmlEntities(html)
+
+        assertFalse(BARE_AMPERSAND_REGEX.containsMatchIn(escaped))
+    }
+
+    @Test
+    fun bareAmpersandsAreEscapedEverywhereIncludingCdataAndAttributes() = runTest {
+        val xml = """
+            <item>
+                <link>https://example.com/a?ref=home&pos=1</link>
+                <enclosure url="https://cdn.example.com/a.mp3?token=x&exp=2" type="audio/mpeg"/>
+                <description><![CDATA[Tom & Jerry, R&D and Q&A]]></description>
+            </item>
+        """.trimIndent()
+
+        val escaped = rssParser().escapeInvalidXmlEntities(xml)
+
+        assertTrue(escaped.contains("?ref=home&amp;pos=1"))
+        assertTrue(escaped.contains("?token=x&amp;exp=2"))
+        assertTrue(escaped.contains("Tom &amp; Jerry, R&amp;D and Q&amp;A"))
+        assertFalse(BARE_AMPERSAND_REGEX.containsMatchIn(escaped))
+    }
+
+    @Test
+    fun alreadyValidEntitiesAreLeftAlone() = runTest {
+        val xml = "<title>A &amp; B &lt;C&gt; &quot;D&quot; &apos;E&apos; &#38; &#x26;</title>"
+
+        assertEquals(xml, rssParser().escapeInvalidXmlEntities(xml))
+    }
+
+    private fun rssParser(): RssParser = RssParser(
+        xmlFetcher = object : XmlFetcher {
+            override suspend fun fetchXml(url: String): ParserInput =
+                error("not used by these tests")
+
+            override suspend fun fetchXmlAsString(url: String): String =
+                error("not used by these tests")
+        },
+        xmlParser = createXmlParser()
+    )
+
+    /**
+     * Roughly what a news homepage looks like to the parser: no newlines, and thousands of
+     * `attribute="…"` pairs holding query strings with bare ampersands in them.
+     */
+    private fun buildMinifiedHtmlPage(): String = buildString {
+        append("<!DOCTYPE html><html lang=\"de\"><head><meta charset=\"utf-8\">")
+        append("<title>Nachrichten</title></head><body>")
+        repeat(HTML_ELEMENT_COUNT) { index ->
+            append("<div class=\"teaser\" data-id=\"$index\">")
+            append("<a href=\"https://www.example.com/artikel-$index?ref=home&pos=$index&t=live\">")
+            append("<img src=\"https://img.example.com/$index.jpg?w=640&h=360\" alt=\"Bild $index\">")
+            append("Schlagzeile $index &mdash; mehr dazu")
+            append("</a></div>")
+        }
+        append("</body></html>")
+    }
+
+    private companion object {
+        // ~4.500 elements of ~200 chars lands around 900 KB, the size of a real news homepage.
+        private const val HTML_ELEMENT_COUNT = 4_500
+        private val BARE_AMPERSAND_REGEX =
+            Regex("&(?!(amp;|lt;|gt;|quot;|apos;|#[0-9]+;|#x[0-9a-fA-F]+;))")
+    }
+}