Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
7 changes: 5 additions & 2 deletions src/jsc/BunHeapProfiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,12 @@
generate_default_filename(&mut filename_buf, config.text_format)?
};

// Append directory if specified
// Append directory if specified. Use `join` rather than `append` so an
// absolute `config.dir` is honored: `append` trims its input as relative
// to the already-rooted path and strips the leading separator, whereas
// `join` resets the accumulated path when a segment is absolute.
if !config.dir.is_empty() {
path.append(config.dir)?;
path.join(&[config.dir])?;

Check warning on line 112 in src/jsc/BunHeapProfiler.rs

View check run for this annotation

Claude / Claude Code Review

Absolute --heap-prof-name still hits append() — same bug class, user-reachable debug_assert

Same bug class one line down: `path.append(filename)` still runs on `config.name`, which is the unvalidated `--heap-prof-name` value — an absolute path there trips the same `debug_assert!(!is_input_absolute(input))` in debug and strips the leading `/` in release. Pre-existing (and shared with `BunCPUProfiler.rs:189`), and `--heap-prof-name` is documented as a bare filename, so non-blocking — but might be worth a `join` here too while you're in this function.
Comment thread
robobun marked this conversation as resolved.
}

// Append filename
Expand Down
30 changes: 30 additions & 0 deletions test/cli/heap-prof.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,36 @@ test("--heap-prof-dir specifies output directory for V8 format", async () => {
expect(files.length).toBeGreaterThan(0);
});

test("--heap-prof-dir honors an absolute output directory", async () => {
// Two separate directories: one is the CWD, the other is the absolute
// target. An absolute --heap-prof-dir must be written to that directory, not
// resolved relative to CWD (which stripped the leading separator).
using cwdDir = tempDir("heap-prof-abs-cwd", {});
using targetDir = tempDir("heap-prof-abs-target", {});

await using proc = Bun.spawn({
cmd: [bunExe(), "--heap-prof", "--heap-prof-dir", String(targetDir), "-e", `console.log("hello");`],
cwd: String(cwdDir),
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout.trim()).toBe("hello");
expect(stderr).toContain("Heap profile written to:");
expect(exitCode).toBe(0);

// The snapshot must land in the absolute target directory.
const targetFiles = Array.from(new Bun.Glob("Heap.*.heapsnapshot").scanSync({ cwd: String(targetDir) }));
expect(targetFiles.length).toBeGreaterThan(0);

// And nothing should have been written anywhere under CWD.
const cwdFiles = Array.from(new Bun.Glob("**/Heap.*.heapsnapshot").scanSync({ cwd: String(cwdDir) }));
expect(cwdFiles).toEqual([]);
});

test("--heap-prof-dir specifies output directory for markdown format", async () => {
using dir = tempDir("heap-prof-md-dir-test", {
"profiles": {},
Expand Down
Loading