Repository navigation
fix(producer): anchor local-font embedding to its url() occurrence - #3405
Conversation
The embed step rewrote the compiled document with
result.replaceAll(localPath, dataUri) — a bare substring replace with no
surrounding syntax. That also rewrites the path anywhere else it appears,
including inside a LONGER url whose tail happens to match, producing a
corrupted value like url("file:///abs/data:font/woff2;base64,...").
Any two paths where one is a suffix of the other collide the same way;
img/logo.ttf and assets/img/logo.ttf are enough. Every sibling rewrite in
this file already anchors on url(...), so this one was the outlier.
Also add file: to LOCAL_FONTFACE_URL_RE's exclusion list. Without it an
absolute file:// src was classified as a project-relative path and
resolved to <projectDir>/file:/abs/..., and the failed read was swallowed
by an empty catch. That catch now logs, since a silently skipped font
means the composition renders in a fallback typeface with nothing saying
why.
Closes #3369
terencecho
left a comment
terencecho
left a comment
There was a problem hiding this comment.
Verdict: LGTM. Correct fix for the suffix-collision bug, tests cover the three failure modes that matter, and the two adjacent changes (file: exclusion + logged catch) are the right shape.
Fix is correct
urlOccurrenceRe uses the standard regex-escape set ([.*+?^${}()|[\]\\]) and a \1 back-reference so opening/closing quote must match. Verified end-to-end against the reported corruption plus a local() + format() fallback chain and a regex-meta path:
input: a { src: url("fonts/x.ttf") } b { src: url("file:///abs/fonts/x.ttf") }
output: a { src: url("data:font/woff2;base64,AAAA") } b { src: url("file:///abs/fonts/x.ttf") }
input: @font-face { src: local("X"), url(fonts/y.ttf) format("woff2"), url("fonts/z.ttf") format("truetype"); }
output: @font-face { src: local("X"), url("data:font/woff2;base64,AAAA") format("woff2"), url("fonts/z.ttf") format("truetype"); }
local() and format() siblings survive; only the matched url() is rewritten. Multi-url() @font-face blocks are handled because the extractor loop feeds each localPath to the anchored replace in turn — regex-collision-free even across sibling url()s where one path is a suffix of another.
Sibling .replaceAll audit — clean
Only three replaceAll(...url...) sites in htmlCompiler.ts; the other two already anchor via `url(${url})` (L1468, L1667) or via quotes (L1344). This PR fixes the last outlier. No other bare-substring rewrites of a font path.
file: exclusion
Correctly adds file: to LOCAL_FONTFACE_URL_RE's negative lookahead. Prior to the fix, url("file:///abs/font.ttf") was silently classified as project-relative, resolved to <projectDir>/file:/abs/..., and the read failure was swallowed by the empty catch. Now correctly ignored at extraction — and the catch logs a warn, so future silent-skip regressions surface.
Test coverage
Five cases including the suffix-collision (img/logo.ttf vs assets/img/logo.ttf — no file:// needed) and regex-meta path (fonts/x+y(1).ttf). The metachar test matters exactly as the PR body says: the fix introduces a regex where there was none, and an un-escaped path would be a new bug in place of the old.
Nit (non-blocking)
Whitespace inside url() — url( "fonts/x.ttf" ) — isn't matched by either LOCAL_FONTFACE_URL_RE (extraction) or urlOccurrenceRe (rewrite). Consistent (extraction and rewrite reject together, so no corruption), pre-existing behavior, and CSS authored via hyperframes won't hit it in practice.
CI green across all required checks (Producer unit + integration, regression shards 1–9, Windows tests, Typecheck, Preflight, Lint, Format, Fallow).
— Review by tai (pr-review)
What
Anchor local-font embedding to its
url(...)occurrence, and stop classifying an absolutefile://src as a project-relative path.Closes #3369.
Why
The embed step rewrote the whole compiled document with a bare substring replace:
localPathis a plain relative path likefonts/x.ttf, with no surroundingurl(orquotes. So it also rewrites that text anywhere else it appears — including inside a
longer URL whose tail happens to match.
Every sibling rewrite in this file already anchors on the surrounding syntax
(
url(${url}),"${url}"). This one was the outlier.The corruption, run literally
That is neither the original URL nor a valid data URI.
It does not need
file://to happen. Any two paths where one is a suffix of the othercollide —
img/logo.ttfandassets/img/logo.ttfare enough — so the blast radius is anyproject with nested asset directories.
How
Replace by matched
url()occurrence:Exported so the collision can be tested directly — it is invisible in ordinary projects and
trivial to reintroduce, since the naive form passes every test that uses a single font.
Two smaller changes in the same area:
file:added toLOCAL_FONTFACE_URL_RE's exclusion list, alongsidedata:andhttp(s):. Without it,url("file:///abs/font.ttf")was read as project-relative,resolved to
<projectDir>/file:/abs/font.ttf, and the failed read was swallowed.catch {}now logs. A font that cannot be read must not fail the render, buta silent skip means the composition renders in a fallback typeface with nothing saying why.
Test plan
Regression tests
htmlCompiler.fontEmbed.test.ts, five cases:url("file:///abs/fonts/x.ttf")survives untouchedfile://assets/img/logo.ttfsurvives whenimg/logo.ttfembedsurl()is left alonefonts/x+y(1).ttfis matched literally, not as a patternThe last one matters because the fix introduces a regex where there was none — an unescaped
path would be a new bug in place of the old one.
Suites
(The three existing suites use
bun:testrather than vitest, so they needbun test.)oxlint+oxfmton both touched files — clean