Skip to content

dogfood: SSR misses the script-data-double-escaped state #1134

Description

@vivek7405

Problem

inertRanges ends a <script> element's content at the first </script>, but the HTML
tokenizer does not always stop there. Once a script body contains <!-- followed by
<script, the tokenizer enters the script-data-double-escaped state, where the next
</script> does NOT close the element. So bytes that are still script text get scanned as
markup, and a component tag among them is instantiated, which is the #1128 damage mode in
the one element the fix explicitly claims to handle.

Confirmed in both directions. Chromium treats the probe as script text:

<div id="b"><script type="text/plain"><!-- <script> </script> <x-probe id="p"></x-probe> --></script><span id="after">A</span></div>
  -> probeIsElement: false, #b children: ["SCRIPT", "SPAN"]

WebJs SSR (on the #1132 branch, which fixed the rest of this family) instantiates it:

...<!-- <script> </script> <p-probe data-wj-host><!--webjs-hydrate--><b>OK</b></p-probe> --></script>...

The trigger is narrow: the legacy pattern of a comment-wrapped inline script that writes a
<script> tag. It is not something a WebJs app would normally author, which is why this is
filed rather than fixed in #1132. It should not stay undocumented though, because both
references/components.md and #1132 assert that <script> content is handled.

Found by a differential fuzz of inertRanges against parse5 (~221k generated fixtures) during
the review of #1132. That sweep found no other divergence apart from this, the deliberate
<noscript> carve-out, a parse5 6.x bug on <noframes> (WebJs matches Chromium, parse5 is
wrong), and SVG foreign content, where every divergence is a harmless false negative needing a
component tag literally inside <svg><title> or <svg><style>.

Design / approach

Model the double-escape transition in the raw-text scan for <script> specifically: while
consuming script content, if <!-- is seen followed later by <script, the element does not
end until a </script> that follows a matching </script-escaped exit. The spec states this
as the script-data-escaped / script-data-double-escaped state pair.

Worth deciding whether it is worth the complexity at all. The alternative disposition is to
document it as a known limitation and close, since the trigger requires authoring a legacy
comment-wrapped script inside a WebJs template, and the failure is loud (visible broken
markup) rather than silent. If it is fixed, the state tracking belongs inside inertRanges's
raw-text branch and nowhere else, so it cannot affect the template tokenizer.

Implementation notes (for the implementing agent)

Where to edit

  • packages/core/src/render-server.js, the raw-text / RCDATA branch of inertRanges (search
    for isTextOnlyTag), specifically the new RegExp('</' + tag + ...) close scan. That single
    scan is the whole surface.
  • isTextOnlyTag and endOfComment in the same file are the neighbouring helpers; neither
    should need changing.

Landmines

  • Read the review trail on fix(core): stop SSR instantiating a component named inside a comment #1132 first. Three separate attempts at this scanner shipped
    failures worse than the bug (a naive indexOf, a word-boundary that matched hyphenated
    custom element names, an unbalanced quote that consumed to EOF), and each was caught only
    because a reviewer reproduced it. Any change here needs a differential check against a real
    parser, not reasoning alone.
  • isRawtextTag is shared with the template tokenizer, where it decides hole escaping.
    Do NOT widen or alter it for this; isTextOnlyTag exists precisely to keep the two apart.
  • Every guard in this scanner is pinned by a counterfactual test, and fix(core): stop SSR instantiating a component named inside a comment #1132 established that
    as the bar after several tests were found to pass with their own guard reverted. Match it.

Invariants to respect

Tests + docs surfaces

  • Unit: packages/core/test/rendering/ssr-comment-not-an-element.test.js, alongside the other
    context cases.
  • Counterfactual: the test must fail when the guard is reverted.
  • Cross-runtime: test/bun/comment-not-an-element.mjs covers this scanner on both runtimes and
    is where a close-scan change should also be asserted.
  • Docs: references/components.md lists the text-only elements. If this is closed as a
    documented limitation instead of fixed, that is the place to say so.

Acceptance criteria

  • A component tag inside a double-escaped script body is not instantiated, OR the
    limitation is documented in references/components.md and this is closed as won't-fix
  • Ordinary <script> content still ends at its first </script> (no regression for the
    normal case, which is every real app)
  • A counterfactual proves the new test fails when the guard is removed
  • Output is unchanged for inputs without a double-escaped script

Activity

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

  • Status
    Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions