fix: harden against confirmed review findings

src/feed/feed.mojo

@@ -37,6 +37,13 @@ from feed.xml_parser import (
     EVENT_EOF,
 )
 
+# Cap on the element-name stack. Real feeds nest only a handful deep; an
+# adversarial document with thousands of unclosed start tags would
+# otherwise grow the stack without bound (memory DoS) and make every
+# liberal end-tag reverse scan O(depth), i.e. O(n^2) over the document.
+# Bounding the stack bounds both. Chosen far above any legitimate feed.
+comptime _MAX_DEPTH = 1024
+
 
 def _canonical_ns_prefix(uri: String) -> String:
     """Canonical prefix for a known namespace URI, or "" if unknown."""
@@ -150,7 +157,11 @@ def parse_feed(var source: String, *, strict: Bool = False) raises -> Feed:
                 var canon = _canon_name(event.name, ns_map)
                 event.name = canon^
             ref name = event.name
-            stack.append(name.copy())
+            # Refuse to grow past the cap — see _MAX_DEPTH. Beyond it the
+            # document is pathological, not a feed, and we trade fidelity
+            # for a hard bound on memory and per-tag scan cost.
+            if len(stack) < _MAX_DEPTH:
+                stack.append(name.copy())
             var depth = len(stack)
             if depth == 1:
                 if name == "rss":
@@ -173,7 +184,13 @@ def parse_feed(var source: String, *, strict: Bool = False) raises -> Feed:
             # <summary>Hello <b>world</b> foo</summary> survives intact.
             if depth <= _field_depth(feed.kind, in_item, item_depth):
                 text = String()
-            if not in_item and (name == "item" or name == "entry"):
+            if name == "item" or name == "entry":
+                # A new item/entry opening while one is still open means
+                # the previous item's </item> was omitted. Flush it before
+                # starting the next so one missing end tag can't silently
+                # drop the whole feed (feedparser recovers both).
+                if in_item:
+                    feed.items.append(item^)
                 in_item = True
                 item_depth = depth
                 item = FeedItem()
@@ -251,10 +268,10 @@ def parse_feed(var source: String, *, strict: Bool = False) raises -> Feed:
 
             if in_item:
                 if name == "item" or name == "entry":
