Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 47 additions & 3 deletions src/runtime/webcore/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,42 @@ fn data_url_response(data_url_: DataURL, global_this: &JSGlobalObject) -> JSValu
)
}

// ──────────────────────────────────────────────────────────────────────────
// file: URLs
// ──────────────────────────────────────────────────────────────────────────
Comment thread
robobun marked this conversation as resolved.
Outdated

/// The `Response` for a `file:` URL wraps a blob that opens the file lazily, so
/// `fetch()` itself has to check the path; otherwise a path that cannot be read
/// still gets a 200 and the error only surfaces from the body reader. Returns
/// the JS error for `fetch()` to reject with.
///
/// Only conditions under which the read is certain to fail are checked (the
/// path does not stat, or is a directory). `stat` rather than an eager `open`:
/// opening a FIFO or a device has side effects, and nothing here needs the fd.
/// Files embedded in a standalone executable come back as byte-backed blobs,
/// which have nothing on disk to check.
fn file_url_unreadable_error(file_blob: &Blob, global_this: &JSGlobalObject) -> Option<JSValue> {
let store = file_blob.store()?;
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();
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,
};
// `stat` attached the scratch copy of the path (`\\?\`-prefixed on
// Windows); report the path as the blob holds it.
Comment thread
robobun marked this conversation as resolved.
Outdated
Some(err.with_path(path.slice()).to_js(global_this))
}

// ──────────────────────────────────────────────────────────────────────────
// Bun__fetchPreconnect
// ──────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -1553,8 +1589,6 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
}
};

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.
Expand All @@ -1564,7 +1598,17 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
)),
);

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());
}

// A bare +1 ref that only `Response::init` below releases, so it is
// created after the early return above.
Comment thread
robobun marked this conversation as resolved.
Outdated
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(
Expand Down
15 changes: 15 additions & 0 deletions test/bundler/bundler_compile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,21 @@ describe("bundler", () => {
outfile: "dist/out",
run: { stdout: "Hello, world!" },
});
// fetch() rejects a file: URL whose path is not on disk. An embedded file
// only exists inside the executable, so it has to keep resolving.
itBundled("compile/FetchFileURLOfEmbeddedFile", {
compile: true,
files: {
"/entry.ts": /* js */ `
import { pathToFileURL } from "node:url";
import embedded from './foo.file' with {type: "file"};
const response = await fetch(pathToFileURL(embedded));
console.log(response.status, (await response.text()).trim());
`,
"/foo.file": `abcd`,
},
run: { stdout: "200 abcd" },
});
itBundled("compile/WorkerRelativePathNoExtension", {
backend: "cli",
compile: true,
Expand Down
43 changes: 26 additions & 17 deletions test/js/web/fetch/fetch-leak.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -526,28 +526,36 @@ test.concurrent(
async () => {
// The leaked impl is "file://<resolved abs path>", 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);
}
for (let i = 0; i < 200; i++) { try { await hit(-i); } catch {} }
for (let i = 0; i < 200; i++) await hit();
Bun.gc(true);
const baseline = rss();

const ITERS = 20000;
for (let i = 0; i < ITERS; i++) {
try { await hit(i); } catch {}
await hit();
if ((i & 1023) === 0) Bun.gc(true);
}
Bun.gc(true);
Expand All @@ -559,15 +567,16 @@ 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.
// ~0.85 KiB × 20000 ≈ 17 MiB raw leak (measured ~26 MiB on debug+ASAN)
// when the extra ref is dropped on the floor; ~11 MiB noise with the fix.
if (deltaMB > 20) {
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(":"),
Expand Down
60 changes: 60 additions & 0 deletions test/js/web/fetch/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
isWindows,
rss,
runFixtureMaxRSS,
tempDir,
tls,
tmpdirSync,
withoutAggressiveGC,
Expand All @@ -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();
Expand Down Expand Up @@ -1789,6 +1791,64 @@ it("fetch() file:// works", async () => {
expect(fileResponseText).toEqual(bunFileText);
gc(true);
});

describe.concurrent("fetch() file:// that cannot be read", () => {
// 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);

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)]) {
await expect(fetch(input)).rejects.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", {});

await expect(fetch(pathToFileURL(String(dir)))).rejects.toMatchObject({
code: "EISDIR",
syscall: "read",
path: reportedPath(String(dir)),
});
});

it.skipIf(isWindows)("still resolves for special files such as /dev/null", 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("ENOENT");
expect(stderr).toContain(reportedPath(missing));
expect(exitCode).toBe(1);
});
});

it("cloned response headers are independent before accessing", () => {
const response = new Response("hello", {
headers: {
Expand Down