From 486b96ab54d890f57332f5bda1c97b78e64cf584 Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Fri, 31 Jul 2026 08:57:06 -0500 Subject: [PATCH 1/3] Tidy the code in `.finish_endtag()` This resolves some IDE warnings: * An unused `method` variable * A type annotation consideration --- src/feedparser_sgmllib/__init__.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/feedparser_sgmllib/__init__.py b/src/feedparser_sgmllib/__init__.py index dffdb10..16cf0d6 100644 --- a/src/feedparser_sgmllib/__init__.py +++ b/src/feedparser_sgmllib/__init__.py @@ -369,7 +369,7 @@ 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: @@ -381,11 +381,8 @@ def finish_endtag(self, tag: str) -> None: found = i 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) From 77b6a6877e285cb3f7b930dfeda1c29e42dd43cc Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Fri, 31 Jul 2026 09:06:25 -0500 Subject: [PATCH 2/3] Add a regression test: closing tags must close innermost tags --- tests/conftest.py | 26 ++++++++++++++++++++++++++ tests/test_sgmllib.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) 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..c9be3d6 100644 --- a/tests/test_sgmllib.py +++ b/tests/test_sgmllib.py @@ -455,6 +455,35 @@ 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"] + + # XXX These tests have been disabled by prefixing their names with # an underscore. The first two exercise outstanding bugs in the # sgmllib module, and the third exhibits questionable behavior From c67e14c1c03ab828d960514837cf7934cd2d0e3c Mon Sep 17 00:00:00 2001 From: Kurt McKee Date: Fri, 31 Jul 2026 09:31:29 -0500 Subject: [PATCH 3/3] Improve closing tag search behavior of `.finish_endtag()` --- .../20260731_090641_kurtmckee_improve_finish_endtag.rst | 9 +++++++++ src/feedparser_sgmllib/__init__.py | 7 ++++--- 2 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 changelog.d/20260731_090641_kurtmckee_improve_finish_endtag.rst diff --git a/changelog.d/20260731_090641_kurtmckee_improve_finish_endtag.rst b/changelog.d/20260731_090641_kurtmckee_improve_finish_endtag.rst new file mode 100644 index 0000000..412ef36 --- /dev/null +++ b/changelog.d/20260731_090641_kurtmckee_improve_finish_endtag.rst @@ -0,0 +1,9 @@ +Changed +------- + +* Improve the closing tag search behavior of ``SGMLParser.finish_endtag()``. + + Previously, the method always searched the entire tag stack + to close the innermost tag, which had a higher search cost. + The tag stack is now searched in reverse order + and breaks on the first matching tag. diff --git a/src/feedparser_sgmllib/__init__.py b/src/feedparser_sgmllib/__init__.py index 16cf0d6..27e832d 100644 --- a/src/feedparser_sgmllib/__init__.py +++ b/src/feedparser_sgmllib/__init__.py @@ -376,9 +376,10 @@ def finish_endtag(self, tag: str) -> None: 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] method: t.Callable[[], t.Any] | None = getattr(self, "end_" + tag, None)