Skip to content

Bun.stdin: reject text/arrayBuffer/bytes/json when process.stdin already holds the reader #35831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 5 commits into from
Closed

Bun.stdin: reject text/arrayBuffer/bytes/json when process.stdin already holds the reader #35831

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
53ead1f
Bun.stdin: route text/json/arrayBuffer/bytes through the cached stream
robobun Jul 25, 2026
dbe1ff0
[autofix.ci] apply automated fixes
autofix-ci[bot] Jul 25, 2026
0f258f3
trim fd_cached_stream doc comment
robobun Jul 25, 2026
5811196
drop fd_cached_stream doc comment
robobun Jul 25, 2026
5c0f05d
fd_cached_stream: gate on JSBlob receiver
robobun Jul 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 51 additions & 8 deletions src/runtime/webcore/Blob.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,17 +210,22 @@ pub trait BlobExt {
get_cached: fn(JSValue) -> Option<JSValue>,
set_cached: fn(JSValue, &JSGlobalObject, JSValue),
) -> JsResult<JSValue>;
fn get_text(&self, global_this: &JSGlobalObject, _: &CallFrame) -> JsResult<JSValue>;
fn fd_cached_stream(&self, this_value: JSValue) -> Option<JSValue>;
fn get_text(&self, global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult<JSValue>;
fn get_text_clone(&self, global_object: &JSGlobalObject) -> Result<JSValue, jsc::JsTerminated>;
fn get_json(&self, global_this: &JSGlobalObject, _: &CallFrame) -> JsResult<JSValue>;
fn get_json(&self, global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult<JSValue>;
fn get_json_share(&self, global_object: &JSGlobalObject) -> Result<JSValue, jsc::JsTerminated>;
fn get_array_buffer_clone(
&self,
global_this: &JSGlobalObject,
) -> Result<JSValue, jsc::JsTerminated>;
fn get_array_buffer(&self, global_this: &JSGlobalObject, _: &CallFrame) -> JsResult<JSValue>;
fn get_array_buffer(
&self,
global_this: &JSGlobalObject,
callframe: &CallFrame,
) -> JsResult<JSValue>;
fn get_bytes_clone(&self, global_this: &JSGlobalObject) -> Result<JSValue, jsc::JsTerminated>;
fn get_bytes(&self, global_this: &JSGlobalObject, _: &CallFrame) -> JsResult<JSValue>;
fn get_bytes(&self, global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult<JSValue>;
fn get_form_data(&self, global_this: &JSGlobalObject, _: &CallFrame) -> JsResult<JSValue>;
fn get_exists_sync(&self) -> JSValue;
fn do_write(&self, global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult<JSValue>;
Expand Down Expand Up @@ -1218,7 +1223,26 @@ impl BlobExt for Blob {

Ok(stream)
}
fn get_text(&self, global_this: &JSGlobalObject, _: &CallFrame) -> JsResult<JSValue> {

fn fd_cached_stream(&self, this_value: JSValue) -> Option<JSValue> {
// `stream_get_cached` is `uncheckedDowncast<JSBlob>`.
js::from_js(this_value)?;
let store = self.store.get().as_deref()?;
let store::Data::File(f) = &store.data else {
return None;
};
if !matches!(f.pathlike, PathOrFileDescriptor::Fd(_)) {
return None;
}
js::stream_get_cached(this_value)
}
Comment thread
robobun marked this conversation as resolved.

fn get_text(&self, global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult<JSValue> {
if let Some(stream) = self.fd_cached_stream(callframe.this()) {
return bun_jsc::from_js_host_call(global_this, || {
global_this.readable_stream_to_text(stream)
});
}
Ok(self.get_text_clone(global_this)?)
}

Expand All @@ -1227,7 +1251,12 @@ impl BlobExt for Blob {
JSPromise::wrap(global_object, |g| self.to_string(g, Lifetime::Clone))
}

fn get_json(&self, global_this: &JSGlobalObject, _: &CallFrame) -> JsResult<JSValue> {
fn get_json(&self, global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult<JSValue> {
if let Some(stream) = self.fd_cached_stream(callframe.this()) {
return bun_jsc::from_js_host_call(global_this, || {
global_this.readable_stream_to_json(stream)
});
}
Ok(self.get_json_share(global_this)?)
}

Expand All @@ -1244,7 +1273,16 @@ impl BlobExt for Blob {
JSPromise::wrap(global_this, |g| self.to_array_buffer(g, Lifetime::Clone))
}

fn get_array_buffer(&self, global_this: &JSGlobalObject, _: &CallFrame) -> JsResult<JSValue> {
fn get_array_buffer(
&self,
global_this: &JSGlobalObject,
callframe: &CallFrame,
) -> JsResult<JSValue> {
if let Some(stream) = self.fd_cached_stream(callframe.this()) {
return bun_jsc::from_js_host_call(global_this, || {
global_this.readable_stream_to_array_buffer(stream)
});
}
Ok(self.get_array_buffer_clone(global_this)?)
}

Expand All @@ -1253,7 +1291,12 @@ impl BlobExt for Blob {
JSPromise::wrap(global_this, |g| self.to_uint8_array(g, Lifetime::Clone))
}

fn get_bytes(&self, global_this: &JSGlobalObject, _: &CallFrame) -> JsResult<JSValue> {
fn get_bytes(&self, global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult<JSValue> {
if let Some(stream) = self.fd_cached_stream(callframe.this()) {
return bun_jsc::from_js_host_call(global_this, || {
global_this.readable_stream_to_bytes(stream)
});
}
Comment on lines +1292 to +1297

@claude claude Bot Jul 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 get_form_data (the fifth sibling, immediately below) still bypasses fd_cached_stream and goes to_form_data → needs_to_read_file() → do_read_file::<ToFormDataWithBytesFn> (Blob.rs:3236-3237), so Bun.stdin.formData() still races a raw read(2) on fd 0 against process.stdin — the exact defect this PR fixes for the other four. formData is on Blob's prototype (response.classes.ts:164) and readable_stream_to_form_data already exists (JSGlobalObject.rs:1188, used by Body.rs:401), so the same-shape fix applies; REVIEW.md makes sibling coverage required scope ("Fix the whole class in the same PR … Grep for every sibling site sharing the pattern").

Extended reasoning...

What the bug is

The PR routes get_text, get_json, get_array_buffer, and get_bytes through the new fd_cached_stream helper so that when process.stdin (built on Bun.stdin.stream()'s cached reader) already holds fd 0, they reject with ERR_INVALID_STATE instead of silently splitting bytes. But the fifth sibling read helper, get_form_data at Blob.rs:1301-1305 — sitting immediately after get_bytes in the same impl block — was not updated. It still calls self.to_form_data(...), which at Blob.rs:3236-3237 does:

if self.needs_to_read_file() {
    return Ok(self.do_read_file::<ToFormDataWithBytesFn>(global));
}

That is the identical raw-read(2)-on-fd-0 code path this PR set out to eliminate.

The code path that triggers it

Bun.stdin is an fd-backed Blob (PathOrFileDescriptor::Fd(0)), so needs_to_read_file() is true. formData is exposed on the Blob prototype at response.classes.ts:164 (formData: { fn: "getFormData", async: true }), so Bun.stdin.formData() is user-reachable. When a program has process.stdin.on('data', ...) armed and then awaits Bun.stdin.formData(), do_read_file schedules a ReadFile task that calls bun_sys::read(fd 0, ...) on the work pool, racing with process.stdin's FileReader — the exact silent-split defect described in the PR body.

Why existing code doesn't prevent it

The "stdin has no content-type, so formData() will reject anyway" objection doesn't help: to_form_data calls do_read_file before any encoding check — the content-type validation happens inside to_form_data_with_bytes, which runs after the file has been fully read. So even though the eventual FormData parse may reject, the raw read(2) has already consumed bytes from fd 0 and stolen them from process.stdin's reader. The user still sees their piped input silently split, plus an unrelated-looking encoding error.

Impact

Same class of bug the PR exists to fix, on the one sibling method it skipped. Per REVIEW.md this is required scope, not scope creep:

Fix the whole class in the same PR (same-class sites are ONE concern, not scope creep). Grep for every sibling site sharing the pattern … If a site is intentionally excluded, say so in the PR.

The PR description does not mention excluding formData. The trait signature at Blob.rs:229 (fn get_form_data(&self, global_this: &JSGlobalObject, _: &CallFrame)) even still has the anonymous _ for the callframe, while the other four were renamed to callframe in this diff — a visible sibling-missed marker.

Step-by-step proof

  1. process.stdin.on('data', ...) — creates the readable-flowing wrapper, which calls Bun.stdin.stream().getReader() and populates JSBlob's cached m_stream slot; the stream is now locked.
  2. Bun.stdin.formData() → generated getFormData → Blob::get_form_data(self, global, callframe) at Blob.rs:1301.
  3. No fd_cached_stream check — goes straight to self.to_form_data(g, Lifetime::Temporary).
  4. to_form_data at Blob.rs:3236: self.needs_to_read_file() is true (fd-backed store) → self.do_read_file::<ToFormDataWithBytesFn>(global).
  5. do_read_file schedules a ReadFile task on the work pool that loops bun_sys::read(fd 0, buf) until EOF.
  6. Meanwhile process.stdin's FileReader is also reading fd 0 on the event loop. The kernel hands each byte to whichever read(2) arrives first — non-deterministic split, exactly what the PR body's repro shows for arrayBuffer().
  7. After the read completes, to_form_data_with_bytes runs and (with no content-type) rejects — but the bytes are already gone from process.stdin's view.

How to fix

Same shape as the other four. readable_stream_to_form_data already exists at JSGlobalObject.rs:1188 and is already used by Body.rs:400-401 for the Request/Response path:

fn get_form_data(&self, global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult<JSValue> {
    if let Some(stream) = self.fd_cached_stream(callframe.this()) {
        let content_type = self
            .get_form_data_encoding()
            .map(|fd| fd.encoding.to_js(global_this))
            .unwrap_or(JSValue::undefined());
        return bun_jsc::from_js_host_call(global_this, || {
            global_this.readable_stream_to_form_data(stream, content_type)
        });
    }
    let _store = self.store.get().clone();
    Ok(JSPromise::wrap(global_this, |g| {
        self.to_form_data(g, Lifetime::Temporary)
    })?)
}

(and add "formData" to the describe.each matrix in bun-stdin-locked.test.ts — REVIEW.md: "Cover the variant matrix … every sibling entry point receiving the same fix".)

Note this shares the same callframe.this()-receiver hazard already flagged for the other four (the JSBuildArtifact comment on line 1242) — whatever fix is applied there should apply here too.

Ok(self.get_bytes_clone(global_this)?)
}

Expand Down
63 changes: 63 additions & 0 deletions test/js/bun/util/bun-stdin-locked.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";

// process.stdin is built on Bun.stdin.stream()'s reader. The Blob read helpers
// on Bun.stdin (text/json/arrayBuffer/bytes) used to bypass that stream and
// read fd 0 directly, so a program that armed process.stdin *and* awaited
// Bun.stdin.arrayBuffer() would see the piped bytes split between the two
// consumers with no error. They now route through the same cached stream and
// reject with ERR_INVALID_STATE, matching Bun.stdin.stream().getReader().

const payload = Buffer.alloc(256 * 1024, "x").toString();

describe.each(["arrayBuffer", "bytes", "text", "json"] as const)(
"Bun.stdin.%s() rejects when process.stdin holds the reader",
method => {
test.concurrent(method, async () => {
const child = `
let n = 0;
process.stdin.on("data", c => (n += c.length));
let result = { state: "pending" };
Bun.stdin.${method}().then(
() => { result = { state: "resolved" }; },
e => { result = { state: "rejected", code: e && (e.code || e.name) }; },
);
await new Promise(r => process.stdin.once("end", r));
await Promise.resolve();
process.stdout.write(JSON.stringify({ n, result }));
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", child],
env: bunEnv,
stdin: "pipe",
stdout: "pipe",
stderr: "pipe",
});
proc.stdin.write(payload);
await proc.stdin.end();
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(JSON.parse(stdout)).toEqual({
n: payload.length,
result: { state: "rejected", code: "ERR_INVALID_STATE" },
});
expect(exitCode).toBe(0);
});
},
);

test.concurrent("Bun.stdin.arrayBuffer() with no other consumer reads every byte", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", `process.stdout.write(String((await Bun.stdin.arrayBuffer()).byteLength));`],
env: bunEnv,
stdin: "pipe",
stdout: "pipe",
stderr: "pipe",
});
proc.stdin.write(payload);
await proc.stdin.end();
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(stdout).toBe(String(payload.length));
expect(exitCode).toBe(0);
});
Loading