Skip to content

fix(removeScripts): unwrap an executable anchor only once - #2306

Open
nimbly1 wants to merge 2 commits into
svg:mainfrom
nimbly1:codex/unwrap-executable-anchor-once
Open

nimbly1 wants to merge 2 commits into
svg:mainfrom
nimbly1:codex/unwrap-executable-anchor-once

Conversation

@nimbly1

@nimbly1 nimbly1 commented Oct 4, 2026 •

Copy link
Copy Markdown

Bug

removeScripts collapses an <a> whose href or xlink:href is javascript:, legacy vbscript:, or an executable data: URL, and moves the anchor's children up to the parent. It did that splice during the anchor's exit, while the parent was still being walked.

Two cases follow from that:

  • SVG anchors often repeat the URL on both href and xlink:href. The plugin unwrapped once per attribute. After the first splice the anchor is no longer a child, so the second indexOf is -1. splice(-1, 1, ...children) replaces the parent's last child.
  • Removing an anchor shortens the live child list, so the walk skips the next sibling. An empty javascript: anchor left the following anchor, still pointing at javascript:, in the document.
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
  <a href="javascript:alert(1)" xlink:href="javascript:alert(2)">
    <rect id="kept"/>
  </a>
  <circle id="sibling"/>
  <a href="javascript:alert(1)"/>
  <a href="javascript:alert(2)"><rect id="next"/></a>
</svg>

Before this change the <circle> is removed and the second anchor is left in place. A safe href paired with an executable xlink:href still unwraps once. Nested executable anchors still unwrap from the inside out.

Fixes #2295.

Verification

  • The new removeScripts.12 fixture failed before the fix and passes after it. All 12 removeScripts cases pass.
  • pnpm test: 521 passed, 3 skipped, no type errors.
  • pnpm lint, pnpm typecheck, and pnpm spellcheck passed.

The regression and browser-bundle jobs were not run locally. removeScripts is not part of preset-default; this applies when that plugin is enabled.

An anchor can store the same javascript: URL in href and xlink:href. The
plugin spliced the anchor out for each attribute. The second splice ran at
index -1 and replaced the parent's last child.

Stop after the first unwrap so siblings stay in place.

Fixes svg#2295
Splicing an anchor out while its parent is still being visited shifts
the live child list, so the next sibling is never visited. An empty
javascript: anchor left the following anchor in the document.

Record each executable anchor once and unwrap it after the walk.
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.

removeScripts deletes innocent sibling elements and duplicates children on dual-href anchors

1 participant