Skip to content

Commit 0d1943a

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 57d9a8a commit 0d1943a

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
@@ -1066,7 +1066,7 @@ function internalConnect(
10661066
err = checkBindError(err, localPort, self._handle);
10671067
if (err) {
10681068
const ex = new ExceptionWithHostPort(err, 'bind', localAddress, localPort);
1069-
self.destroy(ex);
1069+
process.nextTick(emitErrorAndDestroy, self, ex);
10701070
return;
10711071
}
10721072
}
@@ -1076,7 +1076,7 @@ function internalConnect(
10761076

10771077
if (addressType === 6 || addressType === 4) {
10781078
if (self.blockList?.check(address, `ipv${addressType}`)) {
1079-
self.destroy(new ERR_IP_BLOCKED(address));
1079+
process.nextTick(emitErrorAndDestroy, self, new ERR_IP_BLOCKED(address));
10801080
return;
10811081
}
10821082
const req = new TCPConnectWrap();
@@ -1108,12 +1108,20 @@ function internalConnect(
11081108
}
11091109

11101110
const ex = new ExceptionWithHostPort(err, 'connect', address, port, details);
1111-
self.destroy(ex);
1111+
process.nextTick(emitErrorAndDestroy, self, ex);
11121112
} else if ((addressType === 6 || addressType === 4) && hasObserver('net')) {
11131113
startPerf(self, kPerfHooksNetConnectContext, { type: 'net', name: 'connect', detail: { host: address, port } });
11141114
}
11151115
}
11161116

1117+
// Helper function to defer socket destruction to the next tick.
1118+
// This ensures that error handlers have a chance to be set up
1119+
// before the error is emitted, particularly important when using
1120+
// http.request with a custom lookup function.
1121+
function emitErrorAndDestroy(self, err) {
1122+
self.destroy(err);
1123+
}
1124+
11171125

11181126
function internalConnectMultiple(context, canceled) {
11191127
clearTimeout(context[kTimeout]);
@@ -1127,11 +1135,11 @@ function internalConnectMultiple(context, canceled) {
11271135
// All connections have been tried without success, destroy with error
11281136
if (canceled || context.current === context.addresses.length) {
11291137
if (context.errors.length === 0) {
1130-
self.destroy(new ERR_SOCKET_CONNECTION_TIMEOUT());
1138+
process.nextTick(emitErrorAndDestroy, self, new ERR_SOCKET_CONNECTION_TIMEOUT());
11311139
return;
11321140
}
11331141

1134-
self.destroy(new NodeAggregateError(context.errors));
1142+
process.nextTick(emitErrorAndDestroy, self, new NodeAggregateError(context.errors));
11351143
return;
11361144
}
11371145

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)