From 431638d4fefebb400926aa11f5482b42f3e5059f Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 18 Oct 2016 22:30:03 +0200 Subject: [PATCH 1/3] streams: fix regression in `unpipe()` Since 2e568d9 there is a bug where unpiping a stream from a readable stream that has `_readableState.pipesCount > 1` will cause it to remove the first stream in the `_.readableState.pipes` array no matter where in the list the `dest` stream was. This patch corrects that problem. Fixes: https://github.com/nodejs/node/issues/9170 --- lib/_stream_readable.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 9438bf92d34ee2..0844604d960258 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -671,7 +671,7 @@ Readable.prototype.unpipe = function(dest) { if (index === -1) return this; - state.pipes.splice(i, 1); + state.pipes.splice(index, 1); state.pipesCount -= 1; if (state.pipesCount === 1) state.pipes = state.pipes[0]; From b41cf22fbee064bef7fdcd44848ae6907e89bee4 Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Tue, 18 Oct 2016 22:30:03 +0200 Subject: [PATCH 2/3] test: add regression test for `unpipe()` Since 2e568d9 there is a bug where unpiping a stream from a readable stream that has `_readableState.pipesCount > 1` will cause it to remove the first stream in the `_.readableState.pipes` array no matter where in the list the `dest` stream was. --- .../test-stream-pipe-unpipe-streams.js | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 test/parallel/test-stream-pipe-unpipe-streams.js diff --git a/test/parallel/test-stream-pipe-unpipe-streams.js b/test/parallel/test-stream-pipe-unpipe-streams.js new file mode 100644 index 00000000000000..55362253063abd --- /dev/null +++ b/test/parallel/test-stream-pipe-unpipe-streams.js @@ -0,0 +1,26 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const { Readable, Writable } = require('stream'); + +const source = Readable({read: () => {}}); +const dest1 = Writable({write: () => {}}); +const dest2 = Writable({write: () => {}}); + +source.pipe(dest1); +source.pipe(dest2); + +dest1.on('unpipe', common.mustCall(() => {})); +dest2.on('unpipe', common.mustCall(() => {})); + +// Should be able to unpipe them in the reverse order that they were piped. + +source.unpipe(dest2); + +assert.strictEqual(source._readableState.pipes, dest1); +assert.notStrictEqual(source._readableState.pipes, dest2); + +source.unpipe(dest1); + +assert.strictEqual(source._readableState.pipes, null); From f140c3f6be29f94fa0d6fa75df8006c1bf826bae Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 18 Oct 2016 23:14:12 +0200 Subject: [PATCH 3/3] [squash] addres nit for checking `_readableState.pipes` before unpiping --- test/parallel/test-stream-pipe-unpipe-streams.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/parallel/test-stream-pipe-unpipe-streams.js b/test/parallel/test-stream-pipe-unpipe-streams.js index 55362253063abd..c51c100d0f6802 100644 --- a/test/parallel/test-stream-pipe-unpipe-streams.js +++ b/test/parallel/test-stream-pipe-unpipe-streams.js @@ -14,6 +14,10 @@ source.pipe(dest2); dest1.on('unpipe', common.mustCall(() => {})); dest2.on('unpipe', common.mustCall(() => {})); +assert.strictEqual(source._readableState.pipes[0], dest1); +assert.strictEqual(source._readableState.pipes[1], dest2); +assert.strictEqual(source._readableState.pipes.length, 2); + // Should be able to unpipe them in the reverse order that they were piped. source.unpipe(dest2);