Skip to content

fix: Pipe process.stdout.write through Buffer to fix node incompatibility #29232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lindskogen wants to merge 1 commit into
base: main
Choose a base branch
from
+53 −0
Open

fix: Pipe process.stdout.write through Buffer to fix node incompatibility #29232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions src/js/internal/fs/streams.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,9 @@ function _write(data, encoding, cb) {
const fileSink = this[kWriteStreamFastPath];

if (fileSink && fileSink !== true) {
if (typeof data === "string" && encoding && encoding !== "utf8" && encoding !== "utf-8" && encoding !== "buffer") {
data = Buffer.from(data, encoding);
}
const maybePromise = fileSink.write(data);
if ($isPromise(maybePromise)) {
maybePromise
Expand Down Expand Up @@ -603,6 +606,9 @@ function underscoreWriteFast(this: FSStream, data: any, encoding: any, cb: any)
this.fd = fileSink._getFd();
}

if (typeof data === "string" && encoding && encoding !== "utf8" && encoding !== "utf-8" && encoding !== "buffer") {
data = Buffer.from(data, encoding);
}
const maybePromise = fileSink.write(data);
if ($isPromise(maybePromise)) {
maybePromise.then(
Expand Down Expand Up @@ -652,6 +658,9 @@ function writeFast(this: FSStream, data: any, encoding: any, cb: any) {

const fileSink = this[kWriteStreamFastPath];
if (fileSink && fileSink !== true) {
if (typeof data === "string" && encoding && encoding !== "utf8" && encoding !== "utf-8" && encoding !== "buffer") {
data = Buffer.from(data, encoding);
}
const maybePromise = fileSink.write(data);
if ($isPromise(maybePromise)) {
maybePromise
Expand Down
44 changes: 44 additions & 0 deletions test/js/node/process/process-stdio.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,4 +158,48 @@ describe.concurrent("process-stdio", () => {
`hello worldhello again|😋 Get Emoji — All Emojis to ✂️ Copy and 📋 Paste 👌`.repeat(9999),
);
});

// Regression: process.stdout.write(string, encoding) was ignoring the encoding
// parameter on the fast path and UTF-8 encoding the string instead.
test.each(["binary", "latin1"] as const)("process.stdout.write(string, '%s') writes raw bytes", encoding => {
const { stdout } = spawnSync({
cmd: [
bunExe(),
"-e",
`for (let i = 0; i <= 0xff; i++) process.stdout.write(String.fromCharCode(i), ${JSON.stringify(encoding)});`,
],
stdout: "pipe",
stdin: null,
stderr: "inherit",
env: bunEnv,
});
expect(stdout).toBeInstanceOf(Buffer);
expect(stdout.length).toBe(256);
const expected = Buffer.alloc(256);
for (let i = 0; i < 256; i++) expected[i] = i;
expect(Buffer.compare(stdout, expected)).toBe(0);
});

test("process.stdout.write(string, 'hex') decodes hex", () => {
const { stdout } = spawnSync({
cmd: [bunExe(), "-e", `process.stdout.write("deadbeef", "hex");`],
stdout: "pipe",
stdin: null,
stderr: "inherit",
env: bunEnv,
});
expect(Buffer.compare(stdout, Buffer.from([0xde, 0xad, 0xbe, 0xef]))).toBe(0);
});

test("process.stdout.write(string) defaults to UTF-8", () => {
const { stdout } = spawnSync({
cmd: [bunExe(), "-e", `process.stdout.write("héllo");`],
stdout: "pipe",
stdin: null,
stderr: "inherit",
env: bunEnv,
});
// é = U+00E9 = UTF-8 c3 a9
expect(Buffer.compare(stdout, Buffer.from([0x68, 0xc3, 0xa9, 0x6c, 0x6c, 0x6f]))).toBe(0);
});
Comment on lines +164 to +204
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add exit-code assertions to each new spawnSync regression test.

Each new test validates stdout bytes but never asserts process success. Please capture exitCode and assert it after stdout checks in all three tests.

Suggested patch 
-    const { stdout } = spawnSync({
+    const { stdout, exitCode } = spawnSync({
       cmd: [
         bunExe(),
         "-e",
         `for (let i = 0; i <= 0xff; i++) process.stdout.write(String.fromCharCode(i), ${JSON.stringify(encoding)});`,
       ],
@@
     expect(stdout.length).toBe(256);
     const expected = Buffer.alloc(256);
     for (let i = 0; i < 256; i++) expected[i] = i;
     expect(Buffer.compare(stdout, expected)).toBe(0);
+    expect(exitCode).toBe(0);
   });

   test("process.stdout.write(string, 'hex') decodes hex", () => {
-    const { stdout } = spawnSync({
+    const { stdout, exitCode } = spawnSync({
       cmd: [bunExe(), "-e", `process.stdout.write("deadbeef", "hex");`],
       stdout: "pipe",
       stdin: null,
       stderr: "inherit",
       env: bunEnv,
     });
     expect(Buffer.compare(stdout, Buffer.from([0xde, 0xad, 0xbe, 0xef]))).toBe(0);
+    expect(exitCode).toBe(0);
   });

   test("process.stdout.write(string) defaults to UTF-8", () => {
-    const { stdout } = spawnSync({
+    const { stdout, exitCode } = spawnSync({
       cmd: [bunExe(), "-e", `process.stdout.write("héllo");`],
       stdout: "pipe",
       stdin: null,
       stderr: "inherit",
       env: bunEnv,
     });
     // é = U+00E9 = UTF-8 c3 a9
     expect(Buffer.compare(stdout, Buffer.from([0x68, 0xc3, 0xa9, 0x6c, 0x6c, 0x6f]))).toBe(0);
+    expect(exitCode).toBe(0);
   });

As per coding guidelines, “Always check exit codes and test error scenarios when spawning processes in tests” and “Expect stdout assertions before exit code assertions.”

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@test/js/node/process/process-stdio.test.ts` around lines 164 - 204, Each test
that spawns a process (the three tests using spawnSync with bunExe and bunEnv)
lacks an assertion on the child process exit code; update each test to capture
the spawnSync result's exitCode (from the object returned by spawnSync) and add
an assertion after the existing stdout Buffer checks to ensure exitCode is 0 (or
the expected success code). Locate the three tests named
"process.stdout.write(string, '%s') writes raw bytes",
"process.stdout.write(string, 'hex') decodes hex", and
"process.stdout.write(string) defaults to UTF-8" and add the exitCode assertion
immediately after the Buffer.compare / stdout checks for each spawnSync call.

});