From 5b73a7919cbafe735e90cc1b81d3650b83bd0b80 Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Sat, 24 Feb 2018 09:44:42 +0100 Subject: [PATCH] net: do not inherit the no-half-open enforcer `Socket.prototype.destroySoon()` is called as soon as `UV_EOF` is read if the `allowHalfOpen` option is disabled. This already works as a "no-half-open enforcer" so there is no need to inherit another from `stream.Duplex`. --- lib/net.js | 11 +++++++---- test/parallel/test-http-connect.js | 7 +------ test/parallel/test-net-socket-no-halfopen-enforcer.js | 11 +++++++++++ 3 files changed, 19 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-net-socket-no-halfopen-enforcer.js diff --git a/lib/net.js b/lib/net.js index f2cb423f3003ea..7fa1e20aedaec5 100644 --- a/lib/net.js +++ b/lib/net.js @@ -64,6 +64,7 @@ const { ERR_SOCKET_BAD_PORT, ERR_SOCKET_CLOSED } = errors.codes; +const DuplexBase = require('internal/streams/duplex_base'); const dns = require('dns'); const kLastWriteQueueSize = Symbol('lastWriteQueueSize'); @@ -238,7 +239,11 @@ function Socket(options) { // For backwards compat do not emit close on destroy. options.emitClose = false; - stream.Duplex.call(this, options); + // `DuplexBase` is just a slimmed down constructor for `Duplex` which allow + // us to not inherit the "no-half-open enforcer" as there is already one in + // place. Instances of `Socket` are still instances of `Duplex`, that is, + // `socket instanceof Duplex === true`. + DuplexBase.call(this, options); if (options.handle) { this._handle = options.handle; // private @@ -263,8 +268,6 @@ function Socket(options) { this._writev = null; this._write = makeSyncWrite(fd); } - this.readable = options.readable !== false; - this.writable = options.writable !== false; } else { // these will be set once there is a connection this.readable = this.writable = false; @@ -282,7 +285,7 @@ function Socket(options) { this._writableState.decodeStrings = false; // default to *not* allowing half open sockets - this.allowHalfOpen = options && options.allowHalfOpen || false; + this.allowHalfOpen = options.allowHalfOpen || false; // if we have a handle, then start the flow of data into the // buffer. if not, then this will happen when we connect diff --git a/test/parallel/test-http-connect.js b/test/parallel/test-http-connect.js index f90d235521a649..ec2c8846bbb134 100644 --- a/test/parallel/test-http-connect.js +++ b/test/parallel/test-http-connect.js @@ -75,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-net-socket-no-halfopen-enforcer.js b/test/parallel/test-net-socket-no-halfopen-enforcer.js new file mode 100644 index 00000000000000..3df5b6d7b9c8cb --- /dev/null +++ b/test/parallel/test-net-socket-no-halfopen-enforcer.js @@ -0,0 +1,11 @@ +'use strict'; +require('../common'); + +// This test ensures that `net.Socket` does not inherit the no-half-open +// enforcer from `stream.Duplex`. + +const { Socket } = require('net'); +const { strictEqual } = require('assert'); + +const socket = new Socket({ allowHalfOpen: false }); +strictEqual(socket.listenerCount('end'), 1);