fix: encode NTFS-reserved characters in route-derived artifact filenames#721
Open
YevheniiKotyrlo wants to merge 1 commit into
Open
fix: encode NTFS-reserved characters in route-derived artifact filenames#721YevheniiKotyrlo wants to merge 1 commit into
YevheniiKotyrlo wants to merge 1 commit into
Conversation
4 tasks
003bf43 to
e52f0fb
Compare
Dynamic-route patterns (/:param, /*) and concrete generateStaticParams values flow into generated artifact names: preload/loader assets via cleanUrl() and the static HTML path in buildPage. The characters < > : " | ? * are reserved on Windows filesystems - a colon-led segment fails the write outright and a mid-name colon silently creates an NTFS alternate data stream. The per-page build error is caught and logged as "skipping page" while the build exits 0, so on Windows every root-level (or route-group-rooted) dynamic route and every non-SSR dynamic route silently disappears from routeToBuildInfo and `one serve` returns a bare 404 for it; surviving nested +ssr routes write their preloads into invisible ADS streams that vanish from any copied or deployed dist. Encode reserved characters as =hh (hex char code, with literal = self-escaped first so decoding is unambiguous) in cleanUrl()'s filename transform and in the htmlPath artifact, and decode them in getPathFromLoaderPath(). The encoding is identity for paths without reserved characters, so artifact names for every currently-working route are byte-identical; serve-side postfix matching (PRELOAD_JS_POSTFIX_REGEX etc.) is unaffected. Validated on Windows 11: tests/test-app-cases dynamic-route-at-root and catch-all prod phases go 404 -> 200 (routes restored to the manifest, zero "skipping page" warnings) and test-spa-shell-routing goes from 8 failed prod tests to 27/27 in both dev and prod; packages/one vitest 534 passed with 11 new roundtrip/reserved-character tests. On Linux/macOS output is unchanged for clean paths, and pattern-named artifacts become portable (no more :/* in dist filenames for deploy tooling to reject).
e52f0fb to
b4f7ba7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #718.
Summary
Fixes the Windows build bug where dynamic routes silently disappear from prod builds (and
one servethen 404s them). Route patterns (/:param,/*) and concretegenerateStaticParamsvalues flow into generated artifact filenames — preload/loader assets viacleanUrl()and the static HTML path inbuildPage— and< > : " | ? *are reserved on Windows filesystems:build.tscatches it per-page, warns⚠ skipping page …: ENOENT, and exits 0 → the route is missing fromrouteToBuildInfo→ bare 404 in prod. This hits every root-level (or route-group-rooted, e.g.(group)/[param]) dynamic route, every non-SSR dynamic route at any depth (via…/:param.html), and every catch-all (via*).blog_+ hidden stream), vanishing from any copied/deployed dist.Two of the repo's own test packages catch this on Windows:
test-app-cases(dynamic-at-root + catch-all prod phases) andtest-spa-shell-routing(8 prod reload failures — exactly the four dropped routes). It's been masked becauseturbo testwithout--continuecancelled everything behind the first Windows failure.Changes
cleanUrl()encodes reserved characters as=hh(hex char code; literal=self-escaped first so decoding is unambiguous), andgetPathFromLoaderPath()decodes them — same module, so build naming, client fetch URLs, and server-side request decoding stay symmetric.buildPage'shtmlPathartifact gets the same encoding (the manifest stores the encoded name, so serve-side lookups are consistent by construction).PRELOAD_JS_POSTFIX_REGEXetc.) is untouched.cleanUrl.test.ts: roundtrips for patterns/catch-alls/hostile param values/literal=/hex-lookalikes, plus no-reserved-chars-emitted assertions.Test plan
Windows 11 (Bun 1.3.13), monorepo at v1.18.0:
tests/test-app-cases—dynamic-route-at-root-with-assetsandcatch-all-route-at-root-with-assetsprod phases go 404 → 200 (routes restored torouteToBuildInfo, zeroskipping pagewarnings); full package green in dev and prodtests/test-spa-shell-routing— 8 failed prod tests → 27/27 in both phases; all four previously-dropped routes (dashboard/[appId],thread/[id],(chat)/[serverId],(chat)/[serverId]/[channelId]) registered againpackages/onevitest — 534 passed / 0 failed (incl. the 11 new tests)oxlint0/0,oxfmt --checkclean,tsc --noEmitcleanOn Linux/macOS, output is unchanged for clean paths — and pattern-named artifacts become portable as a bonus (no more literal
:/*indist/filenames for deploy tooling to reject).Notes
Detailed diagnosis (NTFS ADS forensics, per-face scoping matrix, masking analysis) is in the linked issue. The behavioral question worth a maintainer call: pattern-built artifacts for non-prerendered dynamic routes now get stable encoded names — if you'd rather skip emitting those artifacts entirely for pattern paths, happy to rework in that direction instead.