Skip to content

transform.js: regex-based HTML surgery silently truncates pages and misses several URL attributes #48

Description

@oto-macenauer-absa

Summary

src/utils/transform.js parses and rewrites arbitrary third-party HTML with regular expressions. Several of those patterns fail on legal input, and the failure mode is silent — a page renders half-empty or a theme leaks, with no build error.

Concrete defects

1. </body> inside a script or comment truncates the page (:124)

const bodyMatch = html.match(/<body\b([^>]*)>([\s\S]*?)<\/body>/i);

Non-greedy: the first </body> anywhere wins. A sub-app that ships document.write("</body>"), or any inlined JS/JSON string containing that literal, loses everything after it. <head> (:123) has the same shape.

2. stripThemeBootstrap fails on any < inside the script (transform.js:95-97)

html.replace(/<script>[^<]*localStorage[^<]*classList[^<]*<\/script>\s*/gi, '')

[^<]* cannot cross a <, so a bootstrap containing i < n, a <= b, or --> is not stripped. That script then runs and re-adds the dark class — the exact thing the "light only" invariant exists to prevent, and it is the sub-app's own code deciding, so it will differ per app. The bodyClass sanitiser at :147-150 only strips the class from the static attribute, not from what the script does at runtime.

3. Only href/src/action are rewritten (:53-68)

Not rewritten, so they 404 under /{prefix}/{slug}/:

  • srcset / imagesrcset — responsive images are common in doc themes
  • poster, data, formaction
  • url(...) inside inline <style> blocks and inline style= attributes
  • content of <meta property="og:image">

Note the external CSS case is handled, but separately and in a different file (scripts/build-vite.js:285-290), which is its own bug — see the CSS depth issue.

4. Attribute regexes match inside comments and text nodes, e.g. a code sample in the docs showing href="/foo" gets rewritten in the rendered prose.

Suggested fix

Parse the HTML instead of pattern-matching it. parse5 or node-html-parser handles all four cases and is a build-time-only dependency, so it costs nothing at runtime. A rewriter over a real DOM can also walk a single attribute allowlist rather than three regexes, and can drop the sub-app theme bootstrap by inspecting the script's text content instead of guessing at its shape.

If a full parser is not wanted, at minimum: match the last </body>/</head>, and replace [^<]* with [\s\S]*? in stripThemeBootstrap.

Worth adding fixtures for each case to tests/build-integrity.spec.js — none of these are covered today.

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions