diff --git a/src/runtime/webcore/fetch.rs b/src/runtime/webcore/fetch.rs index 2a41b66e2cc4..fd9d0ff0e8a8 100644 --- a/src/runtime/webcore/fetch.rs +++ b/src/runtime/webcore/fetch.rs @@ -224,6 +224,31 @@ fn data_url_response(data_url_: DataURL, global_this: &JSGlobalObject) -> JSValu ) } +/// The `TypeError` for `fetch("file:...")` to reject with when the blob's path cannot be read. +fn file_url_unreadable_error(file_blob: &Blob, global_this: &JSGlobalObject) -> Option { + let store = file_blob.store()?; + // A file embedded in a standalone executable comes back as a byte store; nothing to check. + let blob::store::Data::File(file) = &store.data else { + return None; + }; + let PathOrFileDescriptor::Path(path) = &file.pathlike else { + return None; + }; + + let mut path_buf = bun_paths::path_buffer_pool::get(); + // `stat`, not `open`: opening a FIFO or a device has side effects, and the fd is not needed. + let err = match bun_sys::stat(path.slice_z(&mut path_buf)) { + Ok(stat) if bun_sys::S::ISDIR(stat.st_mode as bun_sys::Mode) => { + bun_sys::Error::from_code(bun_sys::E::EISDIR, bun_sys::Tag::read) + } + Ok(_) => return None, + Err(err) => err, + }; + // Report the blob's path, not the `\\?\`-prefixed scratch copy `stat` attached. + let system_error: jsc::SystemError = err.with_path(path.slice()).to_system_error().into(); + Some(system_error.to_type_error_instance(global_this)) +} + // ────────────────────────────────────────────────────────────────────────── // Bun__fetchPreconnect // ────────────────────────────────────────────────────────────────────────── @@ -1553,8 +1578,6 @@ fn fetch_impl( } }; - url_string = jsc::URL::file_url_from_string(BunString::borrow_utf8(temp_file_path)); - // `find_or_create_file_from_path` is typed against the // `crate::webcore::node_types` stub (until it's swapped to a // re-export of `crate::node::types`); construct that variant here. @@ -1564,7 +1587,16 @@ fn fetch_impl( )), ); - break 'blob Blob::find_or_create_file_from_path(&mut pathlike, global_this, true); + let file_blob = Blob::find_or_create_file_from_path(&mut pathlike, global_this, true); + + if let Some(err) = file_url_unreadable_error(&file_blob, global_this) { + return Ok(JSPromise::rejected_promise(global_this, err).to_js()); + } + + // +1 ref released only by `Response::init`: must stay after the early return. + url_string = jsc::URL::file_url_from_string(BunString::borrow_utf8(temp_file_path)); + + break 'blob file_blob; }; let response = bun_core::heap::into_raw(Box::new(Response::init( diff --git a/test/bundler/bundler_compile.test.ts b/test/bundler/bundler_compile.test.ts index cdc71b4a9eeb..7d76c2a2dbcb 100644 --- a/test/bundler/bundler_compile.test.ts +++ b/test/bundler/bundler_compile.test.ts @@ -301,12 +301,18 @@ describe("bundler", () => { }, }); // https://github.com/oven-sh/bun/issues/8697 + // Also covers fetch() of the embedded file's file: URL: fetch() rejects a + // file: URL whose path is not on disk, and an embedded file only exists + // inside the executable, so it has to keep resolving. itBundled("compile/EmbeddedFileOutfile", { compile: true, files: { "/entry.ts": /* js */ ` + import { pathToFileURL } from "node:url"; import bar from './foo.file' with {type: "file"}; if ((await Bun.file(bar).text()).trim() !== "abcd") throw "fail"; + const response = await fetch(pathToFileURL(bar)); + if (response.status !== 200 || (await response.text()).trim() !== "abcd") throw "fetch fail"; console.log("Hello, world!"); `, "/foo.file": /* js */ ` diff --git a/test/js/web/fetch/fetch-leak.test.ts b/test/js/web/fetch/fetch-leak.test.ts index 5c9a9cf290d1..d4302147d981 100644 --- a/test/js/web/fetch/fetch-leak.test.ts +++ b/test/js/web/fetch/fetch-leak.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, tls as COMMON_CERT, gc, isASAN, isCI, isDebug } from "harness"; +import { bunEnv, bunExe, tls as COMMON_CERT, gc, isASAN, isCI, isDebug, tempDir } from "harness"; import { once } from "node:events"; import { createServer } from "node:http"; import net from "node:net"; @@ -526,29 +526,42 @@ test.concurrent( async () => { // The leaked impl is "file://", and fetch_impl decodes // url.path into a stack PathBuffer that is 1024 bytes on macOS/BSD, 4096 on - // Linux, ~98 KiB on Windows. Use a ~900-byte path so decode_into succeeds on - // every platform and url_string is actually assigned, with enough iterations - // for the small per-call leak to show in RSS. + // Linux, ~98 KiB on Windows. Use a ~850-byte path so decode_into succeeds on + // every platform, with enough iterations for the small per-call leak to + // show in RSS. The file has to exist: fetch() rejects for a path that does + // not stat before url_string is ever created. + using dir = tempDir("fetch-file-url-leak", {}); const script = /* js */ ` - const pad = Buffer.alloc(900, "a").toString(); - // Windows strips the leading "/" then asserts is_absolute_windows() in - // PosixToWinNormalizer under debug_assertions, which needs a drive letter. - const prefix = process.platform === "win32" ? "file:///C:/" : "file:///"; + import { mkdirSync, writeFileSync } from "node:fs"; + import { join } from "node:path"; + import { pathToFileURL } from "node:url"; + + const component = Buffer.alloc(200, "a").toString(); + const parent = join(process.cwd(), component, component, component, component); + mkdirSync(parent, { recursive: true }); + const file = join(parent, "file.txt"); + writeFileSync(file, "hello"); + const url = pathToFileURL(file).href; + const rss = process.platform === "darwin" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint : process.memoryUsage.rss; - async function hit(i) { - // Fresh path per iteration so each leaked ref pins a distinct impl. - // The file does not exist; the Response is created (with url_string set) - // and the lazy Blob body is never read, so no fs I/O happens. - await fetch(prefix + i + pad); + async function hit() { + // Every call builds a fresh url_string impl for the Response. The lazy + // Blob body is never read, so the Response is the only thing created. + await fetch(url); + } + // Warm up until the heap and allocator have reached their steady state, so + // the baseline is not taken while they are still growing. + for (let i = 0; i < 2000; i++) { + await hit(); + if ((i & 255) === 0) Bun.gc(true); } - for (let i = 0; i < 200; i++) { try { await hit(-i); } catch {} } Bun.gc(true); const baseline = rss(); const ITERS = 20000; for (let i = 0; i < ITERS; i++) { - try { await hit(i); } catch {} - if ((i & 1023) === 0) Bun.gc(true); + await hit(); + if ((i & 255) === 0) Bun.gc(true); } Bun.gc(true); const final = rss(); @@ -559,15 +572,18 @@ test.concurrent( finalMB: (final / 1024 / 1024) | 0, deltaMB: Math.round(deltaMB * 10) / 10, })); - // ~0.9 KiB × 20000 ≈ 18 MiB raw leak (measured ~32 MiB on debug+ASAN) - // when the extra ref is dropped on the floor; ~12 MiB noise with the fix. - if (deltaMB > 20) { + // Each leaked impl is ~0.9 KiB, so 20000 of them are ~17 MiB raw. Leaking vs + // fixed: 26-27 vs 8-10 MiB on debug+ASAN (extra ref re-added), 19-21 vs ~2 MiB + // on a release build (leak approximated by retaining one equal-sized string + // per call). + if (deltaMB > ${isASAN ? 18 : 12}) { throw new Error("fetch(file://) leaked " + deltaMB.toFixed(1) + " MB over " + ITERS + " iterations"); } `; await using proc = Bun.spawn({ cmd: [bunExe(), "--smol", "-e", script], + cwd: String(dir), env: { ...bunEnv, ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "quarantine_size_mb=0"].filter(Boolean).join(":"), diff --git a/test/js/web/fetch/fetch.test.ts b/test/js/web/fetch/fetch.test.ts index c0b1327b1fc4..5029fb749205 100644 --- a/test/js/web/fetch/fetch.test.ts +++ b/test/js/web/fetch/fetch.test.ts @@ -16,6 +16,7 @@ import { isWindows, rss, runFixtureMaxRSS, + tempDir, tls, tmpdirSync, withoutAggressiveGC, @@ -27,6 +28,7 @@ import type { AddressInfo } from "net"; import net from "net"; import { join } from "path"; import { Readable } from "stream"; +import { pathToFileURL } from "url"; import { gzipSync } from "zlib"; const tmp_dir = tmpdirSync(); @@ -1789,6 +1791,124 @@ it("fetch() file:// works", async () => { expect(fileResponseText).toEqual(bunFileText); gc(true); }); + +describe.concurrent("fetch() file:// that cannot be read", () => { + const isRoot = !isWindows && process.getuid?.() === 0; + // The file: branch keeps the URL's forward slashes in the path it reports, + // which only differs from path.join() on Windows. + const reportedPath = (p: string) => (isWindows ? p.replaceAll("\\", "/") : p); + + async function rejection(input: string | URL | Request): Promise { + const error = await fetch(input).then( + () => { + throw new Error("fetch() resolved"); + }, + (error: unknown) => error, + ); + // Every fetch() failure is a TypeError (a network error in fetch spec + // terms); the system error fields ride along on it. + expect(error).toBeInstanceOf(TypeError); + return error; + } + + it("rejects with ENOENT when the file does not exist", async () => { + using dir = tempDir("fetch-file-url", { "exists.txt": "exists" }); + const missing = join(String(dir), "missing.txt"); + const url = pathToFileURL(missing); + + for (const input of [url.href, url, new Request(url)]) { + expect(await rejection(input)).toMatchObject({ + code: "ENOENT", + syscall: "stat", + path: reportedPath(missing), + }); + } + + // A sibling that does exist is unaffected. + const response = await fetch(pathToFileURL(join(String(dir), "exists.txt"))); + expect([response.status, await response.text()]).toEqual([200, "exists"]); + }); + + it("rejects with EISDIR when the path is a directory", async () => { + using dir = tempDir("fetch-file-url-dir", {}); + + expect(await rejection(pathToFileURL(String(dir)))).toMatchObject({ + code: "EISDIR", + syscall: "read", + path: reportedPath(String(dir)), + }); + }); + + it.skipIf(isWindows || isRoot)("rejects with EACCES when a parent directory cannot be searched", async () => { + using dir = tempDir("fetch-file-url-eacces", { "locked/file.txt": "secret" }); + const locked = join(String(dir), "locked"); + const file = join(locked, "file.txt"); + + chmodSync(locked, 0o000); + try { + expect(await rejection(pathToFileURL(file))).toMatchObject({ + code: "EACCES", + syscall: "stat", + path: reportedPath(file), + }); + } finally { + chmodSync(locked, 0o755); + } + }); + + it.skipIf(isWindows || isRoot)("leaves a permission check on the file itself to the body read", async () => { + // Only what stat() reports is checked before the Response is created. + // access(2) can disagree with open(2), so a file that stats but cannot be + // opened still resolves and the open error comes from the body, as before. + using dir = tempDir("fetch-file-url-mode", { "file.txt": "secret" }); + const file = join(String(dir), "file.txt"); + chmodSync(file, 0o000); + + const response = await fetch(pathToFileURL(file)); + expect(response.status).toBe(200); + await expect(response.text()).rejects.toMatchObject({ + code: "EACCES", + syscall: "open", + path: reportedPath(file), + }); + }); + + it.skipIf(isWindows)("resolves for a FIFO without opening it", async () => { + using dir = tempDir("fetch-file-url-fifo", {}); + const fifo = join(String(dir), "pipe"); + mkfifo(fifo); + + // Nothing ever writes to the pipe, so an implementation that opened the + // path up front instead of stat()ing it would block here. The body is + // deliberately left unread. + const response = await fetch(pathToFileURL(fifo)); + expect(response.status).toBe(200); + }); + + it.skipIf(isWindows)("resolves for a character device", async () => { + const response = await fetch("file:///dev/null"); + expect([response.status, await response.text()]).toEqual([200, ""]); + }); + + it("reports the rejection as unhandled when nothing catches it", async () => { + using dir = tempDir("fetch-file-url-unhandled", {}); + const missing = join(String(dir), "missing.txt"); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `fetch(${JSON.stringify(pathToFileURL(missing).href)});`], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stdout).toBe(""); + expect(stderr).toContain("TypeError: ENOENT"); + expect(stderr).toContain(reportedPath(missing)); + expect(exitCode).toBe(1); + }); +}); + it("cloned response headers are independent before accessing", () => { const response = new Response("hello", { headers: {