shell: open a Response over Bun.file(path) as a file redirect

RO
16d ago
+343-82

Problem

  • $`cmd < ${new Response(Bun.file(path))}` runs cmd on an empty stdin at exit 0 when the file is missing, and creates the file. The bare ${Bun.file(path)} exits 1 with bun: 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 reads blob.shared_view(), empty for a file.
  • On that arm > does not truncate and >> does not append.

Fix

  • Interpreter::redirect_target classifies a redirect once. A Response over Bun.file(path) is a path word: both redirect setups open it with shell_openat and 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 Windows cat), 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_CREAT in 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 < fifo does.
  • .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 a Response over an existing file on POSIX, already works on main.
  • await fetch("file:///missing") resolves with ok: true and status 200, so cat < ${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.

ChangeCellsWhich
Silent exit 0 becomes exit 1 that names the file3spawned < missing (main also creates the file), builtin < missing and missing directory
Wrong content at exit 0 becomes right content3spawned >, >>, 2> to an existing file (main writes at offset 0 and keeps the tail)
A throw becomes a write at exit 010builtin >, >>, 2>, &> to an existing or missing file (8), spawned &> (2)
A throw becomes a shell error, exit 15the same 5 pairs into a missing directory
Error text only4spawned <, >, >>, 2> into a missing directory: main names the command, this branch names the file
Unchanged5spawned < 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.bodyUsed stays false, 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.rs and src/spawn/process.rs, not run). response.text() keeps the process cwd.
  • Bun.file("") in a Response gives bun: ambiguous redirect, as the bare form does.
  • Windows only, read from the code and not run: > to such a Response when the file has the hidden or system attribute. On main a spawned command writes into the file, because spawn opens it with OPEN_ALWAYS. Here the open fails with EPERM, as > hidden.txt and the bare form do on main: shell_openat opens with CREATE_ALWAYS and 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 .body was read, new Response(Bun.file(path).stream()), .clone(), and fetch() of a file: URL.

What stays as on main

  • A second redirect from the same Response reads 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 a Response or 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, builtin cat < ${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:801 is 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 is Bun.build with a NUL in naming or outdir, 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, builtin echo: no redirect 1105 to 1101, path word 1587 to 1418, Buffer 1572 to 1588, Blob 1419 to 1446, in-memory Response 1730 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 Response 205.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, socketpair are equal for no redirect, a path word, a Buffer and an in-memory Response, spawned and builtin.
  • Syscalls, Response over an existing file: spawned net +1 (bun: +1 openat, +1 fcntl, +1 close. The child before exec: -1 openat, -1 close). Builtin: +1 openat, +1 close (main opens nothing).
  • Scans of a string Response body per redirect (calls of to_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 < fifo takes 1452 ms on both.
  • Opens of a missing read target with O_CREAT: each spawned shell form goes from 1 to 0. The 7 Bun.spawn stdin forms keep theirs.
  • strace, valgrind and bloaty are not installed on the machine. The counts come from a ptrace counter, gdb and size.

Related changes

  • #42786 rejects a Response body 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 puts throw_if_body_not_in_memory on the RedirectTarget::Object arm and merges the two docs sentences.
  • #46665 gives Bun.spawn path slots one flags table. It also sends a Response over a path file to shell_openat for spawned commands, in Cmd.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 a Response becomes a path word only in redirect_target.
  • #37641 removes O_CREAT from the stdin path open in spawn. That is a Bun.spawn default 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_value as a match over the store kinds. This PR moved the path test of that arm into file_store_path, so the one that lands second merges the two.
  • #41812 rejects a Response at Bun.spawn stdout. This PR accepts a Response over a file as a shell output target, because main already writes through it for spawned >, >> and 2>.

Questions for a maintainer

  1. Output: is a Response over Bun.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.
  2. 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.
  3. The relative path base on macOS and Windows (see the ledger).
  4. 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. The string body consumption does not leak cases 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-darwin passes. The builtin cat < 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 RedirectTarget enum replaced two optional locals in each setup, and an empty path became an ambiguous redirect.
  • Not done: test.failing rows for the cells that stay wrong, because they are other defects and are handed off. A test for the second use of a Response, 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