Skip to content

Commit 80b4e6d

Browse files
ShogunPandadanielleadams
authored andcommittedApr 11, 2023
http: use listenerCount when adding noop event
PR-URL: #46769 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 3b077a6 commit 80b4e6d

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed
 

‎lib/_http_server.js

+20-1
Original file line numberDiff line numberDiff line change
@@ -817,10 +817,29 @@ const requestHeaderFieldsTooLargeResponse = Buffer.from(
817817
`HTTP/1.1 431 ${STATUS_CODES[431]}\r\n` +
818818
'Connection: close\r\n\r\n', 'ascii',
819819
);
820+
821+
function warnUnclosedSocket() {
822+
if (warnUnclosedSocket.emitted) {
823+
return;
824+
}
825+
826+
warnUnclosedSocket.emitted = true;
827+
process.emitWarning(
828+
'An error event has already been emitted on the socket. ' +
829+
'Please use the destroy method on the socket while handling ' +
830+
"a 'clientError' event.",
831+
);
832+
}
833+
820834
function socketOnError(e) {
821835
// Ignore further errors
822836
this.removeListener('error', socketOnError);
823-
this.on('error', noop);
837+
838+
if (this.listenerCount('error', noop) === 0) {
839+
this.on('error', noop);
840+
} else {
841+
warnUnclosedSocket();
842+
}
824843

825844
if (!this.server.emit('clientError', e, this)) {
826845
// Caution must be taken to avoid corrupting the remote peer.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Flags: --no-warnings
2+
3+
'use strict';
4+
5+
const common = require('../common');
6+
const assert = require('assert');
7+
const http = require('http');
8+
const net = require('net');
9+
10+
// This test sends an invalid character to a HTTP server and purposely
11+
// does not handle clientError (even if it sets an event handler).
12+
//
13+
// The idea is to let the server emit multiple errors on the socket,
14+
// mostly due to parsing error, and make sure they don't result
15+
// in leaking event listeners.
16+
17+
{
18+
let i = 0;
19+
let socket;
20+
process.on('warning', common.mustCall());
21+
22+
const server = http.createServer(common.mustNotCall());
23+
24+
server.on('clientError', common.mustCallAtLeast((err) => {
25+
assert.strictEqual(err.code, 'HPE_INVALID_METHOD');
26+
assert.strictEqual(err.rawPacket.toString(), '*');
27+
28+
if (i === 20) {
29+
socket.end();
30+
} else {
31+
socket.write('*');
32+
i++;
33+
}
34+
}, 1));
35+
36+
server.listen(0, () => {
37+
socket = net.createConnection({ port: server.address().port });
38+
39+
socket.on('connect', () => {
40+
socket.write('*');
41+
});
42+
43+
socket.on('close', () => {
44+
server.close();
45+
});
46+
});
47+
}

0 commit comments

Comments
 (0)
Please sign in to comment.