Skip to content

Fix StripHTML crash when strip_comments is disabled - #3009

Merged
facelessuser merged 2 commits into
facelessuser:mainfrom
harshajv7981:fix/striphtml-keep-comments
Oct 24, 2026
Merged

facelessuser merged 2 commits into
facelessuser:mainfrom
harshajv7981:fix/striphtml-keep-comments

Conversation

@harshajv7981

Copy link
Copy Markdown
Contributor

Fixes #3008

Problem

StripHtmlPostprocessor.run() checks self.strip_js_on_attributes, which __init__ never sets. Any config with strip_comments: False raises AttributeError on every document.

Change

Drop the stale attribute from the check: strip = self.strip_comments or self.re_attributes. re_attributes is built whenever strip_js_on_attributes is on (it adds on\w+) or strip_attributes is non-empty, so this is what the original line meant. With everything disabled it's still a no-op. Added a changelog entry under 12.3.1.

Verification

  • New regression test tests/test_extensions/test_striphmtl.py::TestStripHTMLKeepComments::test_keep_comments fails before this change with the AttributeError and passes after.
  • tests/test_extensions: 783 passed, no new failures.

I found this with patchwork, an automated code-audit tool I run, and verified the reproduction test in an isolated sandbox. If tool-assisted reports aren't welcome here, let me know and I won't send more.

run() referenced self.strip_js_on_attributes, which __init__ never
sets, so any config with strip_comments: False raised AttributeError.
re_attributes already covers the on* attributes, so drop the stale check.

Fixes facelessuser#3008
@gir-bot gir-bot added S: needs-review Needs to be reviewed and/or approved. C: docs Related to documentation. C: source Related to source code. C: striphtml Related to the striphtml extension. C: tests Related to testing. labels Oct 23, 2026
@facelessuser

Copy link
Copy Markdown
Owner

Good find, thanks for the PR!

@gir-bot lgtm

@gir-bot gir-bot added S: approved The pull request is ready to be merged. and removed S: needs-review Needs to be reviewed and/or approved. labels Oct 24, 2026
@facelessuser
facelessuser merged commit 9d879fa into facelessuser:main Oct 24, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C: docs Related to documentation. C: source Related to source code. C: striphtml Related to the striphtml extension. C: tests Related to testing. S: approved The pull request is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StripHTML: AttributeError when strip_comments is False

3 participants