Skip to content

Commit a2843f8

Browse files
RajeshKumar11aduh95
authored andcommitted
net: defer synchronous destroy calls in internalConnect
Defer socket.destroy() calls in internalConnect and internalConnectMultiple to the next tick. This ensures that error handlers have a chance to be set up before errors are emitted, particularly important when using http.request with a custom lookup function that returns synchronously. Previously, if a synchronous lookup function returned an IP that triggered an immediate error (e.g., via blockList), the error would be emitted before the HTTP client had set up its error handler (which happens via process.nextTick in onSocket). This caused unhandled 'error' events. Fixes: #48771 PR-URL: #61658 Refs: #51038 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com>
1 parent bad3f02 commit a2843f8

File tree

2 files changed

+60
-5
lines changed

2 files changed

+60
-5
lines changed

lib/net.js

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,7 +1125,7 @@ function internalConnect(
11251125
err = checkBindError(err, localPort, self._handle);
11261126
if (err) {
11271127
const ex = new ExceptionWithHostPort(err, 'bind', localAddress, localPort);
1128-
self.destroy(ex);
1128+
process.nextTick(emitErrorAndDestroy, self, ex);
11291129
return;
11301130
}
11311131
}
@@ -1135,7 +1135,7 @@ function internalConnect(
11351135

11361136
if (addressType === 6 || addressType === 4) {
11371137
if (self.blockList?.check(address, `ipv${addressType}`)) {
1138-
self.destroy(new ERR_IP_BLOCKED(address));
1138+
process.nextTick(emitErrorAndDestroy, self, new ERR_IP_BLOCKED(address));
11391139
return;
11401140
}
11411141
const req = new TCPConnectWrap();
@@ -1167,12 +1167,20 @@ function internalConnect(
11671167
}
11681168

11691169
const ex = new ExceptionWithHostPort(err, 'connect', address, port, details);
1170-
self.destroy(ex);
1170+
process.nextTick(emitErrorAndDestroy, self, ex);
11711171
} else if ((addressType === 6 || addressType === 4) && hasObserver('net')) {
11721172
startPerf(self, kPerfHooksNetConnectContext, { type: 'net', name: 'connect', detail: { host: address, port } });
11731173
}
11741174
}
11751175

1176+
// Helper function to defer socket destruction to the next tick.
1177+
// This ensures that error handlers have a chance to be set up
1178+
// before the error is emitted, particularly important when using
1179+
// http.request with a custom lookup function.
1180+
function emitErrorAndDestroy(self, err) {
1181+
self.destroy(err);
1182+
}
1183+
11761184

11771185
function internalConnectMultiple(context, canceled) {
11781186
clearTimeout(context[kTimeout]);
@@ -1186,11 +1194,11 @@ function internalConnectMultiple(context, canceled) {
11861194
// All connections have been tried without success, destroy with error
11871195
if (canceled || context.current === context.addresses.length) {
11881196
if (context.errors.length === 0) {
1189-
self.destroy(new ERR_SOCKET_CONNECTION_TIMEOUT());
1197+
process.nextTick(emitErrorAndDestroy, self, new ERR_SOCKET_CONNECTION_TIMEOUT());
11901198
return;
11911199
}
11921200

1193-
self.destroy(new NodeAggregateError(context.errors));
1201+
process.nextTick(emitErrorAndDestroy, self, new NodeAggregateError(context.errors));
11941202
return;
11951203
}
11961204

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
'use strict';
2+
const common = require('../common');
3+
const http = require('http');
4+
const net = require('net');
5+
6+
// This test verifies that errors occurring synchronously during connection
7+
// when using http.request with a custom lookup function and blockList
8+
// can be caught by the error handler.
9+
// Regression test for https://github.com/nodejs/node/issues/48771
10+
11+
// The issue occurs when:
12+
// 1. http.request() is called with a custom synchronous lookup function
13+
// 2. The lookup returns an IP that triggers a synchronous error (e.g., blockList)
14+
// 3. The error is emitted before http's error handler is set up (via nextTick)
15+
//
16+
// The fix defers socket.destroy() calls in internalConnect to the next tick,
17+
// giving http.request() time to set up its error handlers.
18+
19+
const blockList = new net.BlockList();
20+
blockList.addAddress(common.localhostIPv4);
21+
22+
// Synchronous lookup that returns the blocked IP
23+
const lookup = (_hostname, _options, callback) => {
24+
callback(null, common.localhostIPv4, 4);
25+
};
26+
27+
const req = http.request({
28+
host: 'example.com',
29+
port: 80,
30+
lookup,
31+
family: 4, // Force IPv4 to use simple lookup path
32+
createConnection: (opts) => {
33+
// Pass blockList to trigger synchronous ERR_IP_BLOCKED error
34+
return net.createConnection({ ...opts, blockList });
35+
},
36+
}, common.mustNotCall());
37+
38+
// This error handler must be called.
39+
// Without the fix, the error would be emitted before http.request()
40+
// returns, causing an unhandled 'error' event.
41+
req.on('error', common.mustCall((err) => {
42+
if (err.code !== 'ERR_IP_BLOCKED') {
43+
throw new Error(`Expected ERR_IP_BLOCKED but got ${err.code}`);
44+
}
45+
}));
46+
47+
req.end();

0 commit comments

Comments
 (0)