net: handle undefined parent in _unrefTimer and _destroy · nodejs/node@646221a · GitHub
Skip to content

Commit 646221a

Browse files
Shivay-98aduh95
authored andcommitted
net: handle undefined parent in _unrefTimer and _destroy
The fix and approach are from #64491 by Shivay-98; this reopens it to get it landed, since the original stalled awaiting requested changes. `Socket.prototype._unrefTimer` and `Socket.prototype._destroy` both walk the `_parent` chain with a strict `!== null` check. During connection teardown a socket's `_parent` can be left `undefined` (for example a TLS socket layered over another stream), so the loop steps onto `undefined` and reads a property off it, throwing a TypeError: Cannot read properties of undefined (reading 'Symbol(timeout)') from an uncaught I/O callback and crashing the process. Using a nullish (`!= null`) check terminates the walk on both `null` and `undefined`. [petter@hightouch.io: apply the same fix to the identical loop in `_destroy`, which the original regression test already exercised via `destroy()`; add direct unit coverage for both paths.] Fixes: #64490 Refs: #64491 Signed-off-by: Petter Häggholm <petter@hightouch.io> PR-URL: #64644 Fixes: #64490 Refs: #64491 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent cce803b commit 646221a

3 files changed

Lines changed: 70 additions & 2 deletions

File tree

lib/net.js

Lines changed: 6 additions & 2 deletions
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
// Walking the `_parent` chain must stop on a nullish link, not only strict
5+
// `null`. During connection teardown a socket's `_parent` can be left
6+
// `undefined`, which previously caused `_unrefTimer()` and `_destroy()` to read
7+
// a property off `undefined` and throw.
8+
// Refs: https://github.com/nodejs/node/issues/64490
9+
10+
const assert = require('assert');
11+
const net = require('net');
12+
13+
{
14+
const socket = new net.Socket();
15+
socket._parent = undefined;
16+
socket._unrefTimer();
17+
}
18+
19+
{
20+
const socket = new net.Socket();
21+
socket._parent = undefined;
22+
socket.on('error', common.mustNotCall());
23+
socket.destroy();
24+
assert.strictEqual(socket.destroyed, true);
25+
}
Lines changed: 39 additions & 0 deletions

0 commit comments

Comments
 (0)