-                    if effective_depth == item_depth:
-                        feed.items.append(item^)
-                        item = FeedItem()
-                        in_item = False
+                    # Flushing is handled uniformly below, keyed on the
+                    # item element leaving the stack — so a missing </item>
+                    # closed by an ancestor flushes on the same path.
+                    pass
                 elif effective_depth == item_depth + 1:
                     _assign_item_field(
                         item,
@@ -297,6 +314,15 @@ def parse_feed(var source: String, *, strict: Bool = False) raises -> Feed:
                 elif name == "subtitle":
                     _set_if_empty(feed.description, value)
 
+            # Flush a still-open item whenever its start element leaves the
+            # stack: its own </item> (normal close) or an ancestor closing
+            # over an item whose </item> was omitted (liberal recovery).
+            if in_item and effective_depth <= item_depth:
+                feed.items.append(item^)
+                item = FeedItem()
+                in_item = False
+                pub_date_authoritative = False
+
             stack.shrink(idx)
             # Clear accumulation once a field-level element closes; a
             # nested child closing keeps the running text.

src/feed/xml_parser.mojo

@@ -31,6 +31,26 @@ def _is_space(b: UInt8) -> Bool:
     return b == 0x20 or b == 0x09 or b == 0x0A or b == 0x0D
 
 
+def _is_valid_xml_char(cp: Int) -> Bool:
+    """XML 1.0 Char production: the scalars a document may contain.
+
+    Excludes NUL and the C0 control block (except tab, LF, CR),
+    surrogates, and out-of-range codepoints. A numeric character
+    reference to a forbidden scalar (e.g. `�`) must not be emitted
+    verbatim — a raw NUL/control byte can truncate or corrupt whatever
+    consumes the decoded text.
+    """
+    if cp == 0x09 or cp == 0x0A or cp == 0x0D:
+        return True
+    if cp >= 0x20 and cp <= 0xD7FF:
+        return True
+    if cp >= 0xE000 and cp <= 0xFFFD:
+        return True
+    if cp >= 0x10000 and cp <= 0x10FFFF:
+        return True
+    return False
+
+
 def _append_codepoint(mut out: String, cp_in: Int):
     """UTF-8 encode a Unicode scalar value and append it to `out`.
 
@@ -492,6 +512,16 @@ struct XmlPullParser(Copyable, Movable):
                 out += String(StringSlice(unsafe_from_utf8=bytes[start:end]))
                 out += String(";")
                 return out^
+            if not _is_valid_xml_char(cp):
+                # Well-formed number, forbidden target (NUL, a C0 control,
+                # a surrogate, or out of range). Strict rejects it; liberal
+                # substitutes U+FFFD rather than emit a raw control byte.
+                if self.strict:
+                    raise self._strict_error(
+                        "character reference to a codepoint XML forbids",
+                        self.pos,
+                    )
+                cp = 0xFFFD
             _append_codepoint(out, cp)
             return out^
         var name = String(StringSlice(unsafe_from_utf8=bytes[start:end]))

test/test_feed.mojo

@@ -128,6 +128,60 @@ def test_stray_end_tag_ignored() raises:
     assert_equal(feed.items[0].title, "One")
 
 
+def test_missing_mid_item_close_recovers_both() raises:
+    # A dropped </item> between two items must not swallow the feed: the
+    # next <item> flushes the still-open one. feedparser recovers both.
+    var source: String = """<rss version="2.0"><channel><title>T</title>
+      <item><title>One</title><guid>g1</guid>
+      <item><title>Two</title><guid>g2</guid></item>
+    </channel></rss>"""
+    var feed = parse_feed(source^)
+    assert_equal(len(feed.items), 2)
+    assert_equal(feed.items[0].title, "One")
+    assert_equal(feed.items[0].guid, "g1")
+    assert_equal(feed.items[1].title, "Two")
+    assert_equal(feed.items[1].guid, "g2")
+
+
+def test_missing_last_item_close_recovers_via_ancestor() raises:
+    # The final </item> is dropped; the enclosing </channel> closes over
+    # it. The item must still be flushed rather than silently discarded.
+    var source: String = """<rss version="2.0"><channel><title>T</title>
+      <item><title>Only</title><guid>g9</guid>
+    </channel></rss>"""
+    var feed = parse_feed(source^)
+    assert_equal(len(feed.items), 1)
+    assert_equal(feed.items[0].title, "Only")
+    assert_equal(feed.items[0].guid, "g9")
+
+
+def test_missing_entry_close_recovers_atom() raises:
+    # Same recovery for Atom <entry>: a dropped </entry> before the next.
+    var source: String = """<feed xmlns="http://www.w3.org/2005/Atom">
+      <entry><title>A</title><id>a1</id>
+      <entry><title>B</title><id>b1</id></entry>
+    </feed>"""
+    var feed = parse_feed(source^)
+    assert_equal(len(feed.items), 2)
+    assert_equal(feed.items[0].title, "A")
+    assert_equal(feed.items[1].title, "B")
+
+
+def test_deep_unclosed_nesting_is_bounded() raises:
+    # Thousands of unclosed start tags plus stray end tags must not grow
+    # the name-stack without bound or make end-tag matching quadratic;
+    # parsing must complete and yield no bogus items.
+    var hostile = String('<rss version="2.0"><channel>')
+    for i in range(4000):  # well past _MAX_DEPTH (1024)
+        hostile += "<t" + String(i) + ">"
+    for _ in range(4000):
+        hostile += "</zzz>"
+    hostile += "</channel></rss>"
+    var feed = parse_feed(hostile^)
+    assert_equal(feed.kind, String(KIND_RSS))
+    assert_equal(len(feed.items), 0)
+
+
 def test_mixed_content_text_survives() raises:
     # Text before, inside, and after nested children must all be kept.
     var source: String = """<feed xmlns="http://www.w3.org/2005/Atom">

test/test_xml_parser.mojo

@@ -80,6 +80,16 @@ def test_out_of_range_codepoint_becomes_replacement() raises:
     assert_equal(events[1].text, "��")
 
 
+def test_forbidden_control_char_ref_becomes_replacement() raises:
+    # NUL and C0 controls are well-formed numeric refs but forbidden XML
+    # characters; they must decode to U+FFFD, never raw control bytes.
+    var events = _events("<t>a&#0;b&#7;c&#x1;d</t>")
+    assert_equal(events[1].text, "a�b�c�d")
+    # Tab/LF/CR remain legal control characters and pass through.
+    var ok = _events("<t>&#9;&#10;&#13;</t>")
+    assert_equal(ok[1].text, "\t\n\r")
+
+
 def test_entities_in_attributes() raises:
     var events = _events('<e title="a &amp; b"/>')
     assert_equal(events[0].attrs["title"], "a & b")
@@ -261,6 +271,11 @@ def test_strict_bare_ampersand() raises:
         _strict_events("<a>fish & chips</a>")
 
 
+def test_strict_forbidden_char_ref() raises:
+    with assert_raises(contains="XML forbids"):
+        _strict_events("<a>&#0;</a>")
+
+
 def test_strict_error_reports_location() raises:
     with assert_raises(contains="line 3"):
         _strict_events("<a>\n<b>\nx</a>\n</b>")