From b970c5d2fd30a572c1792d88a2c053fba7384e97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caleb=20=E3=83=84=20Everett?= Date: Thu, 10 Sep 2026 15:27:13 -0700 Subject: [PATCH] stream: fix async iteration of undefined chunks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hand-rolled async iterator introduced in https://github.com/nodejs/node/pull/64447 unwraps thenable chunks before delivering them, but guards the `then` access with `chunk !== null` while the dereference itself requires `chunk != null`. An object mode stream carrying an `undefined` chunk therefore threw TypeError: Cannot read properties of undefined (reading 'then') Because the throw happens in a microtask rather than rejecting the iterator's promise, it surfaces as an uncaught exception that terminates the process instead of a catchable stream error. `undefined` is a legal chunk value: doc/api/stream.md documents object mode chunks as "any JavaScript value other than `null`" and reserves `null` as the end-of-stream sentinel. `readable.push(undefined)` returns true in object mode, so the stream accepts the value and then crashes on the way out. Read `then` with optional chaining at both sites so that `undefined` is delivered as a value while `null` keeps signalling end-of-stream. `then` is still read at most once, so a getter cannot observe a second access. Refs: https://github.com/nodejs/node/pull/64447 Assisted-by: a closed-source coding agent Signed-off-by: Caleb ツ Everett --- lib/internal/streams/readable.js | 10 +++-- .../test-stream-readable-async-iterators.js | 38 +++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/lib/internal/streams/readable.js b/lib/internal/streams/readable.js index 847b3837af4..fa6ed790083 100644 --- a/lib/internal/streams/readable.js +++ b/lib/internal/streams/readable.js @@ -1522,8 +1522,9 @@ function createAsyncIterator(stream, options) { const chunk = stream.destroyed ? null : stream.read(); if (chunk !== null) { // Read `then` only once so that a getter cannot observe (or throw - // on) a second access. - const then = chunk.then; + // on) a second access. `undefined` is a valid chunk value, so it must + // not be dereferenced here. + const then = chunk?.then; if (typeof then === 'function') { FunctionPrototypeCall(then, chunk, (value) => { inFlight = false; @@ -1595,8 +1596,9 @@ function createAsyncIterator(stream, options) { const chunk = stream.destroyed ? null : stream.read(); if (chunk !== null) { // Read `then` only once so that a getter cannot observe (or - // throw on) a second access. - const then = chunk.then; + // throw on) a second access. `undefined` is a valid chunk value, + // so it must not be dereferenced here. + const then = chunk?.then; if (typeof then === 'function') { inFlight = true; return FunctionPrototypeCall( diff --git a/test/parallel/test-stream-readable-async-iterators.js b/test/parallel/test-stream-readable-async-iterators.js index f9bcaea6057..fb4b052bf28 100644 --- a/test/parallel/test-stream-readable-async-iterators.js +++ b/test/parallel/test-stream-readable-async-iterators.js @@ -989,5 +989,43 @@ async function tests() { })().then(common.mustCall()); } +{ + // An `undefined` chunk is a value, not end-of-stream. Here it is already + // buffered, so it is read on the synchronous fast path. + (async () => { + const r = new Readable({ objectMode: true, read() {} }); + r.push(undefined); + r.push(null); + + const it = r[Symbol.asyncIterator](); + assert.deepStrictEqual(await it.next(), { done: false, value: undefined }); + assert.strictEqual((await it.next()).done, true); + })().then(common.mustCall()); +} + +{ + // An `undefined` chunk pushed after next() is delivered once it arrives. + (async () => { + const r = new Readable({ objectMode: true, read() {} }); + const it = r[Symbol.asyncIterator](); + const next = it.next(); + setImmediate(() => { + r.push(undefined); + r.push(null); + }); + + assert.deepStrictEqual(await next, { done: false, value: undefined }); + assert.strictEqual((await it.next()).done, true); + })().then(common.mustCall()); +} + +{ + // Readable.from() delivers `undefined` values. + (async () => { + assert.deepStrictEqual(await Readable.from([undefined]).toArray(), + [undefined]); + })().then(common.mustCall()); +} + // To avoid missing some tests if a promise does not resolve tests().then(common.mustCall());