Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
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
46 changes: 43 additions & 3 deletions src/runtime/webcore/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,39 @@ fn data_url_response(data_url_: DataURL, global_this: &JSGlobalObject) -> JSValu
)
}

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

/// The body blob opens the file lazily, so this is where `fetch("file:...")`
/// learns that the path cannot be read. Returns the rejection value, a
/// `TypeError` like every other `fetch()` failure (`ValueError::SystemTypeError`).
///
/// `stat`, not `open`: opening a FIFO or a device has side effects and the fd
/// is not needed. Non-file stores (files embedded in a standalone executable)
/// have nothing on disk to check.
Comment thread
robobun marked this conversation as resolved.
Outdated
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,
};
// 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
// ──────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -1553,8 +1586,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 +1595,16 @@ 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());
}

// +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(
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
120 changes: 120 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,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<unknown> {
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: {
Expand Down
Loading