From 53dd60b83af797bff39813a7e2050af705c0c047 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 3 Aug 2021 09:01:31 +0200 Subject: [PATCH 1/2] stream: don't emit 'data' after 'error' or 'close' As per doc we should not emit further events after 'close' and only 'close' after 'error'. Fixes: https://github.com/nodejs/node/issues/39630 --- lib/internal/streams/readable.js | 5 +- test/parallel/test-stream-readable-destroy.js | 83 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/lib/internal/streams/readable.js b/lib/internal/streams/readable.js index a6a1913961bb41..4dd2117546862a 100644 --- a/lib/internal/streams/readable.js +++ b/lib/internal/streams/readable.js @@ -276,6 +276,8 @@ function readableAddChunk(stream, chunk, encoding, addToFront) { if (addToFront) { if (state.endEmitted) errorOrDestroy(stream, new ERR_STREAM_UNSHIFT_AFTER_END_EVENT()); + else if (state.destroyed || state.errored) + return false; else addChunk(stream, state, chunk, true); } else if (state.ended) { @@ -316,6 +318,7 @@ function addChunk(stream, state, chunk, addToFront) { } else { state.awaitDrainWriters = null; } + state.dataEmitted = true; stream.emit('data', chunk); } else { @@ -542,7 +545,7 @@ Readable.prototype.read = function(n) { endReadable(this); } - if (ret !== null) { + if (ret !== null && !state.errorEmitted && !state.closeEmitted) { state.dataEmitted = true; this.emit('data', ret); } diff --git a/test/parallel/test-stream-readable-destroy.js b/test/parallel/test-stream-readable-destroy.js index 9ba3f9cd3653d6..759375c7ca81e7 100644 --- a/test/parallel/test-stream-readable-destroy.js +++ b/test/parallel/test-stream-readable-destroy.js @@ -319,3 +319,86 @@ const assert = require('assert'); })(), /AbortError/); setTimeout(() => controller.abort(), 0); } + +{ + const read = new Readable({ + read() { + }, + }); + read.push('asd'); + + read.on('data', common.mustNotCall()); + read.on('error', common.mustCall((e) => { + read.read(); + })); + read.on('close', common.mustCall((e) => { + read.read(); + })); + read.destroy(new Error('asd')); +} + +{ + const read = new Readable({ + read() { + }, + }); + read.push('asd'); + + read.on('data', common.mustNotCall()); + read.on('close', common.mustCall((e) => { + read.read(); + })); + read.destroy(); +} + +{ + const read = new Readable({ + read() { + }, + }); + read.push('asd'); + + read.on('data', common.mustNotCall()); + read.on('close', common.mustCall((e) => { + read.unshift('asd'); + })); + read.destroy(); +} + +{ + const read = new Readable({ + read() { + }, + }); + read.push('asd'); + + read.on('data', common.mustNotCall()); + read.destroy(); + read.unshift('asd'); +} + +{ + const read = new Readable({ + read() { + }, + }); + read.push('asd'); + + read.on('data', common.mustNotCall()); + read.on('close', common.mustCall((e) => { + read.push('asd'); + })); + read.destroy(); +} + +{ + const read = new Readable({ + read() { + }, + }); + read.push('asd'); + + read.on('data', common.mustNotCall()); + read.destroy(); + read.push('asd'); +} From e0468456c1499ba59dac7ff35f1d6661372e262c Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Thu, 5 Aug 2021 14:01:58 +0200 Subject: [PATCH 2/2] fixup --- test/parallel/test-stream-readable-destroy.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-stream-readable-destroy.js b/test/parallel/test-stream-readable-destroy.js index 759375c7ca81e7..ffea6cb9c869f3 100644 --- a/test/parallel/test-stream-readable-destroy.js +++ b/test/parallel/test-stream-readable-destroy.js @@ -325,13 +325,14 @@ const assert = require('assert'); read() { }, }); - read.push('asd'); read.on('data', common.mustNotCall()); read.on('error', common.mustCall((e) => { + read.push('asd'); read.read(); })); read.on('close', common.mustCall((e) => { + read.push('asd'); read.read(); })); read.destroy(new Error('asd')); @@ -342,10 +343,10 @@ const assert = require('assert'); read() { }, }); - read.push('asd'); read.on('data', common.mustNotCall()); read.on('close', common.mustCall((e) => { + read.push('asd'); read.read(); })); read.destroy(); @@ -356,10 +357,10 @@ const assert = require('assert'); read() { }, }); - read.push('asd'); read.on('data', common.mustNotCall()); read.on('close', common.mustCall((e) => { + read.push('asd'); read.unshift('asd'); })); read.destroy(); @@ -370,7 +371,6 @@ const assert = require('assert'); read() { }, }); - read.push('asd'); read.on('data', common.mustNotCall()); read.destroy(); @@ -382,8 +382,8 @@ const assert = require('assert'); read() { }, }); - read.push('asd'); + read.resume(); read.on('data', common.mustNotCall()); read.on('close', common.mustCall((e) => { read.push('asd'); @@ -396,7 +396,6 @@ const assert = require('assert'); read() { }, }); - read.push('asd'); read.on('data', common.mustNotCall()); read.destroy();