Skip to content

Page-reference checking needs a parser, not another special case #197

Description

@lamemustafa

#178 finding 3 remains open: requireReferencedBundles in scripts/verify-extension-package.mjs matches only double-quoted attributes and requires a stylesheet's rel to precede its href. The current bundler emits exactly that, so the gate passes — it is verified against the emitter's formatting habits rather than against HTML.

A replacement was written and then withdrawn from #186. This records why, and what a correct one has to handle.

Why it was withdrawn

Six review rounds, each finding a real defect in the previous fix:

  1. Regexes matched only double-quoted attributes and rel-before-href — the original finding.
  2. [^>]+ ended a tag at a > inside a quoted attribute value, losing the src after it.
  3. Commented-out markup was scanned as active, rejecting a valid package.
  4. Script and style bodies are raw text; tag-shaped strings inside them were read as references.
  5. The blanking pass collected openers up front and mutated as it went, so a stale offset ran past a closing tag and blanked the next script's opening tag — a package missing that bundle verified clean.
  6. Comment stripping by global regex treated <!-- inside a quoted attribute as a real comment, blanking a real script between two such attributes.
  7. textarea and title are RCDATA and equally inert; example copy inside them was read as a reference.
  8. --!> is a valid comment close in Chrome; matching only --> blanked the rest of the document.
  9. \b matches before a hyphen, so <script-widget> was misread as a raw-text element and its body blanked.

Every miss failed open in one direction or the other — a valid package rejected, or a missing bundle accepted. Both are silent. Two of the holes were introduced by the fix for the previous hole, and the sixth round produced as many findings as the first.

What that says

The defect is the shape, not any one case. A required release gate that parses HTML with hand-rolled scanning keeps being wrong in ways nothing downstream notices, and each patch has narrowed one case while opening another.

What a replacement must handle

The list above is a usable conformance suite, and the regression tests for cases 2–9 were written and pass. Any replacement should also cover:

  • attribute quoting: double, single, unquoted
  • attribute order, and rel as a space-separated token list
  • > inside quoted values
  • comments in both --> and --!> forms, and comment delimiters appearing inside quoted values
  • raw text (script, style) and RCDATA (textarea, title) bodies
  • tag names that continue past the known name, such as custom elements
  • rel="icon" and similar non-bundle links, which must not be treated as references

Recommendation

Use a real HTML parser rather than a seventh special case. That needs a dependency, which is ask-first in this repository, and the trade is worth stating plainly: one small parsing dependency against a gate that has been wrong in nine distinct ways.

Until then the check is exactly as it was — no better, no worse.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions