net: do not inherit the no-half-open enforcer

lpinca · MylesBorins · commit 5b12d3a58ee8 · 2018-03-20T11:33:42.000Z
`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`. PR-URL: #18974 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chen Gang <gangc.cxy@foxmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
diff --git a/lib/net.js b/lib/net.js
@@ -50,6 +50,7 @@ const { async_id_symbol } = process.binding('async_wrap');
 const { newUid, defaultTriggerAsyncIdScope } = require('internal/async_hooks');
 const { nextTick } = require('internal/process/next_tick');
 const errors = require('internal/errors');
+const DuplexBase = require('internal/streams/duplex_base');
 const dns = require('dns');
 
 const kLastWriteQueueSize = Symbol('lastWriteQueueSize');
@@ -211,7 +212,11 @@ function Socket(options) {
   else if (options === undefined)
     options = {};
 
-  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
@@ -236,8 +241,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;
@@ -256,7 +259,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
@@ -68,12 +68,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
@@ -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);
