diff --git a/lib/net.js b/lib/net.js index d0620449c31e7b..e5dc5a82fea213 100644 --- a/lib/net.js +++ b/lib/net.js @@ -665,7 +665,9 @@ ObjectSetPrototypeOf(Socket, stream.Duplex); // Refresh existing timeouts. Socket.prototype._unrefTimer = function _unrefTimer() { - for (let s = this; s !== null; s = s._parent) { + // `_parent` may be null; we use a loose `!= null` check in case external + // code sets it to undefined. + for (let s = this; s != null; s = s._parent) { if (s[kTimeout]) s[kTimeout].refresh(); } @@ -1026,7 +1028,9 @@ Socket.prototype._destroy = function(exception, cb) { this.connecting = false; - for (let s = this; s !== null; s = s._parent) { + // `_parent` may be null; we use a loose `!= null` check in case external + // code sets it to undefined. + for (let s = this; s != null; s = s._parent) { clearTimeout(s[kTimeout]); } diff --git a/test/parallel/test-net-socket-unref-timer-parent-chain.js b/test/parallel/test-net-socket-unref-timer-parent-chain.js new file mode 100644 index 00000000000000..36a88f1c158e59 --- /dev/null +++ b/test/parallel/test-net-socket-unref-timer-parent-chain.js @@ -0,0 +1,25 @@ +'use strict'; +const common = require('../common'); + +// Walking the `_parent` chain must stop on a nullish link, not only strict +// `null`. During connection teardown a socket's `_parent` can be left +// `undefined`, which previously caused `_unrefTimer()` and `_destroy()` to read +// a property off `undefined` and throw. +// Refs: https://github.com/nodejs/node/issues/64490 + +const assert = require('assert'); +const net = require('net'); + +{ + const socket = new net.Socket(); + socket._parent = undefined; + socket._unrefTimer(); +} + +{ + const socket = new net.Socket(); + socket._parent = undefined; + socket.on('error', common.mustNotCall()); + socket.destroy(); + assert.strictEqual(socket.destroyed, true); +} diff --git a/test/parallel/test-net-unref-timer-parent-undefined.js b/test/parallel/test-net-unref-timer-parent-undefined.js new file mode 100644 index 00000000000000..01e45e82f908e0 --- /dev/null +++ b/test/parallel/test-net-unref-timer-parent-undefined.js @@ -0,0 +1,39 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +// A TLS socket whose `_parent` is left `undefined` during teardown must not +// crash when reads land on it (`onStreamRead` -> `_unrefTimer`) or when it is +// destroyed (`_destroy`). Both walk the `_parent` chain. +// Refs: https://github.com/nodejs/node/issues/64490 + +const options = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), +}; + +const server = tls.createServer(options, common.mustCall((conn) => { + setTimeout(() => conn.write('x'), 50); +})); + +server.listen(0, common.mustCall(() => { + const client = tls.connect({ + port: server.address().port, + rejectUnauthorized: false, + }, common.mustCall(() => { + client._parent = undefined; + })); + + client.on('data', common.mustCall(() => { + server.close(); + client.destroy(); + })); + + client.on('error', common.mustNotCall()); +}));