Skip to content

Commit 5b12d3a

Browse files
lpincaMylesBorins
authored andcommitted
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`. PR-URL: #18974 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chen Gang <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent 152c931 commit 5b12d3a

File tree

3 files changed

+19
-10
lines changed

3 files changed

+19
-10
lines changed

lib/net.js

+7-4
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const { async_id_symbol } = process.binding('async_wrap');
5050
const { newUid, defaultTriggerAsyncIdScope } = require('internal/async_hooks');
5151
const { nextTick } = require('internal/process/next_tick');
5252
const errors = require('internal/errors');
53+
const DuplexBase = require('internal/streams/duplex_base');
5354
const dns = require('dns');
5455

5556
const kLastWriteQueueSize = Symbol('lastWriteQueueSize');
@@ -211,7 +212,11 @@ function Socket(options) {
211212
else if (options === undefined)
212213
options = {};
213214

214-
stream.Duplex.call(this, options);
215+
// `DuplexBase` is just a slimmed down constructor for `Duplex` which allow
216+
// us to not inherit the "no-half-open enforcer" as there is already one in
217+
// place. Instances of `Socket` are still instances of `Duplex`, that is,
218+
// `socket instanceof Duplex === true`.
219+
DuplexBase.call(this, options);
215220

216221
if (options.handle) {
217222
this._handle = options.handle; // private
@@ -236,8 +241,6 @@ function Socket(options) {
236241
this._writev = null;
237242
this._write = makeSyncWrite(fd);
238243
}
239-
this.readable = options.readable !== false;
240-
this.writable = options.writable !== false;
241244
} else {
242245
// these will be set once there is a connection
243246
this.readable = this.writable = false;
@@ -256,7 +259,7 @@ function Socket(options) {
256259
this._writableState.decodeStrings = false;
257260

258261
// default to *not* allowing half open sockets
259-
this.allowHalfOpen = options && options.allowHalfOpen || false;
262+
this.allowHalfOpen = options.allowHalfOpen || false;
260263

261264
// if we have a handle, then start the flow of data into the
262265
// buffer. if not, then this will happen when we connect

test/parallel/test-http-connect.js

+1-6
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,7 @@ server.listen(0, common.mustCall(() => {
6868
assert.strictEqual(socket.listeners('connect').length, 0);
6969
assert.strictEqual(socket.listeners('data').length, 0);
7070
assert.strictEqual(socket.listeners('drain').length, 0);
71-
72-
// the stream.Duplex onend listener
73-
// allow 0 here, so that i can run the same test on streams1 impl
74-
assert(socket.listenerCount('end') <= 2,
75-
`Found ${socket.listenerCount('end')} end listeners`);
76-
71+
assert.strictEqual(socket.listeners('end').length, 1);
7772
assert.strictEqual(socket.listeners('free').length, 0);
7873
assert.strictEqual(socket.listeners('close').length, 0);
7974
assert.strictEqual(socket.listeners('error').length, 0);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
'use strict';
2+
require('../common');
3+
4+
// This test ensures that `net.Socket` does not inherit the no-half-open
5+
// enforcer from `stream.Duplex`.
6+
7+
const { Socket } = require('net');
8+
const { strictEqual } = require('assert');
9+
10+
const socket = new Socket({ allowHalfOpen: false });
11+
strictEqual(socket.listenerCount('end'), 1);

0 commit comments

Comments
 (0)