Add an evalScripts example - #1573
Merged
Merged
Conversation
`evalScripts` was documented in the README but demonstrated nowhere, and it is the option the Security section largely exists for. The example injects two SVGs whose scripts write counters on the page, one with `'once'` and one with `'always'`, and a button re-injects both so the difference is visible. The script in `always.svg` sits inside a `<g>`, which keeps the nested-script removal fix from 12.0.0 exercised in a real app rather than only in a fixture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
examples/eval-scripts, the seventh example.evalScriptsis documented in the README but was demonstrated nowhere, and it is the option the Security section largely exists for, which argues for a working reference rather than prose alone.Two SVGs, each carrying a
<script>that increments a counter on the page. One is injected withevalScripts: 'once'and the other withevalScripts: 'always', and a button injects both again. That re-injection is the only thing that separates the two settings: the'once'counter stops at 1 while the'always'counter tracks the injection count. The page also renders the number of<script>elements surviving in the injected markup, which is 0 whichever setting is used.The script in
always.svgsits inside a<g>rather than at the root. That is the case that threwNotFoundErroron removal before 12.0.0 and jammed the element with no callback of any kind, so the example keeps the fix exercised in a real app rather than only in a fixture.Re-injection needs a fresh placeholder
No earlier example injects the same file twice, so this one is the first to run into it: the element passed to
SVGInjectoris replaced by its SVG and is gone from the document, so there is nothing to reuse. The example holds a slot<div>per case and puts a new placeholder in it each time. The README states this, since anyone copying the example to build a re-injecting page hits it immediately.The cache keeps its copy's scripts
Worth recording because it is what makes the demonstration work at all.
loadSvgCachedcaches the parsed document and hands each waiter a clone, so the strip inevalSvgScriptsruns on the clone and the cached original is untouched. Re-injections therefore still find scripts to remove; onlyranScriptsdecides whether they run. Had the strip mutated the cached copy,'always'would have run once and then had nothing left to evaluate.No dev-versus-preview divergence
Unlike #1572,
vite devand the build behave identically here: both SVGs live inpublic/, which the dev server serves verbatim withimage/svg+xml. Checked rather than assumed, so the example README has no dev-mode caveat.Verification
Tests first. The generic example entry plus two dedicated tests were added before the example existed and failed on a
waitForFunctiontimeout, since nothing was injected at that path.npm testgreen:check:format,check:types,lint,build(publint, attw),size4.26 kB ESM / 4.30 kB CJS against the 5.5 kB budget, 360 library tests across chromium, firefox and webkit, then seven examples built against a freshnpm packand 17 example tests.Also driven by hand in real Chrome against the built page served over HTTP: counters read 1 and 3 after three injections,
<script>elements remaining 0, no console errors.🤖 Generated with Claude Code