Repository navigation
fix(cli): cloud render finds media whose file name is stored in another Unicode form than the page's - #7419
Conversation
…er Unicode form than the page's
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes at 6d08ece9. There are two blockers.
1. A decomposed reference to a decomposed file used to work and now fails. Every archive key is now NFC, but the page text is rewritten only for files that get offloaded. Files below OFFLOAD_MIN_BYTES stay in the zip under the NFC key, while the page still names them decomposed. The renderer matches names byte for byte, so it misses them. Main keyed these files by the disk spelling, so a page whose name matched the disk worked.
I ran a probe with the file assets/vide\u0301o.png (decomposed) and a page containing <img src="assets/vide\u0301o.png">:
- main:
buildPublishFileMapkeys areassets/vide%u0301o.png,index.html, so the page's reference is in the archive. - this head: the keys are
assets/vid%E9o.png,index.html, so the page's reference is not in the archive.
This is likely the same population the PR targets. On macOS, a page whose names come from the disk listing (ls output pasted by an agent, a picker that inserts the listed name) is decomposed just like the file. The PR body calls the decomposed-page case unaddressed, but for files that stay in the zip it is a new break, not existing behavior. Two ways to fix it:
- Keep the disk spelling as the key and add the NFC spelling as a second entry for the same
StagedFilewhen the two differ. The clash check still applies, and heavy files are offloaded anyway. - Or NFC-normalize local references in HTML/CSS when staging, so page and key always agree.
Either way, please add a test for a decomposed page with a decomposed small file that stays in the zip. It fails on this head.
2. CI: Comments fails. The comment-share ratchet reports that packages/cli/src/utils/publishProject.ts went from 7.5% to 7.6% (2 comment lines added). The new archiveKey doc comment is what raised it.
Checked and OK
- The 'Fails without the fix:' section names three tests, and I confirmed the test literals really differ (
vid\xe9osvsvide\u0301os).publishProject.test.tsandoffload.test.tspass at this head (109 tests). - The clash check between two files that differ only by Unicode form is a good change, and its error names both files.
- Offload lookups through
archiveKeyare consistent, and staged files are still read from the disk path, so Linux reads aren't affected. - The entry path in
render.tsis normalized to match the keys.
| ); | ||
| } | ||
| diskPathOf.set(key, filePath); | ||
| fileContents.set(key, stageFile(join(absProjectDir, filePath), onDiskMinBytes)); |
There was a problem hiding this comment.
Keying only by NFC drops the decomposed spelling. A small file that stays in the zip and is named decomposed by its page (like the disk) used to match and no longer does. Consider adding the NFC key next to the disk key, or normalizing the references in the page.
There was a problem hiding this comment.
Still requesting changes at 704e6441.
The only change since 6d08ece9 deletes the two-line doc comment on archiveKey. That fixes the Comments ratchet (blocker 2). Blocker 1 from #7419 (review) is not addressed.
Every archive key is still NFC, and the page text is only rewritten for files that get offloaded. So a small file that stays in the zip, with a decomposed name on disk and the same decomposed name in the page, is still missing from what the renderer looks up. Main keyed it by the disk spelling and found it.
Either fix from that review closes it: key by both spellings, or NFC-normalize the page's local references while staging. Please add a test with a decomposed page and a decomposed small file that stays in the zip.
…ill matches its archive entry
|
Pushed 77bacbe. Blocker 1: a page that names a decomposed file the way the disk lists it now matches again. I took the second option: while staging, each non-composed path segment (and its URL-encoded form) is rewritten to its composed spelling in the staged text files, so page and key agree and no twin entry can keep an offloaded heavy file in the zip. New test (a decomposed page, stylesheet and small file that stays in the zip) fails with that call disabled and passes with it; publishProject and offload tests pass (110). Blocker 2 (Comments) was fixed at 704e644 and this push adds no comment lines. |
There was a problem hiding this comment.
Approving at 77bacbee. Both blockers are resolved.
- Regression fixed.
composeNamesInTextrewrites decomposed path segments to NFC in every staged text file, plus theirencodeURIComponentforms. A decomposed page or stylesheet that names a decomposed small file now matches the NFC key that stays in the zip. The new test "rewrites a page and a stylesheet that name a decomposed file the way the disk lists it" is listed under 'Fails without the fix:'. I removed thecomposeNamesInText(...)call and that test failed (1 failed, 73 passed); with the call back,publishProject.test.tsandoffload.test.tspass (110 tests). - Comments ratchet: passes on this head.
Non-blocking:
- The rewrite is a plain text replace over whole text files, not just references, so matching caption or script text is recomposed too. That's canonically equivalent and almost always harmless, but it can change byte-sensitive content such as SRI hashes or a JS length check.
- Encoded references are matched only in
encodeURIComponentform. A name with&,,,+etc. written viaencodeURI, or with lowercase%cc%81, won't be rewritten. This is an edge case and fine to follow up.
CI (CI, CodeQL, Windows, preview-regression) was still running on this head when I posted this.
What
The cloud archive keys a file by the spelling the file system lists, and the render server looks files up byte for byte. A file name with an accent can be stored two ways: composed (one code point for é) or decomposed (e plus a combining accent). macOS lists a file picked in Finder decomposed, while a page almost always writes the name composed. The preview and a local render accept either spelling; the archive did not, so the page's reference matched no file in the zip and the offload step found nothing to rewrite.
The archive now uses one spelling (composed, NFC) for every key it makes: the project files, the files pulled in from outside the project, the entry composition's path, and the lookups the media offload makes when it matches a page's reference to a staged file.
Why
A cloud render of a project with such file names can fail as a missing video source even though the same project renders locally. On Linux the existing check already stops earlier with "The composition references ..., which is not in the project folder".
Related work
None found. I searched the CLI for
normalize(and found no Unicode normalization in the archive or offload code.How
archiveKey()inutils/publishProject.tsis the one place the spelling is decided;stagePublishFiles, the external-asset localizer, the referenced-file include and the entry path incommands/cloud/render.tsall key through it. Reading a file still uses the name the disk lists.cloud/offload.tsnormalizes the key before each lookup in the staged file map, so a page that names the file decomposed also matches.Two files that differ only in Unicode form now fail with a message naming both, instead of one silently replacing the other in the archive.
A page that names a file the way a decomposed disk lists it used to match its archive entry. Keys are now composed, so while staging, every non-composed path segment (and its URL-encoded form) is replaced by its composed spelling in the staged text files (html, css, js, json, svg, xml, txt). Page and key agree whichever spelling the page used.
Not proven: that every failing report involved decomposed names. The reports show names with accents and spaces, and this is the one cause I could reproduce.
.hyperframesignorepatterns are still matched against the spelling the disk lists.Test plan
Fails without the fix:
publishProject.test.ts"keys a file stored decomposed by the composed name a page uses, so the renderer finds it"publishProject.test.ts"refuses two files that differ only by Unicode form instead of letting one overwrite the other"offload.test.ts"offloads a file stored decomposed when the page names it composed, and the reverse"publishProject.test.ts"rewrites a page and a stylesheet that name a decomposed file the way the disk lists it" (a small file that stays in the zip)Unit tests added/updated
Manual testing performed (not run on a Mac with a Finder-picked file)
Documentation updated (if applicable)
Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug
Ran on Linux:
publishProject.test.tsandoffload.test.tspass with the fix; the four tests above fail without it; typecheck is clean for the touched files.render.test.tsandrender.reverseSession.test.tsfail the same tests with and without this change in my environment; the new entry-path key inrender.tshas no test of its own.