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 dffdb10..27e832d 100644 --- a/src/feedparser_sgmllib/__init__.py +++ b/src/feedparser_sgmllib/__init__.py @@ -369,23 +369,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/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