shell: fail a command whose `> ${buf}` redirect overflows the target buffer
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)}leaveshelland exits 0. So does/bin/echo. - Builtins:
BuiltinIO::write_no_io_to(src/runtime/shell/Builtin.rs:352) returnsOk(short), and all 31 callers ofBuiltin::write_no_iodrop the result. Subprocesses:BufferedOutput::append(src/runtime/shell/subproc.rs:1519) clips the chunk under aTODO: We should probably throw error here?.
Fix
- Both paths set the new
Cmd::redirect_overflow.write_no_io_towrites what fits and returnsErr(ENOSPC).BufferedOutput::ArrayBufferrecords a clippedappendforCmd::buffered_output_close. Cmd::nextreads the flag in itsDonearm, where builtins and subprocesses both finish. It writes<argv0>: write error: No space left on deviceto the shell's stderr. Exit code 0 becomes 1. A command that failed on its own keeps its code.- The
Cmdstays inDonewhile 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. Alsoleak.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, aPipeReadercopies each chunk of the child's pipe into it. Cmdis the state node of one command.Builtin::doneandCmd::finish_if_doneboth setCmdState::Doneand re-enterCmd::next, which ends the command.- A write to a stderr file descriptor is asynchronous.
IOWritercallsCmd::on_io_writer_chunkwhen it completes. - The
yesbuiltin is unchanged. It callswrite_no_io_toitself, stops onENOSPC, 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. Theyesbuiltin 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>and2>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:
- The first version wrote the report through
cmd_write_failing_error, which moves theCmdtoWaitingWriteErr.$(echo /usr/bin/env) > ${buf}then crashed a debug build withassertion 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 theCmdreachesDonewhen the pipes close, and the lateron_exitfinished it a second time. TheCmdnow stays inDone, wherefinish_if_donealready ignores late callbacks. Test: "subprocess that exits after the shell starts the report". - The first version printed nothing when the command failed on its own.
- The docs sentence was false for
&>. - This body states the origin and the behaviour change.
- 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 ofwrite_no_io_toisyes, which handles its ownENOSPC. A flag on the sink makes it report twice. - Type the flag as an errno. Only
ENOSPCexists 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