From a26c3a0f00a1c4942faca98e4cc176c7894d2530 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 2 Sep 2019 17:30:56 +0200 Subject: [PATCH 1/2] stream: eos more accurate 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: #29395 --- lib/internal/streams/end-of-stream.js | 16 +++++++-- test/parallel/test-stream-finished.js | 48 +++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js index ca6091fe55fe8a..97971dcda2f1b5 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,8 @@ 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'); +} From 5b6e33a0bbca1e8901799db045aa72093199802d Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 2 Sep 2019 22:42:00 +0200 Subject: [PATCH 2/2] fixup: linting --- lib/internal/streams/end-of-stream.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js index 97971dcda2f1b5..949ab638148d24 100644 --- a/lib/internal/streams/end-of-stream.js +++ b/lib/internal/streams/end-of-stream.js @@ -16,13 +16,13 @@ function isRequest(stream) { function isReadable(stream) { return typeof stream.readable === 'boolean' || typeof stream.readableEnded === 'boolean' || - !!stream._readableState + !!stream._readableState; } function isWritable(stream) { return typeof stream.writable === 'boolean' || typeof stream.writableEnded === 'boolean' || - !!stream._writableState + !!stream._writableState; } function eos(stream, opts, callback) { @@ -59,8 +59,10 @@ function eos(stream, opts, callback) { }; } - let readable = opts.readable || (opts.readable !== false && isReadable(stream)); - let writable = opts.writable || (opts.writable !== false && isWritable(stream)); + let readable = opts.readable || + (opts.readable !== false && isReadable(stream)); + let writable = opts.writable || + (opts.writable !== false && isWritable(stream)); const onlegacyfinish = () => { if (!stream.writable) onfinish();