From 1e9ab300b300098237a0d2a29ac5d52d0d3b590d Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 27 Jan 2020 14:46:10 +0100 Subject: [PATCH 1/2] stream: fix finished w/ 'close' before 'finish' Emitting 'close' before 'finish' on a Writable should result in a premature close error. --- lib/internal/streams/end-of-stream.js | 18 ++++++++++++++---- test/parallel/test-stream-finished.js | 13 +++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js index 877db6fbe1cb34..8061ce1849525a 100644 --- a/lib/internal/streams/end-of-stream.js +++ b/lib/internal/streams/end-of-stream.js @@ -25,6 +25,15 @@ function isWritable(stream) { !!stream._writableState; } +function isWritableFinished(stream) { + if (stream.writableFinished) return true; + const wState = stream._writableState; + if (wState && wState.finished) return true; + if (wState && wState.ended && wState.length === 0 && + !wState.errored) return true; + return false; +} + function eos(stream, opts, callback) { if (arguments.length === 2) { callback = opts; @@ -49,10 +58,11 @@ function eos(stream, opts, callback) { if (!stream.writable) onfinish(); }; - let writableEnded = stream._writableState && stream._writableState.finished; + let writableFinished = stream.writableFinished || + (stream._writableState && stream._writableState.finished); const onfinish = () => { writable = false; - writableEnded = true; + writableFinished = true; if (!readable) callback.call(stream); }; @@ -75,8 +85,8 @@ function eos(stream, opts, callback) { err = new ERR_STREAM_PREMATURE_CLOSE(); return callback.call(stream, err); } - if (writable && !writableEnded) { - if (!stream._writableState || !stream._writableState.ended) + if (writable && !writableFinished) { + if (!isWritableFinished(stream)) err = new ERR_STREAM_PREMATURE_CLOSE(); return callback.call(stream, err); } diff --git a/test/parallel/test-stream-finished.js b/test/parallel/test-stream-finished.js index 5b26349d7476e2..f6515a01b8d077 100644 --- a/test/parallel/test-stream-finished.js +++ b/test/parallel/test-stream-finished.js @@ -202,3 +202,16 @@ const { promisify } = require('util'); assert.strictEqual(err.code, 'ERR_STREAM_PREMATURE_CLOSE'); })); } + +{ + const w = new Writable({ + write(chunk, encoding, callback) { + setImmediate(callback); + } + }); + finished(w, common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_STREAM_PREMATURE_CLOSE'); + })); + w.end('asd'); + w.destroy(); +} From 62825805c2a519167fd62ddeee0ed2590df277fa Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Fri, 31 Jan 2020 20:20:32 +0100 Subject: [PATCH 2/2] fixup: simplify --- lib/internal/streams/end-of-stream.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js index 8061ce1849525a..d6e1c5804eaf4a 100644 --- a/lib/internal/streams/end-of-stream.js +++ b/lib/internal/streams/end-of-stream.js @@ -28,10 +28,8 @@ function isWritable(stream) { function isWritableFinished(stream) { if (stream.writableFinished) return true; const wState = stream._writableState; - if (wState && wState.finished) return true; - if (wState && wState.ended && wState.length === 0 && - !wState.errored) return true; - return false; + if (!wState || wState.errored) return false; + return wState.finished || (wState.ended && wState.length === 0); } function eos(stream, opts, callback) {