From da437c1b5b21acd859e9c5fc0aa9f1285000a20d Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 23 Nov 2021 18:11:00 +0100 Subject: [PATCH 1/3] stream: stricter isReadableNodeStream Fixes: https://github.com/nodejs/node/issues/40938 --- lib/internal/streams/end-of-stream.js | 2 +- lib/internal/streams/utils.js | 6 +++++- test/parallel/test-stream-finished.js | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js index 528137418d42db..fd1216b227973f 100644 --- a/lib/internal/streams/end-of-stream.js +++ b/lib/internal/streams/end-of-stream.js @@ -118,7 +118,7 @@ function eos(stream, options, callback) { return callback.call(stream, errored); } - if (readable && !readableFinished) { + if (readable && !readableFinished && isReadableNodeStream(stream, true)) { if (!isReadableFinished(stream, false)) return callback.call(stream, new ERR_STREAM_PREMATURE_CLOSE()); diff --git a/lib/internal/streams/utils.js b/lib/internal/streams/utils.js index 854daae9de41b8..7838494b3153b1 100644 --- a/lib/internal/streams/utils.js +++ b/lib/internal/streams/utils.js @@ -9,11 +9,15 @@ const { const kDestroyed = Symbol('kDestroyed'); const kIsDisturbed = Symbol('kIsDisturbed'); -function isReadableNodeStream(obj) { +function isReadableNodeStream(obj, strict = false) { return !!( obj && typeof obj.pipe === 'function' && typeof obj.on === 'function' && + ( + !strict || + (typeof obj.pause === 'function' && typeof obj.resume === 'function') + ) && (!obj._writableState || obj._readableState?.readable !== false) && // Duplex (!obj._writableState || obj._readableState) // Writable has .pipe. ); diff --git a/test/parallel/test-stream-finished.js b/test/parallel/test-stream-finished.js index 51838a382ee3bb..28406de519e41c 100644 --- a/test/parallel/test-stream-finished.js +++ b/test/parallel/test-stream-finished.js @@ -643,3 +643,19 @@ testClosed((opts) => new Writable({ write() {}, ...opts })); const s = new Stream(); finished(s, common.mustNotCall()); } + +{ + const server = http.createServer(common.mustCall(function (req, res) { + fs.createReadStream(__filename).pipe(res) + finished(res, common.mustCall(function (err) { + if (err) { + throw err + } + })) + })).listen(0, function () { + http.request({ method: 'GET', port: this.address().port }, common.mustCall(function (res) { + res.resume() + server.close() + })).end() + }) +} From 4188695c38c6f7edbb953b38319358ea7d7243f1 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 23 Nov 2021 18:20:46 +0100 Subject: [PATCH 2/3] fixup --- test/parallel/test-stream-finished.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/parallel/test-stream-finished.js b/test/parallel/test-stream-finished.js index 28406de519e41c..f7b569af2a113f 100644 --- a/test/parallel/test-stream-finished.js +++ b/test/parallel/test-stream-finished.js @@ -648,9 +648,7 @@ testClosed((opts) => new Writable({ write() {}, ...opts })); const server = http.createServer(common.mustCall(function (req, res) { fs.createReadStream(__filename).pipe(res) finished(res, common.mustCall(function (err) { - if (err) { - throw err - } + assert.strictEqual(err, undefined); })) })).listen(0, function () { http.request({ method: 'GET', port: this.address().port }, common.mustCall(function (res) { From 133cc2c55270baece72e12e1a927de3e245603ac Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 23 Nov 2021 18:22:56 +0100 Subject: [PATCH 3/3] fixup: lint --- test/parallel/test-stream-finished.js | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-stream-finished.js b/test/parallel/test-stream-finished.js index f7b569af2a113f..b05b8c542190eb 100644 --- a/test/parallel/test-stream-finished.js +++ b/test/parallel/test-stream-finished.js @@ -645,15 +645,18 @@ testClosed((opts) => new Writable({ write() {}, ...opts })); } { - const server = http.createServer(common.mustCall(function (req, res) { - fs.createReadStream(__filename).pipe(res) - finished(res, common.mustCall(function (err) { + const server = http.createServer(common.mustCall(function(req, res) { + fs.createReadStream(__filename).pipe(res); + finished(res, common.mustCall(function(err) { assert.strictEqual(err, undefined); - })) - })).listen(0, function () { - http.request({ method: 'GET', port: this.address().port }, common.mustCall(function (res) { - res.resume() - server.close() - })).end() - }) + })); + })).listen(0, function() { + http.request( + { method: 'GET', port: this.address().port }, + common.mustCall(function(res) { + res.resume(); + server.close(); + }) + ).end(); + }); }