Skip to content

Fix remove_tags with greater-than in quoted attributes - #294

Open
deepakganesh78 wants to merge 1 commit into
scrapy:masterfrom
deepakganesh78:fix/issue284-remove-tags-quoted-gt
Open

Fix remove_tags with greater-than in quoted attributes#294
deepakganesh78 wants to merge 1 commit into
scrapy:masterfrom
deepakganesh78:fix/issue284-remove-tags-quoted-gt

Conversation

@deepakganesh78

Copy link
Copy Markdown

Fixes #284.

Reproduction

On current master, remove_tags('<a title="x>y">texty</a>') returns y">texty because tag matching stops at the > inside the quoted attribute value. The same happens for single-quoted attribute values.

Root cause

remove_tags used a regular expression that treated every raw > as the end of a tag. That cuts valid tags in the middle when quoted attributes contain >, leaking the tail of the opening tag into the output.

Fix

Replace the remove_tags tag regex with a small linear scanner that reads the tag name and ignores > while inside a quoted attribute value that starts after =. Quotes in unquoted attribute values keep the previous behavior, so malformed inputs such as <a b=c"d>keep</a> are still handled as before.

Compatibility notes

This changes remove_tags behavior only for valid tags whose quoted attribute values contain >. Existing which_ones and keep filtering is preserved.

Validation

  • Regression check with the fix reverted: python -m pytest tests\test_html.py::TestRemoveTags::test_remove_tags_with_gt_in_quoted_attribute tests\test_html.py::TestRemoveTags::test_remove_tags_with_unquoted_quote_in_attribute -q -> 1 failed, 1 passed (expected failure: y">texty vs texty).
  • Targeted tests: python -m pytest tests\test_html.py::TestRemoveTags -q -> 11 passed.
  • Full test suite: python -m pytest -> 409 passed, 4 skipped, 74 xfailed.
  • Lint on touched files: python -m ruff check w3lib\html.py tests\test_html.py -> all checks passed.
  • Formatting check on touched files: python -m ruff format --check w3lib\html.py tests\test_html.py -> 2 files already formatted.
  • Type check: python -m mypy w3lib tests -> success, no issues in 20 source files.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.72727% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.11%. Comparing base (8d10401) to head (e5fbddf).
⚠️ Report is 47 commits behind head on master.

Files with missing lines Patch % Lines
w3lib/html.py 92.72% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #294      +/-   ##
==========================================
- Coverage   98.47%   98.11%   -0.37%     
==========================================
  Files           9        9              
  Lines         853      902      +49     
  Branches      177      193      +16     
==========================================
+ Hits          840      885      +45     
- Misses          5        7       +2     
- Partials        8       10       +2     
Files with missing lines Coverage Δ
w3lib/html.py 97.57% <92.72%> (-2.43%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq

codspeed-hq Bot commented Aug 3, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 45 untouched benchmarks


Comparing deepakganesh78:fix/issue284-remove-tags-quoted-gt (e5fbddf) with master (8d10401)

Open in CodSpeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

remove_tags() leaks markup when an attribute value contains '>' (and the obvious fix is a ReDoS)

1 participant