From 5f9d5c3a8900d73fbc7f8df85b607ad7a9acc6a7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 17:26:26 +0000 Subject: [PATCH 1/4] terminal: throw when a termios flag assignment fails The inputFlags/outputFlags/localFlags/controlFlags setters ran tcgetattr/tcsetattr on the PTY master and dropped both results, so an assignment the PTY rejected (EINVAL from glibc when the kernel refuses the requested c_cflag bits, ENOTTY/EBADF when the fd is no longer a tty) returned as if it had been applied. A setter's return value is not observable from JS, so throwing is the only way to report it, and it is what resize() and setRawMode() already do. get_termios/set_termios now return bun_sys::Result and the setter throws the SystemError when either call fails. Reading still yields 0 on failure and assigning on a closed terminal is still a no-op; the closed check moves after the value coercion so a valueOf() that closes the terminal takes the no-op path rather than throwing EBADF. Clears the error_collapsed_to_bool entry for Terminal.rs in mordant-baseline.toml. --- mordant-baseline.toml | 1 - packages/bun-types/bun.d.ts | 8 +-- src/runtime/api/bun/Terminal.rs | 24 +++---- test/js/bun/terminal/terminal.test.ts | 97 ++++++++++++++++++++++++++- 4 files changed, 111 insertions(+), 19 deletions(-) diff --git a/mordant-baseline.toml b/mordant-baseline.toml index 109483f7b029..9c2e75240b92 100644 --- a/mordant-baseline.toml +++ b/mordant-baseline.toml @@ -64,7 +64,6 @@ "defaulted_failure:src/runtime/server/RequestContext.rs" = 1 "defaulted_failure:src/runtime/test_runner/pretty_format.rs" = 4 "derived_field:src/runtime/test_runner/bun_test.rs" = 1 -"error_collapsed_to_bool:src/runtime/api/bun/Terminal.rs" = 1 "error_collapsed_to_bool:src/runtime/api/bun/h2_frame_parser.rs" = 32 "error_collapsed_to_bool:src/runtime/ffi/ffi_body.rs" = 1 "field_valid_only_when:src/runtime/cli/filter_run.rs" = 1 diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts index d8c9402e2871..673c3d69f28b 100644 --- a/packages/bun-types/bun.d.ts +++ b/packages/bun-types/bun.d.ts @@ -8296,7 +8296,7 @@ declare module "bun" { * Terminal input flags (c_iflag from termios). * Controls input processing behavior like ICRNL, IXON, etc. * Returns 0 if the terminal is closed. - * Setting returns true on success, false on failure. + * Setting is a no-op on a closed terminal and throws if the PTY rejects the new flags. */ inputFlags: number; @@ -8304,7 +8304,7 @@ declare module "bun" { * Terminal output flags (c_oflag from termios). * Controls output processing behavior like OPOST, ONLCR, etc. * Returns 0 if the terminal is closed. - * Setting returns true on success, false on failure. + * Setting is a no-op on a closed terminal and throws if the PTY rejects the new flags. */ outputFlags: number; @@ -8312,7 +8312,7 @@ declare module "bun" { * Terminal local flags (c_lflag from termios). * Controls local processing like ICANON, ECHO, ISIG, etc. * Returns 0 if the terminal is closed. - * Setting returns true on success, false on failure. + * Setting is a no-op on a closed terminal and throws if the PTY rejects the new flags. */ localFlags: number; @@ -8320,7 +8320,7 @@ declare module "bun" { * Terminal control flags (c_cflag from termios). * Controls hardware characteristics like CSIZE, PARENB, etc. * Returns 0 if the terminal is closed. - * Setting returns true on success, false on failure. + * Setting is a no-op on a closed terminal and throws if the PTY rejects the new flags. */ controlFlags: number; } diff --git a/src/runtime/api/bun/Terminal.rs b/src/runtime/api/bun/Terminal.rs index e181da0fbb13..f5315d395f12 100644 --- a/src/runtime/api/bun/Terminal.rs +++ b/src/runtime/api/bun/Terminal.rs @@ -1364,7 +1364,7 @@ impl Terminal { if self.flags.get().contains(Flags::CLOSED) || self.master_fd.get() == Fd::INVALID { return JSValue::js_number(0.0); } - let Some(termios_data) = get_termios(self.master_fd.get()) else { + let Ok(termios_data) = get_termios(self.master_fd.get()) else { return JSValue::js_number(0.0); }; let raw: u64 = match FIELD { @@ -1389,13 +1389,14 @@ impl Terminal { } #[cfg(unix)] { - if self.flags.get().contains(Flags::CLOSED) || self.master_fd.get() == Fd::INVALID { - return Ok(()); - } let num = value.coerce_f64(global_object)?; - let Some(mut termios_data) = get_termios(self.master_fd.get()) else { + // Checked after coercion: a valueOf() may have closed the terminal. + let master_fd = self.master_fd.get(); + if self.flags.get().contains(Flags::CLOSED) || master_fd == Fd::INVALID { return Ok(()); - }; + } + let mut termios_data = + get_termios(master_fd).map_err(|err| err.throw(global_object))?; let max_val: f64 = libc::tcflag_t::MAX as f64; // Match Zig's `@max(0, @min(num, max_val))`: apply min first so NaN // resolves to max_val (f64::min returns the non-NaN operand), not 0. @@ -1407,8 +1408,7 @@ impl Terminal { TermiosField::Lflag => termios_data.c_lflag = bits, TermiosField::Cflag => termios_data.c_cflag = bits, } - let _ = set_termios(self.master_fd.get(), &termios_data); - Ok(()) + set_termios(master_fd, &termios_data).map_err(|err| err.throw(global_object)) } } @@ -1675,14 +1675,14 @@ type Termios = sys::posix::Termios; /// Get terminal attributes using tcgetattr #[cfg(unix)] -fn get_termios(fd: Fd) -> Option { - sys::posix::tcgetattr(fd.native()).ok() +fn get_termios(fd: Fd) -> sys::Result { + sys::posix::tcgetattr(fd.native()) } /// Set terminal attributes using tcsetattr (TCSANOW = immediate) #[cfg(unix)] -fn set_termios(fd: Fd, termios_p: &Termios) -> bool { - sys::posix::tcsetattr(fd.native(), sys::posix::TCSA::Now, termios_p).is_ok() +fn set_termios(fd: Fd, termios_p: &Termios) -> sys::Result<()> { + sys::posix::tcsetattr(fd.native(), sys::posix::TCSA::Now, termios_p) } impl Terminal { diff --git a/test/js/bun/terminal/terminal.test.ts b/test/js/bun/terminal/terminal.test.ts index ec0544e4a446..c6e1d2fc1881 100644 --- a/test/js/bun/terminal/terminal.test.ts +++ b/test/js/bun/terminal/terminal.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isLinux, isWindows, tempDir } from "harness"; +import { bunEnv, bunExe, isGlibc, isLinux, isWindows, tempDir } from "harness"; import { readFileSync } from "node:fs"; import { join } from "node:path"; @@ -474,16 +474,109 @@ describe("Bun.Terminal", () => { expect(terminal.inputFlags).toBe(0); }); + test.skipIf(isWindows)("setting flags on a terminal closed while coercing the value is a no-op", () => { + const terminal = new Bun.Terminal({}); + + terminal.localFlags = { + valueOf() { + terminal.close(); + return 0; + }, + } as any; + + expect(terminal.closed).toBe(true); + }); + + test.skipIf(!isGlibc)("setting flags the PTY refuses to apply throws", async () => { + // Linux forces CREAD on for PTYs. glibc's tcsetattr reads the attributes + // back and fails with EINVAL when the requested change did not take; + // libcs that only issue the ioctl report success, hence the gate. + const CREAD = 0x80; + await using terminal = new Bun.Terminal({}); + const original = terminal.controlFlags; + expect(original & CREAD).toBe(CREAD); + + let caught: any; + try { + terminal.controlFlags = original & ~CREAD; + } catch (e) { + caught = e; + } + + expect({ code: caught?.code, syscall: caught?.syscall, controlFlags: terminal.controlFlags }).toEqual({ + code: "EINVAL", + syscall: "ioctl", + controlFlags: original, + }); + }); + + test.skipIf(isWindows)("setting flags throws when the terminal's fd is not a PTY", async () => { + // Portable version of the above: swap the PTY master fd out for + // /dev/null (ENOTTY) behind the terminal's back. openpty() hands out + // the lowest free fd, so the master lands on the fd number the probe + // just freed. + const script = ` + const fs = require("node:fs"); + // The first terminal may open unrelated fds on the side (openpty is + // loaded lazily); construct one up front so the probe below sees a + // settled fd table. + new Bun.Terminal({}).close(); + const probe = fs.openSync("/dev/null", "r"); + fs.closeSync(probe); + + const terminal = new Bun.Terminal({}); + const flags = terminal.localFlags; + fs.closeSync(probe); + const replacement = fs.openSync("/dev/null", "r"); + + let thrown = null; + try { + terminal.localFlags = flags; + } catch (e) { + thrown = { code: e.code, syscall: e.syscall }; + } + console.log(JSON.stringify({ + swapped: replacement === probe, + hadFlagsBeforeSwap: flags !== 0, + flagsAfterSwap: terminal.localFlags, + closed: terminal.closed, + thrown, + })); + terminal.close(); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", script], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual({ + swapped: true, + hadFlagsBeforeSwap: true, + flagsAfterSwap: 0, + closed: false, + thrown: { code: "ENOTTY", syscall: "ioctl" }, + }); + expect(exitCode).toBe(0); + }); + test.skipIf(isWindows)("NaN clamps to tcflag_t max, same as Infinity", async () => { // Assigning NaN must clamp to the same value as Infinity (tcflag_t::MAX), // matching the reference `@max(0, @min(num, max))` semantics. The kernel // may mask some bits per field, so compare NaN against Infinity rather - // than a hard-coded constant. + // than a hard-coded constant. Both assignments start from the original + // flags: re-requesting the already-masked bits asks the PTY for a change + // it refuses, which glibc's tcsetattr reports as EINVAL. await using terminal = new Bun.Terminal({ cols: 80, rows: 24 }); for (const prop of ["inputFlags", "outputFlags", "localFlags", "controlFlags"] as const) { + const original = terminal[prop]; terminal[prop] = Infinity; const fromInfinity = terminal[prop]; + terminal[prop] = original; terminal[prop] = NaN; const fromNaN = terminal[prop]; expect({ prop, fromNaN }).toEqual({ prop, fromNaN: fromInfinity }); From 907efaa2113486c495ee050e03578cb3d97b62c8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 19:11:21 +0000 Subject: [PATCH 2/4] ci: retrigger From 2a8bc0db1187f80ae2c0f7671b73b7920b4bcba4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 19:37:28 +0000 Subject: [PATCH 3/4] test(terminal): drop the EINVAL case, it depends on Debian's glibc patch Upstream glibc's tcsetattr is a single TCSETS ioctl; the EINVAL for a c_cflag change the pty driver refuses comes from the patch Debian and Ubuntu apply to their glibc, so the isGlibc gate would fail the test on other glibc distributions. The /dev/null ENOTTY test covers the throw on every libc. --- test/js/bun/terminal/terminal.test.ts | 38 ++++++--------------------- 1 file changed, 8 insertions(+), 30 deletions(-) diff --git a/test/js/bun/terminal/terminal.test.ts b/test/js/bun/terminal/terminal.test.ts index c6e1d2fc1881..7c0fd3ad5981 100644 --- a/test/js/bun/terminal/terminal.test.ts +++ b/test/js/bun/terminal/terminal.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isGlibc, isLinux, isWindows, tempDir } from "harness"; +import { bunEnv, bunExe, isLinux, isWindows, tempDir } from "harness"; import { readFileSync } from "node:fs"; import { join } from "node:path"; @@ -487,34 +487,11 @@ describe("Bun.Terminal", () => { expect(terminal.closed).toBe(true); }); - test.skipIf(!isGlibc)("setting flags the PTY refuses to apply throws", async () => { - // Linux forces CREAD on for PTYs. glibc's tcsetattr reads the attributes - // back and fails with EINVAL when the requested change did not take; - // libcs that only issue the ioctl report success, hence the gate. - const CREAD = 0x80; - await using terminal = new Bun.Terminal({}); - const original = terminal.controlFlags; - expect(original & CREAD).toBe(CREAD); - - let caught: any; - try { - terminal.controlFlags = original & ~CREAD; - } catch (e) { - caught = e; - } - - expect({ code: caught?.code, syscall: caught?.syscall, controlFlags: terminal.controlFlags }).toEqual({ - code: "EINVAL", - syscall: "ioctl", - controlFlags: original, - }); - }); - test.skipIf(isWindows)("setting flags throws when the terminal's fd is not a PTY", async () => { - // Portable version of the above: swap the PTY master fd out for - // /dev/null (ENOTTY) behind the terminal's back. openpty() hands out - // the lowest free fd, so the master lands on the fd number the probe - // just freed. + // No libc rejects a termios update on a healthy PTY portably, so swap + // the PTY master fd out for /dev/null (ENOTTY) behind the terminal's + // back. openpty() hands out the lowest free fd, so the master lands on + // the fd number the probe just freed. const script = ` const fs = require("node:fs"); // The first terminal may open unrelated fds on the side (openpty is @@ -568,8 +545,9 @@ describe("Bun.Terminal", () => { // matching the reference `@max(0, @min(num, max))` semantics. The kernel // may mask some bits per field, so compare NaN against Infinity rather // than a hard-coded constant. Both assignments start from the original - // flags: re-requesting the already-masked bits asks the PTY for a change - // it refuses, which glibc's tcsetattr reports as EINVAL. + // flags: re-requesting only the already-masked c_cflag bits is a + // tcsetattr that changes nothing, which Debian's and Ubuntu's patched + // glibc reports as EINVAL (upstream glibc and other libcs return 0). await using terminal = new Bun.Terminal({ cols: 80, rows: 24 }); for (const prop of ["inputFlags", "outputFlags", "localFlags", "controlFlags"] as const) { From 49b64486538b22c6e7c7be44fdf6ab0e4e7856fb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 21:48:09 +0000 Subject: [PATCH 4/4] test(terminal): swap the master for a regular file, not /dev/null Termios ioctls on /dev/null fail with ENODEV on macOS; a regular file fails with ENOTTY on every POSIX kernel, which is what the test asserts. --- test/js/bun/terminal/terminal.test.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/js/bun/terminal/terminal.test.ts b/test/js/bun/terminal/terminal.test.ts index 7c0fd3ad5981..f0a26e568def 100644 --- a/test/js/bun/terminal/terminal.test.ts +++ b/test/js/bun/terminal/terminal.test.ts @@ -489,22 +489,23 @@ describe("Bun.Terminal", () => { test.skipIf(isWindows)("setting flags throws when the terminal's fd is not a PTY", async () => { // No libc rejects a termios update on a healthy PTY portably, so swap - // the PTY master fd out for /dev/null (ENOTTY) behind the terminal's - // back. openpty() hands out the lowest free fd, so the master lands on - // the fd number the probe just freed. + // the PTY master fd out for a regular file behind the terminal's back. + // openpty() hands out the lowest free fd, so the master lands on the fd + // number the probe just freed. A regular file fails termios ioctls with + // ENOTTY on every POSIX kernel; /dev/null would be ENODEV on macOS. const script = ` const fs = require("node:fs"); // The first terminal may open unrelated fds on the side (openpty is // loaded lazily); construct one up front so the probe below sees a // settled fd table. new Bun.Terminal({}).close(); - const probe = fs.openSync("/dev/null", "r"); + const probe = fs.openSync(process.execPath, "r"); fs.closeSync(probe); const terminal = new Bun.Terminal({}); const flags = terminal.localFlags; fs.closeSync(probe); - const replacement = fs.openSync("/dev/null", "r"); + const replacement = fs.openSync(process.execPath, "r"); let thrown = null; try {