From 64746c76a9e33a2197efdb89f8bd019deeb06c2d Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Fri, 23 Feb 2018 10:52:25 +0100 Subject: [PATCH] stream: add no-half-open enforcer only if needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The listener does not do anything if `allowHalfOpen` is enabled. Add it only when `allowHalfOpen` is disabled. PR-URL: https://github.com/nodejs/node/pull/18953 Reviewed-By: Tobias Nießen Reviewed-By: Ruben Bridgewater Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- lib/_stream_duplex.js | 11 +++++------ test/parallel/test-http-connect.js | 14 ++++++++------ test/parallel/test-stream-duplex.js | 2 ++ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/_stream_duplex.js b/lib/_stream_duplex.js index 288b8b538c3219..59ce83292789b5 100644 --- a/lib/_stream_duplex.js +++ b/lib/_stream_duplex.js @@ -58,10 +58,10 @@ function Duplex(options) { this.writable = false; this.allowHalfOpen = true; - if (options && options.allowHalfOpen === false) + if (options && options.allowHalfOpen === false) { this.allowHalfOpen = false; - - this.once('end', onend); + this.once('end', onend); + } } Object.defineProperty(Duplex.prototype, 'writableHighWaterMark', { @@ -96,9 +96,8 @@ Object.defineProperty(Duplex.prototype, 'writableLength', { // the no-half-open enforcer function onend() { - // if we allow half-open state, or if the writable side ended, - // then we're ok. - if (this.allowHalfOpen || this._writableState.ended) + // If the writable side ended, then we're ok. + if (this._writableState.ended) return; // no more data can be written. diff --git a/test/parallel/test-http-connect.js b/test/parallel/test-http-connect.js index 9b7432f03a7542..907f42979c3c7c 100644 --- a/test/parallel/test-http-connect.js +++ b/test/parallel/test-http-connect.js @@ -30,6 +30,13 @@ server.on('connect', common.mustCall((req, socket, firstBodyChunk) => { assert.strictEqual(req.method, 'CONNECT'); assert.strictEqual(req.url, 'google.com:443'); + // Make sure this socket has detached. + assert.strictEqual(socket.listeners('close').length, 0); + assert.strictEqual(socket.listeners('drain').length, 0); + assert.strictEqual(socket.listeners('data').length, 0); + assert.strictEqual(socket.listeners('end').length, 1); + assert.strictEqual(socket.listeners('error').length, 1); + socket.write('HTTP/1.1 200 Connection established\r\n\r\n'); let data = firstBodyChunk.toString(); @@ -68,12 +75,7 @@ server.listen(0, common.mustCall(() => { assert.strictEqual(socket.listeners('connect').length, 0); assert.strictEqual(socket.listeners('data').length, 0); assert.strictEqual(socket.listeners('drain').length, 0); - - // the stream.Duplex onend listener - // allow 0 here, so that i can run the same test on streams1 impl - assert(socket.listenerCount('end') <= 2, - `Found ${socket.listenerCount('end')} end listeners`); - + assert.strictEqual(socket.listeners('end').length, 1); assert.strictEqual(socket.listeners('free').length, 0); assert.strictEqual(socket.listeners('close').length, 0); assert.strictEqual(socket.listeners('error').length, 0); diff --git a/test/parallel/test-stream-duplex.js b/test/parallel/test-stream-duplex.js index 1cc54db05206a3..2b2f9a9a0e6265 100644 --- a/test/parallel/test-stream-duplex.js +++ b/test/parallel/test-stream-duplex.js @@ -29,6 +29,8 @@ const stream = new Duplex({ objectMode: true }); assert(Duplex() instanceof Duplex); assert(stream._readableState.objectMode); assert(stream._writableState.objectMode); +assert(stream.allowHalfOpen); +assert.strictEqual(stream.listenerCount('end'), 0); let written; let read;