[vitest-pool-workers] Fix module fallback failure on non-ASCII workspace paths (Windows) - #14713
Merged
petebacondarwin merged 14 commits intoJul 20, 2026
Conversation
… the module fallback redirect response Fixes #14655. On Windows, a workspace path containing non-ASCII characters (e.g. CJK characters) caused `buildRedirectResponse()` to set a raw non-ASCII file path directly as an HTTP `Location` header value. HTTP headers are restricted to the Latin-1/ASCII byte range, so undici's `ByteString` conversion throws (`Cannot convert argument to a ByteString because the character at index N has a value of NNNNN which is greater than 255`), which surfaces to users as a misleading "No such module" runtime-start failure — since the very first module resolved via this path is the pool's own `cloudflare:test-internal` import. This mirrors the fix already applied for the same class of bug in #10774 (a raw path set directly as an HTTP header value), just at a different call site. `encodeURI()` is used rather than `encodeURIComponent()` so that path separators and the Windows drive-letter colon (e.g. `/C:/a/b/c`) remain intact, matching how `ensureRootedPath()` already expects paths to look; only the actually-invalid header bytes (non-ASCII characters) get percent-encoded.
…d when building the redirect Location header Half-fix follow-up: workerd carries the Location header value through verbatim as the next request's `specifier` param rather than URI-decoding it, so without this the encodeURI()'d path from the previous commit comes back containing literal percent-escapes instead of the real (non-ASCII) characters, and file resolution fails with "Not found". Verified end-to-end on Windows CI against the real repro (see PR description).
…rom the decoded one used for filesystem resolution
Second follow-up fix: workerd tracks the in-flight module request by the exact literal `specifier` string it sent (our encodeURI()'d Location value from the previous redirect), and rejects the response if the JSON module's `name` field doesn't match that literal string exactly ("returned module name does not match specifier"). Decoding `target` unconditionally (previous commit) broke this, since `name` is built from `target`. This threads a separate `rawTarget` (undecoded, but with the same Windows leading-slash handling) through to `load()`/`buildModuleResponse()` for the `name` field, while all internal filesystem resolution continues to use the decoded `target`. Verified end-to-end on Windows CI against the real repro.
… non-ASCII cwd Third and final piece of the #14655 fix: this header carries the JSON-serialized project cwd, which is restricted to the Latin-1/ASCII byte range same as any HTTP header, but a non-ASCII workspace path breaks it the same way the Location header did. Verified end-to-end on Windows CI: the real repro now runs its test file, past this header, with no further ByteString errors.
…atch the encodeURIComponent() on write
…e we intentionally return in buildModuleResponse's identity field Fifth piece of the #14655 fix. Since buildModuleResponse's `name` must stay raw/encoded to satisfy workerd's specifier matching (previous commit), that raw identity becomes the referrer for every import statement inside that module, propagating the encoded form forward to all subsequent fallback requests. Decoding referrer the same way as target fixes filesystem resolution for imports made from any module whose own name required encoding. Verified end-to-end: this was the actual cause of the next failure encountered on Windows CI (vitest/dist/worker.js failing to import @vitest/utils/source-map).
🦋 Changeset detectedLatest commit: d47d872 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
workers-devprod
requested review from
a team and
petebacondarwin
and removed request for
a team
July 16, 2026 05:41
Contributor
|
Codeowners approval required for this PR:
Show detailed file reviewers |
![devin-ai-integration[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…al '%' in workspace paths Devin Review flagged that decodeURI() was applied unconditionally to the specifier and referrer before the try/catch that's supposed to handle errors. A workspace path containing a literal '%' that was never encodeURI()-produced (e.g. "50%off") throws URIError on decode, which was uncaught here and crashed the whole handler instead of falling through to the existing 404 response. Add a safeDecodeURI() helper that catches URIError and falls back to the raw value, so malformed/never-encoded paths degrade to a 404 instead of an unhandled rejection.
Contributor
Author
|
Thanks @devin-ai-integration[bot] — good catch. Fixed in f944dfc: Note this addresses the throw case specifically. The silent-corruption case (a literal |
@cloudflare/autoconfig
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… encoding The previous follow-up blindly `decodeURI()`'d every specifier/referrer and caught the `URIError` that a literal `%` (e.g. `50%off`) would throw. That only handled the *throwing* case: a real workspace path segment that happens to look like valid percent-encoding (e.g. `build%20output`) was silently decoded to the wrong path, and the "is this one of our own encoded values?" decision was fundamentally a guess. We own the only place these values are ever encoded (`buildRedirectResponse()`), so mark our own output instead of guessing: - `encodeRedirectLocation()` percent-encodes only paths that aren't header-safe and tags them with a rooted `/__mf_vitest_encoded__` sentinel. - `decodeEncodedSpecifier()` decodes iff that sentinel is present; everything else (bare `cloudflare:*`/`node:*` specifiers, untouched original paths, paths with a literal `%`) passes through verbatim. This makes the change a strict superset of the pre-PR behaviour and removes any chance of mis-decoding a real `%`. The sentinel is a leading, rooted segment on purpose: workerd derives a relative import's specifier by joining it onto the referrer's directory (`kj::Path::eval`), which drops the final path segment but keeps the leading ones, so a trailing marker wouldn't survive for imports made from an encoded module. Literal `%` is now escaped before encoding so a path mixing non-ASCII and a literal `%` round-trips losslessly (which `encodeURI()`/`decodeURI()` can't do). Adds unit tests driving the exported `handleModuleFallbackRequest` and the encode/decode helpers directly, covering: literal-`%` pass-through (the silent-corruption case the old code got wrong), non-ASCII and mixed non-ASCII+`%` round-trips, astral characters, Windows drive-letter shape, a non-ASCII redirect Location that no longer throws, an echoed sentinel resolving the real file, and the 404 fall-through. `vitest.config.mts` gains a `define` stub for the build-time builtin-modules global so these source-importing unit tests can run.
petebacondarwin
force-pushed
the
fix/14655-nonascii-fallback-redirect-header
branch
from
July 17, 2026 14:14
a2a1006 to
b0a45bc
Compare
Co-authored-by: Pete Bacon Darwin <pete@bacondarwin.com>
petebacondarwin
approved these changes
Jul 20, 2026
workers-devprod
approved these changes
Jul 20, 2026
workers-devprod
left a comment
workers-devprod
left a comment
Contributor
There was a problem hiding this comment.
Codeowners reviews satisfied
Merged
5 tasks
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
3 participants
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 #14655
Problem
On Windows, when a project's workspace path contains non-ASCII characters (e.g. CJK characters like
開発),@cloudflare/vitest-pool-workersfails at startup with:before any tests run.
Root cause
The module fallback service's redirect response (
buildRedirectResponse()inmodule-fallback.ts) sets the target file path directly as the HTTPLocationheader value. HTTP headers are restricted to the Latin-1/ASCII byte range (enforced by undici'sByteStringconversion), so any non-ASCII byte in the path throws:This is the same anti-pattern previously fixed in #10774 for a different code path (
WORKER_ENTRY_PATH_HEADERin vite-plugin-cloudflare), just a different call site.Root cause confirmed against a real repro: forked gotisgate/workers-vitest-nonascii-path-repro and added a
windows-latestCI workflow that recreates akaihatsu-開発path and runs the test suite withNODE_DEBUG=vitest-pool-workers, iterating until green.Fix
Five coordinated changes were needed, all in
packages/vitest-pool-workers:module-fallback.tsbuildRedirectResponse()— percent-encode the file path withencodeURI()before setting it as theLocationheader.module-fallback.tshandleModuleFallbackRequest()— workerd passes theLocationvalue through verbatim as the next request'sspecifier(it doesn't decode it), so decode it back withdecodeURI()for filesystem resolution.module-fallback.tsbuildModuleResponse()/load()— workerd tracks the in-flight request by the exact (still-encoded) specifier string and rejects the response if the module'snamefield doesn't match exactly, so a secondrawTarget(undecoded) value is threaded through for thenamefield, while the decodedtargetis used for filesystem operations.module-fallback.ts— the encodednamebecomes the referrer for every import inside that module, soreferrergets the samedecodeURI()treatment astarget.pool/index.tsconnectToMiniflareSocket()+worker/index.ts— theMF-Vitest-Worker-Dataheader also embeds a raw (potentially non-ASCII)process.cwd()value; same class of bug, fixed withencodeURIComponent()/decodeURIComponent().Verification
Ran the patched package against the real repro on Windows CI (workflow at
ci/repro-windows.ymlin the forked repro repo), confirming a clean run on the actualkaihatsu-開発path:Latest green run: https://github.com/dumbCodesOnly/workers-vitest-nonascii-path-repro/actions/runs/29473133807
Two changesets are included, one per distinct header-encoding bug fixed (redirect
Locationheader, andMF-Vitest-Worker-Dataheader).