From 8709a408d2eb31a996077cf0ea48c5fbba93adae Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 2 Sep 2019 17:30:56 +0200 Subject: [PATCH] stream: use more accurate end-of-stream writable and readable detection The value of stream.readable and stream.writable should not be used to detect whether a stream is Writable or Readable. Refs: https://github.com/nodejs/node/issues/29395 PR-URL: https://github.com/nodejs/node/pull/29409 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- lib/internal/streams/end-of-stream.js | 18 ++++++++-- test/parallel/test-stream-finished.js | 48 +++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js index ca6091fe55fe8a..949ab638148d24 100644 --- a/lib/internal/streams/end-of-stream.js +++ b/lib/internal/streams/end-of-stream.js @@ -13,6 +13,18 @@ function isRequest(stream) { return stream.setHeader && typeof stream.abort === 'function'; } +function isReadable(stream) { + return typeof stream.readable === 'boolean' || + typeof stream.readableEnded === 'boolean' || + !!stream._readableState; +} + +function isWritable(stream) { + return typeof stream.writable === 'boolean' || + typeof stream.writableEnded === 'boolean' || + !!stream._writableState; +} + function eos(stream, opts, callback) { if (arguments.length === 2) { callback = opts; @@ -47,8 +59,10 @@ function eos(stream, opts, callback) { }; } - let readable = opts.readable || (opts.readable !== false && stream.readable); - let writable = opts.writable || (opts.writable !== false && stream.writable); + let readable = opts.readable || + (opts.readable !== false && isReadable(stream)); + let writable = opts.writable || + (opts.writable !== false && isWritable(stream)); const onlegacyfinish = () => { if (!stream.writable) onfinish(); diff --git a/test/parallel/test-stream-finished.js b/test/parallel/test-stream-finished.js index c5e792b45e2e4b..12a40b35b97fd7 100644 --- a/test/parallel/test-stream-finished.js +++ b/test/parallel/test-stream-finished.js @@ -318,3 +318,51 @@ const { promisify } = require('util'); })); r.destroy(); } + +{ + // Test is readable check through readable + const streamLike = new EE(); + streamLike.readable = false; + finished(streamLike, common.mustCall()); + streamLike.emit('end'); +} + +{ + // Test is readable check through readableEnded + const streamLike = new EE(); + streamLike.readableEnded = true; + finished(streamLike, common.mustCall()); + streamLike.emit('end'); +} + +{ + // Test is readable check through _readableState + const streamLike = new EE(); + streamLike._readableState = {}; + finished(streamLike, common.mustCall()); + streamLike.emit('end'); +} + +{ + // Test is writable check through writable + const streamLike = new EE(); + streamLike.writable = false; + finished(streamLike, common.mustCall()); + streamLike.emit('finish'); +} + +{ + // Test is writable check through writableEnded + const streamLike = new EE(); + streamLike.writableEnded = true; + finished(streamLike, common.mustCall()); + streamLike.emit('finish'); +} + +{ + // Test is writable check through _writableState + const streamLike = new EE(); + streamLike._writableState = {}; + finished(streamLike, common.mustCall()); + streamLike.emit('finish'); +}