shell: open a Response over Bun.file(path) as a file redirect
Problem
$`cmd < ${new Response(Bun.file(path))}`runscmdon an empty stdin at exit 0 when the file is missing, and creates the file. The bare${Bun.file(path)}exits 1 withbun: No such file or directory: <path>.- On main a spawned command gives the path to spawn, which opens it with
O_CREAT(src/spawn_sys/spawn_process.rs:801). A builtin readsblob.shared_view(), empty for a file. - On that arm
>does not truncate and>>does not append.
Fix
Interpreter::redirect_targetclassifies a redirect once. AResponseoverBun.file(path)is a path word: both redirect setups open it withshell_openatand the flags of the operator.- The classifier only reads. The redirect takes the body after the open succeeds.
- Verified:
test/js/bun/shell/bunshell.test.ts, "redirect to a Response over Bun.file(path)" (46 tests, 37 fail on main), and 9 more shell suites. - Self-reviewed: 29 concerns raised, 27 addressed (Notes).
Background
- A command is spawned or a builtin (
echo, on Windowscat), each with its own redirect setup. - The template turns
${Bun.file(path)}into path text.${response}stays a JS object until the command starts. - Weighed: classify in the template (never uses the body), drop
O_CREATin spawn only (cannot reach builtins).
Downsides
- 25 of 30 redirect cells change for such a
Response, 10 from a throw to a write (ledger in Notes). - A builtin now blocks on a FIFO open (1 ms to 1452 ms in a probe), as the bare
< fifodoes. .text+1,024 bytes. A redirect to another JS object runs 6 to 27 more instructions.
Notes
Origin and reach
- A fuzz run found this. No user reported it, and there is no issue. The request was: "a Response whose body is a Blob over a file is opened like the bare
Bun.file, so that a missing file fails the command." - The form that a person is most likely to write,
<from aResponseover an existing file on POSIX, already works on main. await fetch("file:///missing")resolves withok: trueand status 200, socat < ${await fetch(fileUrl)}is the same case. The caller has no sign that the body is a missing file.- On main,
cmd < ${await response.blob()}and the bare${Bun.file(path)}already fail as they should (measured on a release build).
Repro
import { $ } from "bun";
import { existsSync } from "node:fs";
const missing = process.cwd() + "/none.txt";
const r = await $`cat < ${new Response(Bun.file(missing))}`.quiet().nothrow();
console.log(r.exitCode, JSON.stringify(r.stderr.toString()), existsSync(missing));
// main: 0 "" true
// this PR: 1 "bun: No such file or directory: /.../none.txt" false
Behaviour changes, main to this branch
The matrix is 2 exec forms (a spawned command, builtin echo) x 5 operators (<, >, >>, 2>, &>) x 3 file states (existing, missing, in a missing directory). Target: new Response(Bun.file(path)). Linux x64, debug builds of main 5c6d4e24cd and of this branch. The bare Bun.file(path) gives the same result on both builds in all 30 cells. This branch equals the bare form in 30 of 30 cells, main in 5.
| Change | Cells | Which |
|---|---|---|
| Silent exit 0 becomes exit 1 that names the file | 3 | spawned < missing (main also creates the file), builtin < missing and missing directory |
| Wrong content at exit 0 becomes right content | 3 | spawned >, >>, 2> to an existing file (main writes at offset 0 and keeps the tail) |
| A throw becomes a write at exit 0 | 10 | builtin >, >>, 2>, &> to an existing or missing file (8), spawned &> (2) |
| A throw becomes a shell error, exit 1 | 5 | the same 5 pairs into a missing directory |
| Error text only | 4 | spawned <, >, >>, 2> into a missing directory: main names the command, this branch names the file |
| Unchanged | 5 | spawned < existing, spawned >, >>, 2> to a missing file, builtin < existing |
The throws on main are Cannot redirect stdout/stderr to an immutable blob. Expected a file (builtin, for a file) and Blobs are immutable, and cannot be used for stdout/stderr (spawned &>, because the arm takes the body two times).
More changes outside the matrix:
- After a failed open,
response.bodyUsedstaysfalse, and a stream that the caller holds stays unlocked. On main the body is used. - A new output file of a spawned command gets mode 0666 before umask, as a path word does. On main it gets 0664.
- A relative path resolves against the cwd of the shell on every platform, as the bare form does. On main this was already so on Linux. On macOS and Windows main uses the process cwd (read from
src/spawn_sys/posix_spawn.rsandsrc/spawn/process.rs, not run).response.text()keeps the process cwd. Bun.file("")in aResponsegivesbun: ambiguous redirect, as the bare form does.- Windows only, read from the code and not run:
>to such aResponsewhen the file has the hidden or system attribute. On main a spawned command writes into the file, because spawn opens it withOPEN_ALWAYS. Here the open fails withEPERM, as> hidden.txtand the bare form do on main:shell_openatopens withCREATE_ALWAYSand has no fallback for a hidden file. That fallback is handed to a separate change. - The body forms that are still the file behave the same: after
.bodywas read,new Response(Bun.file(path).stream()),.clone(), andfetch()of afile:URL.
What stays as on main
- A second redirect from the same
Responsereads a used body: empty stdin at exit 0. The bare form reads the file again. #42786 turns a used body into a throw. - A sliced file (
Bun.file(p).slice(1, 3)) is opened whole, in aResponseor bare (#41209, #44301). Bun.file(fd)and in-memory bodies with a builtin:echo out > ${Bun.file(fd)}exits 0 and writes nothing,echo out > ${new Response(Bun.file(fd))}throws,echo out > ${new Response(new Blob(["x"]))}exits 0 and drops the output, builtincat < ${Bun.file(fd)}reads nothing. These are other defects of the same function. #35318 fixed them and was closed by the stale sweep. They are handed to a separate change.Bun.spawn({ stdin: Bun.file(missing) })still creates the file.spawn_process.rs:801is not changed here. After this PR no shell redirect reaches that open.- A path with a NUL byte throws
ERR_INVALID_ARG_VALUE, as the bare form does. The only producer on main isBun.buildwith a NUL innamingoroutdir, which panics in a debug build before it returns. That is why no test covers the branch. On a release build of this branch the throw is measured.
Numbers
Release builds of main 5c6d4e24cd and of this change on that commit, Linux x64.
.text: 82,231,705 to 82,232,729 bytes (size -A).- Instructions inside
Cmd::transition_to_exec, JIT off, single-stepped with gdb, warm runs, builtinecho: no redirect 1105 to 1101, path word 1587 to 1418, Buffer 1572 to 1588, Blob 1419 to 1446, in-memoryResponse1730 to 1736. - Allocator calls per command (breakpoints on the mimalloc entry points, 220 minus 20 commands): path word 209.26 to 208.26, Buffer 201.26 to 201.26, in-memory
Response205.285 to 205.285. - Syscalls per command (ptrace count, 120 minus 20 commands), routes that do not change:
open,openat,close,dup2,fcntl,fstat,read,write,socketpairare equal for no redirect, a path word, a Buffer and an in-memoryResponse, spawned and builtin. - Syscalls,
Responseover an existing file: spawned net +1 (bun: +1openat, +1fcntl, +1close. The child before exec: -1openat, -1close). Builtin: +1openat, +1close(main opens nothing). - Scans of a string
Responsebody per redirect (calls ofto_utf8_from_latin1, debug builds): 2 on main, 2 on this branch. - FIFO: builtin
echo hi < ${new Response(Bun.file(fifo))}with a writer that opens after 1.5 s takes 1 ms on main (it opens nothing) and 1452 ms here with 0 timer callbacks. The bare< fifotakes 1452 ms on both. - Opens of a missing read target with
O_CREAT: each spawned shell form goes from 1 to 0. The 7Bun.spawnstdin forms keep theirs. strace,valgrindandbloatyare not installed on the machine. The counts come from a ptrace counter, gdb andsize.
Related changes
- #42786 rejects a
Responsebody that is not in memory (a stream, a used body). It lets a file body pass, so the two do not overlap in behaviour. They edit the same two functions. The one that lands second putsthrow_if_body_not_in_memoryon theRedirectTarget::Objectarm and merges the two docs sentences. - #46665 gives
Bun.spawnpath slots one flags table. It also sends aResponseover a path file toshell_openatfor spawned commands, inCmd.rs. This PR does that for both exec forms through one classifier, so the one that lands second drops or rebases that hunk. - #40084 moves the open of a FIFO redirect off the JS thread for
ast::Redirect::Atom. It does not see this route, because aResponsebecomes a path word only inredirect_target. - #37641 removes
O_CREATfrom the stdin path open in spawn. That is aBun.spawndefault change and is not part of this PR. - #46673 throws for an S3-backed Blob in a shell command. It rewrites the Blob arm of
handle_template_valueas a match over the store kinds. This PR moved the path test of that arm intofile_store_path, so the one that lands second merges the two. - #41812 rejects a
ResponseatBun.spawnstdout. This PR accepts aResponseover a file as a shell output target, because main already writes through it for spawned>,>>and2>.
Questions for a maintainer
- Output: is a
ResponseoverBun.file(path)an output target in the shell? Main accepts it for spawned>,>>,2>with wrong flags and throws elsewhere. This PR makes it the path word for every operator. The other choice is to reject it for output. A sliced body is included:echo out > ${new Response(Bun.file(p).slice(1, 3))}replaces the file, as the bare${Bun.file(p).slice(1, 3)}does on main. On main the builtin throws for it, and a spawned command writes at offset 0 and keeps the tail. - Command start or template time: this PR classifies when the command starts, so the redirect uses the body, as on main. A template-time form would make
${response}a path in every position and never use the body. - The relative path base on macOS and Windows (see the ledger).
- Should
Bun.spawn({ stdin: Bun.file(missing) })stop creating the file? It is a separate change.
Verification
bun bd test test/js/bun/shell/bunshell.test.ts: 510 pass, 0 fail. The new block on a debug build of main: 9 pass, 37 fail. The 9 are controls.- Also run with the fix:
bunshell-file,bunshell-instance,file-io,lazy,throw,exec,yield,pipeline_stack,assignments-in-pipeline. test/js/web/fetch/body.test.ts: 775 or 776 of 776 pass. Thestring body consumption does not leakcases time out at 5 s at random in a debug build, on main too.bun run rust:check-all x86_64-pc-windows-msvc aarch64-apple-darwinpasses. The builtincat <content test and the relative path tests run on Windows and macOS only in CI.
Self-review
- Done after review: the classifier became read-only (an early draft turned a stream back into its file before the open, which locked the caller's stream), one
RedirectTargetenum replaced two optional locals in each setup, and an empty path became an ambiguous redirect. - Not done:
test.failingrows for the cells that stay wrong, because they are other defects and are handed off. A test for the second use of aResponse, because it would pin a result that #42786 changes.
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/bunshell.test.ts