diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9c5c848..03110b5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,6 +20,36 @@ Please see the fragment files in the `changelog.d directory`_. .. scriv-insert-here +.. _changelog-2.1.0: + +2.1.0 - 2026-08-02 +================== + +Fixed +----- + +* Close tags more correctly, even if the end tag has trailing text. + + For example, ```` now closes ```` correctly. + The trailing content is not available to parsers. + +* Fix quadratic CPU usage in ``SGMLParser.feed()``. + + This can occur when a large document with an unfinished token + (like `` None: """Initialize and reset this instance.""" self.verbose = verbose + + # These variables help guard against quadratic CPU usage behavior + # without significantly increasing memory usage. + # + # Previously, each call to `.feed()` resulted in string-concatenation + # and a regular expression matching from the start of the raw data + # even though it was known that the existing raw data wouldn't match. + # For large unclosed tokens (like ` None: """Reset this instance. Loses all unprocessed data.""" self.__starttag_text: str | None = None - self.rawdata = "" + self._rawdata = "" + self._pending = StringIO() + self._retry_at = 0 self.stack: list[str] = [] self.lasttag = "???" self.nomoretags = 0 self.literal = 0 super().reset() + @property + def rawdata(self) -> str: + self._flush_pending() + return self._rawdata + + @rawdata.setter + def rawdata(self, value: str) -> None: + self._rawdata = value + self._pending = StringIO() + + def _flush_pending(self) -> None: + """Merge any chunks queued by `feed()` into `self._rawdata`.""" + + if self._pending.tell(): + self._rawdata += self._pending.getvalue() + self._pending = StringIO() + def setnomoretags(self) -> None: """Enter literal mode (CDATA) till EOF. @@ -99,11 +142,19 @@ def feed(self, data: str) -> None: all the processing is done by goahead().) """ - self.rawdata = self.rawdata + data + self._pending.write(data) + if len(self._rawdata) + self._pending.tell() < self._retry_at: + # The last attempt to resolve the pending token failed. + # Skip re-scanning until enough new data has arrived + # to make another attempt worthwhile. + return + self._flush_pending() self.goahead(False) def close(self) -> None: """Handle the remaining data.""" + + self._flush_pending() self.goahead(True) def error(self, message: str) -> t.NoReturn: @@ -113,6 +164,7 @@ def error(self, message: str) -> t.NoReturn: # and data to be processed by a subsequent call. If 'end' is # true, force handling all data as if followed by EOF marker. def goahead(self, end: bool) -> None: + # Accessing `self.rawdata` flushes pending data. rawdata = self.rawdata i = 0 n = len(rawdata) @@ -225,6 +277,15 @@ def goahead(self, end: bool) -> None: self.handle_data(rawdata[i:n]) i = n self.rawdata = rawdata[i:] + if i == 0 and n > 0: + # No progress was made this call. + # Use exponential back off to set how much new data must arrive + # before the next scan retry. + # This ensures a persistently-incomplete token costs O(n) overall + # instead of O(n**2). + self._retry_at = n * 2 + else: + self._retry_at = 0 # XXX if end: check for empty stack # Extensions for the DOCTYPE scanner: @@ -361,6 +422,8 @@ def finish_starttag( # Internal -- finish processing of end tag def finish_endtag(self, tag: str) -> None: + if (match := tagfind.match(tag)) is not None: + tag = match.group(0) if not tag: found = len(self.stack) - 1 if found < 0: @@ -369,23 +432,21 @@ def finish_endtag(self, tag: str) -> None: else: if tag not in self.stack: try: - method = getattr(self, "end_" + tag) + getattr(self, "end_" + tag) except AttributeError: self.unknown_endtag(tag) else: self.report_unbalanced(tag) return found = len(self.stack) - for i in range(found): - if self.stack[i] == tag: - found = i + for i, stack_tag in enumerate(reversed(self.stack)): + if stack_tag == tag: + found = len(self.stack) - 1 - i + break while len(self.stack) > found: tag = self.stack[-1] - try: - method = getattr(self, "end_" + tag) - except AttributeError: - method = None - if method: + method: t.Callable[[], t.Any] | None = getattr(self, "end_" + tag, None) + if method is not None: self.handle_endtag(tag, method) else: self.unknown_endtag(tag) diff --git a/src/feedparser_sgmllib/_compat.py b/src/feedparser_sgmllib/_compat.py new file mode 100644 index 0000000..2f67667 --- /dev/null +++ b/src/feedparser_sgmllib/_compat.py @@ -0,0 +1,38 @@ +# Manage compatibility across supported Python versions. + +import sys + +__all__ = [ + "StringIO", +] + + +if sys.version_info >= (3, 12): + from io import StringIO +else: + # In Python >=3.12, `io.StringIO` has good memory allocation performance, + # which makes it a good candidate for collecting unparsed input data + # (see the comment in `SGMLParser.__init__()` about CPU and memory usage). + # + # However, the performance of `io.StringIO` is poor in Python 3.11 and below. + # For these lower Python versions, bytearray exhibits good performance + # but requires manual tracking of the length of the decoded string + # as well as calls to `.encode()` and `.decode()`. + # + # Therefore, the `io.StringIO` interface is mimicked here + # and backed by a bytearray implementation. + class StringIO: + def __init__(self) -> None: + self._buffer = bytearray() + self._length = 0 + + def write(self, text: str) -> int: + self._buffer.extend(text.encode()) + self._length += len(text) + return len(text) + + def getvalue(self) -> str: + return self._buffer.decode() + + def tell(self) -> int: + return self._length diff --git a/tests/conftest.py b/tests/conftest.py index d4dd00d..e9aa9db 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -128,3 +128,29 @@ def handle_charref(self, name): def handle_entityref(self, name): self.events.append(("entityref", name)) sgmllib.SGMLParser.handle_entityref(self, name) + + +class NestedTagCollector(sgmllib.SGMLParser): + def __init__(self) -> None: + super().__init__() + self.events = [] + + def start_a(self, attrs): + self.events.append(("start_a", attrs)) + + def end_a(self): + self.events.append(("end_a",)) + + def start_b(self, attrs): + self.events.append(("start_b", attrs)) + + def end_b(self): + self.events.append(("end_b",)) + + def unknown_endtag(self, tag): + self.events.append(("unknown_endtag", tag)) + + +@pytest.fixture +def nested_tag_collector() -> NestedTagCollector: + return NestedTagCollector() diff --git a/tests/test_sgmllib.py b/tests/test_sgmllib.py index 0387e8a..348903a 100644 --- a/tests/test_sgmllib.py +++ b/tests/test_sgmllib.py @@ -1,5 +1,7 @@ import io import pathlib +import time +import tracemalloc import pytest @@ -455,6 +457,190 @@ def test_attrfind_trailing_dollar_sign(): assert sgmllib.attrfind.search(text).groups() == ("i", '="true"', '"true"') +def test_endtag_closes_innermost_matching_tag(nested_tag_collector): + """ + Regression test: Confirm closing tags close innermost tags. + + For example: + + + + The must close id=2 only, leaving open. + """ + + nested_tag_collector.feed('') + + assert nested_tag_collector.events == [ + ("start_a", [("id", "1")]), + ("start_b", []), + ("unknown_endtag", "c"), + ("start_a", [("id", "2")]), + ("end_a",), + ] + assert nested_tag_collector.stack == ["a", "b"] + + # The innermost (id=2) should be closed, and should still be open. + # Therefore, `end_b` should be called, not `unknown_endtag`. + nested_tag_collector.feed("") + assert nested_tag_collector.events[-1] == ("end_b",) + assert nested_tag_collector.stack == ["a"] + + +@pytest.mark.parametrize( + "trailing_text", + ( + " bonus", + "/", + "=x", + ";", + ",", + ), +) +def test_endtag_with_trailing_content_still_closes_tag( + cdata_event_collector, trailing_text +): + """Verify that a closing tag with trailing content still closes the tag.""" + + cdata_event_collector.check_events( + f"contentafter", + [ + ("starttag", "cdata", []), + ("data", "content"), + ("endtag", "cdata"), + ("data", "after"), + ], + ) + assert cdata_event_collector.stack == [] + + +def test_endtag_with_no_valid_tag_name_does_not_close_open_tag(nested_tag_collector): + """Verify invalid tag names do not close any open tags.""" + + nested_tag_collector.feed("contentafter") + + assert nested_tag_collector.events == [ + ("start_a", []), + ("unknown_endtag", "123"), + ] + assert nested_tag_collector.stack == ["a"] + + +@pytest.mark.parametrize( + "prefix", + ( + pytest.param('