shell: fail a command whose `> ${buf}` redirect overflows the target buffer

RO
2mo ago
+283-35

Problem

  • A shell command that writes more than its > ${buf} target (Buffer, TypedArray, ArrayBuffer) holds drops the rest and exits 0. echo hello world > ${Buffer.alloc(4)} leaves hell and exits 0. So does /bin/echo.
  • Builtins: BuiltinIO::write_no_io_to (src/runtime/shell/Builtin.rs:352) returns Ok(short), and all 31 callers of Builtin::write_no_io drop the result. Subprocesses: BufferedOutput::append (src/runtime/shell/subproc.rs:1519) clips the chunk under a TODO: We should probably throw error here?.

Fix

  • Both paths set the new Cmd::redirect_overflow. write_no_io_to writes what fits and returns Err(ENOSPC). BufferedOutput::ArrayBuffer records a clipped append for Cmd::buffered_output_close.
  • Cmd::next reads the flag in its Done arm, where builtins and subprocesses both finish. It writes <argv0>: write error: No space left on device to the shell's stderr. Exit code 0 becomes 1. A command that failed on its own keeps its code.
  • The Cmd stays in Done while the report is in flight, so a late process exit cannot finish it twice.
  • Verified: test/js/bun/shell/bunshell.test.ts, 15 new tests, 13 fail on 1.4.3, on Linux and Windows. Also leak.test.ts, commands/, yield.test.ts. Self-reviewed: 7 concerns raised, 5 addressed (Notes).

Background

  • A builtin writes into the buffer directly (BuiltinIO::ArrayBuf). For a subprocess, a PipeReader copies each chunk of the child's pipe into it.
  • Cmd is the state node of one command. Builtin::done and Cmd::finish_if_done both set CmdState::Done and re-enter Cmd::next, which ends the command.
  • A write to a stderr file descriptor is asynchronous. IOWriter calls Cmd::on_io_writer_chunk when it completes.
  • The yes builtin is unchanged. It calls write_no_io_to itself, stops on ENOSPC, and exits 1.
Notes

Origin and behaviour change. Another session found this by a read of the code. No user reported it. It changes behaviour: a command that overflowed its target exited 0 and now exits 1, so the default $ throws a ShellError. #34698 fixed the same bug and was closed only as stale. Its closing comment asks for a new PR if the bug still reproduces. It does, on 1.4.3 and on main.

Policy, for a maintainer to rule on.

  • Exit 1 plus name: write error: <strerror> is "Policy A" in the #40033 thread (what bash and coreutils do). Nobody has ruled on it yet. The yes builtin already exits 1 for a full buffer target.
  • A command that fails on its own keeps its exit code, and the shell still prints the report. Without the report, diff a b > ${buf} (exit 1 means "the files differ") hides the overflow. The other option is exit 1 always, which is what main does for a failed output relay.
  • The report names args[0]. For a subprocess that is the resolved path, the same name the child prints for its own errors.

Open PRs that touch the same lines.

  • #42179 edits the same two write arms (live_slice_mut). It fixes a crash, so it should merge first. This PR then needs a small rebase. Two of its tests shrink the target of an external command to 0 bytes and expect exit 0. With this PR they expect exit 1 and the report.
  • #43131 gives &> one cursor per target. Until it lands, &> can lose bytes with no overflow, because each stream fits on its own. So the new docs sentence names > and 2> only.
  • #40033 reports failed fd writes from Builtin::done. It uses the same message format. The overlap is textual only (Cmd::buffered_output_close, Cmd::on_io_writer_chunk).

Not in this PR. The shell does not stop a command whose target is full. A command that never stops writing (/usr/bin/yes > ${buf}) never settles, on 1.4.3 and with this PR. The yes builtin is not affected. #43204 tracks it, with a pointer to the maxBuffer budget of the pipe reader. The docs sentence says that the command still runs to its end.

Self-review. Addressed:

  1. The first version wrote the report through cmd_write_failing_error, which moves the Cmd to WaitingWriteErr. $(echo /usr/bin/env) > ${buf} then crashed a debug build with assertion failed: matches!(interp.as_cmd(this).state, CmdState::WaitingWriteErr). A command that is only a command substitution has an exit code before it runs. So the Cmd reaches Done when the pipes close, and the later on_exit finished it a second time. The Cmd now stays in Done, where finish_if_done already ignores late callbacks. Test: "subprocess that exits after the shell starts the report".
  2. The first version printed nothing when the command failed on its own.
  3. The docs sentence was false for &>.
  4. This body states the origin and the behaviour change.
  5. This body states the merge order with #42179 and #43131.

Not done:

  • Record the overflow on the sink, not in Builtin::write_no_io. The only direct caller of write_no_io_to is yes, which handles its own ENOSPC. A flag on the sink makes it report twice.
  • Type the flag as an errno. Only ENOSPC exists on this path today.

Fail before. On 1.4.3 these fail: both "builtin, N of 12 bytes fit", "builtin that writes one chunk per operand", "builtin, with stderr on a file descriptor", "builtin: the right side of || runs", "builtin: only the window of a Uint8Array view is written", "builtin that also fails on its own", both "subprocess, N of 11 bytes fit", "subprocess: stderr", "subprocess: the right side of || runs", "subprocess: its own exit code is kept", "subprocess that exits after the shell starts the report". The two exact-fit tests pass before and after.

Existing tests that changed. leak.test.ts memleak_ArrayBuffer and memleak_Buffer write a 16 KB file into a 128-byte target on purpose. They now expect exit 1 and the report. cat is a subprocess on POSIX and a builtin on Windows, so both paths run in CI.

Suites run (debug build): all of bunshell.test.ts, bunshell-instance.test.ts, commands/, yield.test.ts, throw.test.ts, shelloutput.test.ts, exec.test.ts, file-io.test.ts, epipe.test.ts, shell-write-fault.test.ts, shell-pipe-read-fault.test.ts, shell-hang.test.ts, and the two leak tests. On Windows: bunshell.test.ts, bunshell-instance.test.ts, yes.test.ts, the two leak tests. Failures that also occur without this change: ls.test.ts "permission denied" (the container runs as root) and, on the Windows machine, tilde_expansion.


no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/shell/leak.test.ts, test/js/bun/shell/bunshell.test.ts