Repository navigation
Conversation
…d path For a browser, `url(./my%20photo.png)` in a stylesheet names the file `my photo.png`. The bundler looked up the text as written and the build failed with "Could not resolve". When a `url()` or `@import` of a CSS file does not resolve as written, both bundler resolve sites now try the path with its percent escapes decoded (`percent_decoded_css_url`). A file with the literal name still wins. An escape stays as written when the resolver or a browser reads its byte another way: `%2F`, `%5C`, a decoded `#`, `?`, `%`, `"`, control byte or trailing space, bytes that are not UTF-8, and a `:` in the first path segment. HTML `src`/`href` and `composes` are not decoded. Under a watcher, the directory of the decoded path is dropped from the cache and watched like the directory of the literal text. The failure log is fetched after the retries, because the dev server marks the file stale when it hands out that log. The resolver treats ENAMETOOLONG as a missing file when it reads a directory. A long name written as escapes logged "Cannot read directory", and that error failed the build. The two resolve loops bind their result with `let mut` directly. With the second binding, the new `break` added one copy of the Result for each record that resolves.
|
Status: ready for review. How I reproduced it, on 1.4.2 and on main (38afee6): head -c 140000 /dev/zero | tr '\0' x > 'my photo.png'
printf 'a { background: url(./my%%20photo.png) }\n' > enc.css
bun build ./enc.css --outdir o
# error: Could not resolve: "./my%20photo.png"With this branch the build writes |
|
Updated 5:12 AM PT - Oct 22nd, 2026
⏳ @robobun, your commit 2613436 is still building in
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe resolver now decodes eligible percent-encoded CSS URL paths after literal-path resolution fails. The bundler retries resolution and updates failure tracking for both spellings. New tests cover bundling, output, platform-specific cases, and development-server reloads. ChangesCSS URL resolution
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: resolving percent-encoded CSS url() and @import paths by their decoded path. |
| Description check | |
The description provides the problem, implementation details, verification results, limitations, and testing scope. It does not use the template headings exactly, but it covers the required informatio… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
-
Explanation
The description provides the problem, implementation details, verification results, limitations, and testing scope. It does not use the template headings exactly, but it covers the required information in detail.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the lifetime of the decoded slice in percent_decoded_css_url (it is allocated in self.graph.heap, which outlives the Result that may borrow it, and track_resolution_failure joins the specifier into its own path buffer rather than retaining the slice), and the relocated log_for_resolution_failures call in run_resolver (it is still reached on every error path that logs, since the only paths that skip it continue or break out of the loop before any error is recorded).
Extended reasoning...
The change adds a percent-decoding retry for CSS url()/@ import resolution in both bundler resolve loops, exports a new helper from the resolver crate, and widens two directory-read error branches to swallow ENAMETOOLONG; it touches no auth, crypto, or data-exposure surface, but it does turn a build failure into a passing build with changed output, and five inline findings cover the output, dev-server watch, and errno-swallowing consequences, so a human should weigh them.
Findings marked 🟡 are optional suggestions and need no follow-up push.
| if strings::contains_any(decoded, b"#?%") | ||
| // The output is the raw file name, which cannot carry these to a browser. | ||
| || strings::contains_char(decoded, b'"') | ||
| || decoded.iter().any(u8::is_ascii_control) | ||
| || !strings::is_valid_utf8(decoded) | ||
| || strings::ends_with_char(decoded, b' ') | ||
| // A `:` here reads as a scheme or a drive letter. | ||
| || strings::contains_char(first_segment, b':') | ||
| { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
🔴 Users running bun build --minify on url(./my%20photo.png) now get a passing build whose rule the browser drops, where the base failed with Could not resolve. The guard at src/resolver/resolver.rs:6958 refuses " and control bytes because the copied asset's name is substituted verbatim into the printed url(), yet accepts space, ', ( and ), which the unquoted minified form cannot carry either. Fix: make the emitted url() valid for every byte the decoder accepts in both quoted and unquoted output, e.g. escape at the asset substitution in src/bundler/Chunk.rs:781, or until then refuse those bytes here as " is. The PR calls this accepted; it turns a loud build error into silently broken production CSS. [also at: src/resolver/resolver.rs:6967 - Users bundling a CSS url() to a copied asset with --minify now get a build that passes but a rule the browser drops, where the base failed loudly.]
Why this was flagged
A stylesheet contains url(./my%20photo.png) (or any name whose decoded form has a space, ', ( or )) and the build runs with --minify/minifyWhitespace. percent_decoded_css_url at src/resolver/resolver.rs:6956-6966 accepts the decoded ./my photo.png. With minify, src/css/values/url.rs:110-140 prints the asset placeholder unquoted, and src/bundler/Chunk.rs:769-781 substitutes dest_path verbatim, so the output is url(./my photo-<hash>.png). Per CSS Syntax an unquoted url token with whitespace followed by a non-) is a bad-url token, so the declaration is dropped. On the base branch the same input fails at build time with Could not resolve: "./my%20photo.png", so the author sees it. The guard's own comment at src/resolver/resolver.rs:6957 says bytes the raw output name cannot carry are refused, but that criterion is applied only to ", control bytes and a trailing space, not to the bytes serialize_unquoted_url (src/css/css_parser.rs:5842-5844) must escape.
Verification: Acknowledged in the PR description's "Downsides" and a pinned test. percent_decoded_css_url (src/resolver/resolver.rs:6956-6966) accepts ./my photo.png; src/css/values/url.rs:110-124 prints the placeholder unquoted and src/bundler/Chunk.rs:769-781 substitutes dest_path verbatim, yielding url(./my photo-<hash>.png), a bad-url token the browser drops, where the base printed Could not resolve.
| if let Some(decoded) = decoded | ||
| && let Ok(result) = transpiler.resolver.resolve( | ||
| source_dir, | ||
| decoded, | ||
| import_record.kind, | ||
| ) | ||
| { | ||
| break 'inner result; |
There was a problem hiding this comment.
🔴 Users serving a bundled HTML page with Bun.serve whose CSS has url(./my%20photo.png) to a copied asset now get a 404 for it where the base failed the build. The decoded hit adopted at src/bundler/bundle_v2.rs:7011 (and :2796) names the output ./my photo-<hash>.png, so HTMLBundle.rs:584 registers the route /my photo-<hash>.png. The browser requests /my%20photo-<hash>.png and the uws router matches segments literally, so nothing answers. Fix: a decoded hit must produce an output name a browser can fetch back, e.g. percent-encode the copied asset's name in the emitted CSS and route, or refuse decoded names the server cannot route.
Why this was flagged
A stylesheet in a Bun.serve HTML bundle (production, Bun.serve({ routes: { '/': index.html } })) contains url(./my%20photo.png) and the file on disk is my photo.png of 128 KiB or more (COPY_THRESHOLD, src/bundler/bundled_ast.rs:374) or uses the file loader. The raw resolve fails with ModuleNotFound; percent_decoded_css_url returns the decoded name and the second resolve at src/bundler/bundle_v2.rs:7005-7011 succeeds, so the record's path becomes /dir/my photo.png and the copied output file is ./my photo-<hash>.png. src/runtime/server/HTMLBundle.rs:571-588 strips the . and calls append_static_route with /my photo-<hash>.png. The browser percent-encodes the space in the request path, /my%20photo-<hash>.png; packages/bun-uws/src/HttpContext.h:593 routes getUrlForRouting() unchanged and HttpRouter.h:255 compares segments byte-for-byte without decoding, so the static route never matches and the server falls to its 404/fallback. On the base the build itself failed with Could not resolve: "./my%20photo.png", so the user saw the problem at build time.
Verification: src/bundler/bundle_v2.rs:7005-7011 break 'inner result on the decoded second resolver.resolve(source_dir, decoded, ...), so the record now resolves to my photo.png where base raised Could not resolve. A browser resolving url("./my photo-<hash>.png") requests /my%20photo-<hash>.png, which does not byte-equal /my photo-<hash>.png, so no static route fires and the asset 404s.
| if !matches!( | ||
| err, | ||
| crate::Error::Sys(bun_errno::SystemErrno::ENOENT) | ||
| | crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG) | ||
| ) { |
There was a problem hiding this comment.
🟡 (optional) Users of bun build and the dev server lose the Cannot read directory "...": ENAMETOOLONG diagnostic for every import kind, not only the CSS percent-decoded retry. Both directory-read sites (src/resolver/resolver.rs:4580-4584 and src/resolver/resolver.rs:5911-5913) now treat ENAMETOOLONG like ENOENT, so a JS import whose directory exceeds NAME_MAX or whose absolute path exceeds PATH_MAX reports only Could not resolve. Fix: keep the errno visible for the non-CSS case, for example by emitting it as a non-fatal note instead of dropping it, or by silencing it only when the probe comes from a CSS url()/@ import record that has a % escape; the decoded retry must still succeed without a logged error.
Why this was flagged
A JS file does import "./<300 a>.js" or lives in a tree whose absolute path is longer than PATH_MAX (such a file can exist on Linux). read_dir_info reaches the ENAMETOOLONG branch at src/resolver/resolver.rs:4580 or load_as_file reaches src/resolver/resolver.rs:5911, and with this diff both skip add_error_fmt for ENAMETOOLONG. The base branch prints Cannot read directory "<dir>": ENAMETOOLONG followed by Could not resolve; after merging only Could not resolve is printed, so an operator cannot tell a missing file from a path-length limit. The silencing is needed only so the raw probe of an escaped CSS name (29 CJK letters become a 261-byte segment) does not fail the build before the decoded retry at src/bundler/bundle_v2.rs:2790 and src/bundler/bundle_v2.rs:7003 succeeds, but the change applies to every kind and every caller of the resolver. The repository review guidance says to map only the specific expected errno to the benign path; nothing in the diff bounds this to CSS records.
Verification: The trigger is any importer whose specifier or resolved directory path makes openat/readdir return ENAMETOOLONG. src/resolver/resolver.rs:4580-4584 now reads if !matches!(err, Sys(ENOENT) | Sys(ENAMETOOLONG)), and src/resolver/resolver.rs:5911-5913 adds | Sys(ENAMETOOLONG) => {} to the silent arm. Neither site consults the import kind. The build still fails with Could not resolve; only the extra logged error is dropped.
| ); | ||
| } | ||
| if busted { | ||
| had_busted_dir_cache = true; | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Tell Bake's Dev Server to wait for the file to be imported. | ||
| dev.track_resolution_failure( | ||
| &import_record.source_file, | ||
| &import_record.specifier, | ||
| target.bake_graph(), | ||
| self.graph.input_files.items_loader() | ||
| [import_record.importer_source_index as usize], | ||
| ) | ||
| .expect("oom"); | ||
| for spelling in spellings { |
There was a problem hiding this comment.
🟡 (optional) Dev-server users with a percent-encoded CSS url() whose file appears later keep an OS directory watch and a dead dependency entry for the rest of the session, where the base released both once the file appeared. Both spellings are tracked at src/bundler/bundle_v2.rs:2811-2820 (and bundle_v2.rs:7040-7048), but hot_reload_event.rs:140-152 frees only the dep that resolves and frees the watcher only when no dep remains; the raw ./my%20photo.png dep never resolves, so the watcher on that directory is never freed and the raw spelling is re-resolved on every event in that directory. Fix: when one spelling of a record resolves, free the sibling deps registered for the same (source file, record), e.g. track the decoded spelling only, or key deps by record so a hit removes both.
Why this was flagged
Trigger: Bake dev server, a stylesheet with url(./my%20photo.png) while my photo.png does not exist yet, then the user creates the file. Both spellings are passed to dev.track_resolution_failure at src/bundler/bundle_v2.rs:2811-2820 and src/bundler/bundle_v2.rs:7040-7048; directory_watch_store.rs:257-287 appends one Dep per spelling to the same directory watcher. On the create event, src/runtime/bake/dev_server/hot_reload_event.rs:129-147 resolves each Dep; only the decoded one resolves and is freed (line 147), the raw one is pushed back onto the chain (lines 149-151), so the watcher at line 155-161 is never freed. On the base branch there was only the raw dep, which stayed only while the import was genuinely unresolved; here the import now resolves but the directory watch and the stale dep persist for the session and every later change in that directory re-runs resolver.resolve on the raw spelling. Remedy: free sibling deps of the same record when any spelling resolves.
Verification: Trigger: Bake dev server, url(./my%20photo.png) missing at first build, my photo.png created later. On the directory event, only a dep whose resolve succeeds is freed (src/runtime/bake/dev_server/hot_reload_event.rs:147), every other dep is re-linked (lines 150-151). ./my%20photo.png never resolves once my photo.png exists, so that dep and the directory watch persist.
| let decoded = _resolver::percent_decoded_css_url( | ||
| self.arena(), | ||
| loader, | ||
| import_record.path.text, | ||
| import_record.kind, | ||
| ); | ||
| if let Some(decoded) = decoded | ||
| && let Ok(result) = transpiler.resolver.resolve( | ||
| source_dir, | ||
| decoded, | ||
| import_record.kind, | ||
| ) | ||
| { | ||
| break 'inner result; | ||
| } |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: pre-existing (partial fix): a Bun.build({ files }) user whose in-memory CSS references url(./my%20photo.png) beside an in-memory my photo.png still gets Could not resolve, while the same layout on disk now builds. Both resolve sites consult file_map.resolve only for the raw spelling (src/bundler/bundle_v2.rs:6926, src/bundler/bundle_v2.rs:2696) and the decoded retry at src/bundler/bundle_v2.rs:7005 goes to the disk resolver only. Fix: try the decoded spelling against the FileMap before (or alongside) the disk resolver at both sites, ideally inside one shared retry helper so the two loops stay in lockstep. [also at: src/bundler/bundle_v2.rs:7003 - pre-existing, partial fix: users of Bun.build({ files }) whose in-memory CSS says url(./my%20photo.png) still get Could not resolve after this change.]
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
A JS API build passes files: { "/app/entry.css": ".a{background:url(./my%20photo.png)}", "/app/my photo.png": <bytes> } with /app/entry.css as entrypoint. In resolve_import_records the FileMap lookup at src/bundler/bundle_v2.rs:6926 is keyed by the raw text ./my%20photo.png and misses; resolve_with_framework at src/bundler/bundle_v2.rs:6989 misses on disk; the new retry at src/bundler/bundle_v2.rs:6998-7012 decodes to ./my photo.png but only calls transpiler.resolver.resolve, which reads the real filesystem, so the virtual file is never consulted and the record falls through to the Could not resolve: "./my%20photo.png" error at src/bundler/bundle_v2.rs:7175. run_resolver has the same shape: FileMap at src/bundler/bundle_v2.rs:2696 for the raw text only, decoded retry at src/bundler/bundle_v2.rs:2782-2797 against the disk resolver only. The base branch fails this input the same way, so this is the sibling input source the fix leaves behind rather than a regression; the PR's variant matrix (api/cli/plugin-declines) does not include files.
Verification: pre-existing (partial fix: the base branch fails this by the same route). A Bun.build({ files: {...} }) build whose in-memory CSS references url(./my%20photo.png). FileMap::resolve (src/bundler/bundle_v2.rs:968-1060) never percent-decodes. The decoded retry at lines 6998-7012 calls only the disk resolver, so the build logs Could not resolve. None of the new tests exercise it.
Problem
bun buildfails on a CSSurl()or@importthat a browser loads:url(./my%20photo.png)besidemy photo.pnggiveserror: Could not resolve: "./my%20photo.png". No user reported this.src/resolver/resolver.rs:1427). It decodes only a packageexportstarget.Fix
percent_decoded_css_urldecodes the path. Both bundler resolve sites try it after the text as written fails, so a literala%20b.pngstill wins. CSS importers only.resolveor a browser reads its byte another way:%23,%3F,%25,%22,%2F,%5C, control bytes.test/bundler/esbuild/css.test.ts(16 tests, 11 fail on main),test/bake/dev/css.test.ts(4, all fail). Self-reviewed: 20 concerns raised, 18 addressed.Background
resolve_import_recordsandrun_resolver(records anonResolveplugin declines).resolve_and_auto_install: it cannot tell a CSS importer from an HTML one, and every runtimeimportenters it.Downsides
--minifyor a productionBun.servepage, a name with a space now builds but itsurl()does not load. The raw spelling does the same on main. css: quote and escape the asset path spliced into url() #41557 and bundler(html): percent-encode output paths written into src and href, and serve them under that name #41793 fix both..text: +2,048 bytes.Cannot read directory ...: ENAMETOOLONG.Notes
Repro (1.4.2 and main 491aaad, linux x64):
With this change the build writes
url("./my photo-8j4pkn61.png"),url("./Geist[wght]-8j4pkn61.woff2")andurl("data:image/svg+xml;base64,eA==#m").Order of the tries. For a record of a CSS file:
?query#fragment. Steps 1 and 2 are on main, inside the resolver.Step 3 is a full second resolve. So
--external, tsconfigpaths, thebrowsermap and a package withoutexportstreat the decoded path as they treat the raw spelling. A build can pass with no file on disk when the decoded path is external (css/URLPercentEncodedExternal). The suffix is not put back for step 3. The printer appends it from the text as written.What stays as written.
%2Fand%5C, by the rule thatESModule::finalizeapplies to anexportstarget (INVALID_PERCENT_CHARS). The resolver reads\as a separator on every platform.#,?or%.resolvecuts at#and?when the name is not a file, and anexportsmap decodes%once more. A browser does the same with the raw output name.", control byte, trailing space, or byte sequence that is not UTF-8. The output is the raw file name. css: quote and escape the asset path spliced into url() #41557 escapes"and control bytes in the CSS string. The others need the output path to be percent-encoded, which no PR does yet for CSS.:in the first path segment, which reads as a scheme or a drive letter. SoC:/dir/my%20photo.pngstays as written on Windows.100%.png).Dot segments are decoded:
%2e%2e/is the parent directory, as for a browser.Dev server and
--watch. Each pass tries the literal text and then the decoded path. If both miss, the directories of both spellings are dropped from the cache once, and the pass runs again. Then both spellings go totrack_resolution_failure, so a file that is created later at the decoded path rebuilds the stylesheet. The failure log is now fetched after the retries, becauselog_for_resolution_failuresmarks the file stale in the dev server. A decoded hit drops no directory from the cache: 3 rebuilds of a stylesheet with 5 encoded URLs gave 0 drops, as for the raw spelling.Loop result binding. Both loops now bind their result with
let mut. With the old second binding (let mut resolve_result = resolve_result;), the newbreakmade the compiler copy the 296-byteResultonce more for each record that resolves. The numbers are below.ENAMETOOLONG. 29 CJK letters are a 261-byte path segment when written as escapes. The probe of that text logged
Cannot read directory "...": ENAMETOOLONG, and a logged error fails the build even when the decoded name resolves. Both read sites now treatENAMETOOLONGlikeENOENT. Forimport "./<300 a>.js",bun buildprinted that line andCould not resolve. It now printsCould not resolveonly.require()of such a path givesMODULE_NOT_FOUNDbefore and after.Measurements. Release builds of main 491aaad without and with this diff, linux x64. Tools: gdb hit counts and single-step counts,
llvm-size,llvm-nm,llvm-objdump.perf,strace,valgrindandbloatyare not installed here, so there is no heap-block count..text107,392,716 to 107,394,764 bytes (+2,048).dataandbssare equal. The stripped file is 107,566,816 bytes for both. The new function is 775 bytes.resolve_import_records+472,on_resolve+319,load_as_file+30,dir_info_cached_miss+5.resolve_and_auto_install,resolve,resolve_with_framework,resolve_without_symlinks,check_relative_pathandESModule::finalize: same size, 0 instructions differ.url()records that resolve, counted insideresolve_import_records: calls 2,803 to 2,803, instructions 72,234 to 73,436 (+6 per record, register reloads). With the old second binding: 3,003 calls, one more 296-bytememmoveper record.onResolveplugin that declines, counted insideon_resolve:memmovecalls 402 to 402. The compiler no longer inlinespath_with_pretty_initializedthere: +2 calls and 37 fewer instructions inside the function per record.url()without%: resolver calls 1,001 to 1,001.percent_decoded_css_urlruns once per failed record and returns before any allocation.#suffix). Raw, literal-%and plain names: 101 calls and 151 syscalls before and after.%(16, 16, 27 and 18 bytes for the repro).Tests.
css.test.tsfail. The other 5 pin what must not change: the refused escapes,composes, and HTMLsrc/href. Each gate and each refusal was removed in a separate build, and at least one test failed each time. The same holds for the order of the tries at each site, the suffix, bothENAMETOOLONGsites, and the decoded cache drop and watch at each site.css/URLPercentEncodedEveryByte(Linux) builds one file per byte with both hex cases of its escape. It pins the accepted set, and checks withnew URLthat a URL parser gives each accepted byte back.css/URLPercentEncodedFileLoaderMinifiedpins today's unquoted output for a name with a space. css: quote and escape the asset path spliced into url() #41557 changes that expectation.src="./my%20photo.png"keeps its error. bundler(html): decode character references and percent escapes in src/href URLs #41616 changes that expectation.test/bundler/esbuild/css.test.tsandtest/bake/dev/{css,html,bundle,esm,hot}.test.ts,bundler_html,css-modules,bundler_plugin,test/js/bun/resolve/resolve.test.ts,resolve-error.test.ts, the long-path tests ofbun-build-api.test.ts, thebyte-searchandcomment-copsource lints,cargo clippyforbun_resolverandbun_bundler, andcargo checkof both crates forx86_64-pc-windows-msvcandaarch64-apple-darwin.Other open PRs on the same lines or rules. This PR does not depend on any of them.
ResultFlags::REMOVED_URL_SUFFIX. The decoded retry cuts the suffix outside the resolver, so the second of the two PRs to land sets the flag for a decoded hit whose text had a suffix. The#mand?v=2#fragrows fail without it. Both PRs edit the imports ofcss.test.ts, so git reports a conflict.?queryretry to the same two arms, and one module per query. The second to land orders the two retries. The metafile assertion incss/URLPercentEncoded*checks that each file stays one input.run_resolver. The second call site goes with it.src/hrefin the scanner, with the same refused list and the decoded name first.url(./my%20sheet.css)) prints the absolute path, as the raw spelling does on main.Bun.serve404.Not fixed here.
filesofBun.buildmatch by exact key. They do not have the?query#fragmentretry either.%23,%3F,%25or%22.src/hrefandcomposes ... from.Self-review. 20 concerns raised, 18 addressed. Not taken:
let mut resolve_resultbinding. Without the change each resolved record copies theResultonce more.