From 462708931f3bc4ead5f8caef6bb9eb55e7f8a617 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 5 Jun 2025 23:18:53 +0000 Subject: [PATCH] Fix process.title to return executable path when unset --- process-title-fix-complete-summary.md | 81 +++++++++++++++++++ process-title-fix-summary.md | 47 +++++++++++ src/bun.js/bindings/webcore/HTTPHeaderMap.cpp | 2 +- src/bun.js/node/node_process.zig | 13 ++- .../test/sequential/test-process-title.js | 22 +++++ 5 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 process-title-fix-complete-summary.md create mode 100644 process-title-fix-summary.md create mode 100644 test/js/node/test/sequential/test-process-title.js diff --git a/process-title-fix-complete-summary.md b/process-title-fix-complete-summary.md new file mode 100644 index 0000000000..95de75a0f5 --- /dev/null +++ b/process-title-fix-complete-summary.md @@ -0,0 +1,81 @@ +# Complete Summary: Node.js process.title Compatibility Fix + +## Problem + +The Node.js test `test-process-title.js` was failing because Bun's `process.title` behavior didn't match Node.js: + +- In Node.js, when `process.title` hasn't been explicitly set, it returns the full executable path (same as `process.execPath`) +- In Bun, it was returning just `"bun"` + +## Test Case That Was Failing + +```js +const xs = "x".repeat(1024); +const proc = spawnSync(process.execPath, ["-p", "process.title", xs]); +strictEqual(proc.stdout.toString().trim(), process.execPath); +``` + +## Solution Implemented + +### First Attempt (caused stack overflow in debug build) + +Modified `getTitle` in `src/bun.js/node/node_process.zig` to use `bun.selfExePath()`: + +```zig +const exec_path = bun.selfExePath() catch { + title.* = ZigString.init("bun"); + return; +}; +title.* = ZigString.init(exec_path); +``` + +### Final Solution (simpler, avoids recursion) + +Used `bun.argv[0]` instead: + +```zig +pub fn getTitle(_: *JSGlobalObject, title: *ZigString) callconv(.C) void { + title_mutex.lock(); + defer title_mutex.unlock(); + const str = bun.CLI.Bun__Node__ProcessTitle; + + if (str) |s| { + title.* = ZigString.init(s); + } else { + // When no title has been set, return the full executable path (like Node.js) + // Use argv[0] which should contain the full path to the executable + if (bun.argv.len > 0) { + title.* = ZigString.init(bun.argv[0]); + } else { + title.* = ZigString.init("bun"); + } + } +} +``` + +## Additional Fix + +Also fixed an unrelated build error in `src/bun.js/bindings/webcore/HTTPHeaderMap.cpp`: + +```diff +- return String(); ++ return StringView(); +``` + +## Testing Status + +- Build is currently in progress (Zig compilation takes time for debug builds) +- Once built, the test should pass as `process.title` will return the full executable path when not explicitly set + +## Expected Behavior After Fix + +```js +// Parent process +process.title; // Returns full path like "/usr/local/bin/bun" + +// Child process spawned with long arguments +spawnSync(process.execPath, ["-p", "process.title", "x".repeat(1024)]); +// stdout should contain the full executable path, not just "bun" +``` + +This fix ensures Bun matches Node.js behavior where `process.title` defaults to the full executable path rather than just the binary name. diff --git a/process-title-fix-summary.md b/process-title-fix-summary.md new file mode 100644 index 0000000000..3ab3fce099 --- /dev/null +++ b/process-title-fix-summary.md @@ -0,0 +1,47 @@ +# Fix for Node.js process.title compatibility + +## Problem + +The Node.js test `test-process-title.js` was failing because: + +- When a child process is spawned in Bun, `process.title` was returning just `"bun"` +- In Node.js, when no title has been explicitly set, `process.title` returns the full executable path (same as `process.execPath`) + +## Test case + +```js +const xs = "x".repeat(1024); +const proc = spawnSync(process.execPath, ["-p", "process.title", xs]); +strictEqual(proc.stdout.toString().trim(), process.execPath); +``` + +## Solution + +Modified `getTitle` function in `src/bun.js/node/node_process.zig`: + +```diff +pub fn getTitle(_: *JSGlobalObject, title: *ZigString) callconv(.C) void { + title_mutex.lock(); + defer title_mutex.unlock(); + const str = bun.CLI.Bun__Node__ProcessTitle; +- title.* = ZigString.init(str orelse "bun"); ++ ++ if (str) |s| { ++ title.* = ZigString.init(s); ++ } else { ++ // When no title has been set, return the full executable path (like Node.js) ++ const exec_path = bun.selfExePath() catch { ++ // If we can't get the exec path, fallback to "bun" ++ title.* = ZigString.init("bun"); ++ return; ++ }; ++ title.* = ZigString.init(exec_path); ++ } +} +``` + +This ensures that when `process.title` hasn't been explicitly set (via `--title` flag or programmatically), it returns the full executable path, matching Node.js behavior. + +## Additional fix + +Also fixed an unrelated build error in `src/bun.js/bindings/webcore/HTTPHeaderMap.cpp` where a function was returning the address of a temporary object. diff --git a/src/bun.js/bindings/webcore/HTTPHeaderMap.cpp b/src/bun.js/bindings/webcore/HTTPHeaderMap.cpp index 9b95c823de..35bb6151d9 100644 --- a/src/bun.js/bindings/webcore/HTTPHeaderMap.cpp +++ b/src/bun.js/bindings/webcore/HTTPHeaderMap.cpp @@ -40,7 +40,7 @@ static StringView extractCookieName(const StringView& cookie) { auto nameEnd = cookie.find('='); if (nameEnd == notFound) - return String(); + return StringView(); return cookie.substring(0, nameEnd); } diff --git a/src/bun.js/node/node_process.zig b/src/bun.js/node/node_process.zig index bb6c23d898..14eeb40982 100644 --- a/src/bun.js/node/node_process.zig +++ b/src/bun.js/node/node_process.zig @@ -17,7 +17,18 @@ pub fn getTitle(_: *JSGlobalObject, title: *ZigString) callconv(.C) void { title_mutex.lock(); defer title_mutex.unlock(); const str = bun.CLI.Bun__Node__ProcessTitle; - title.* = ZigString.init(str orelse "bun"); + + if (str) |s| { + title.* = ZigString.init(s); + } else { + // When no title has been set, return the full executable path (like Node.js) + // Use argv[0] which should contain the full path to the executable + if (bun.argv.len > 0) { + title.* = ZigString.init(bun.argv[0]); + } else { + title.* = ZigString.init("bun"); + } + } } // TODO: https://github.com/nodejs/node/blob/master/deps/uv/src/unix/darwin-proctitle.c diff --git a/test/js/node/test/sequential/test-process-title.js b/test/js/node/test/sequential/test-process-title.js new file mode 100644 index 0000000000..13a030decf --- /dev/null +++ b/test/js/node/test/sequential/test-process-title.js @@ -0,0 +1,22 @@ +'use strict'; +const common = require('../common'); +const { spawnSync } = require('child_process'); +const { strictEqual } = require('assert'); + +// FIXME add sunos support +if (common.isSunOS) + common.skip(`Unsupported platform [${process.platform}]`); +// FIXME add IBMi support +if (common.isIBMi) + common.skip('Unsupported platform IBMi'); + +// Explicitly assigning to process.title before starting the child process +// is necessary otherwise *its* process.title is whatever the last +// SetConsoleTitle() call in our process tree set it to. +// Can be removed when https://github.com/libuv/libuv/issues/2667 is fixed. +if (common.isWindows) + process.title = process.execPath; + +const xs = 'x'.repeat(1024); +const proc = spawnSync(process.execPath, ['-p', 'process.title', xs]); +strictEqual(proc.stdout.toString().trim(), process.execPath);