Skip to content

Commit 774eb19

Browse files
ShogunPandatargos
authored andcommitted
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 9e65996 commit 774eb19

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
@@ -813,10 +813,29 @@ const requestHeaderFieldsTooLargeResponse = Buffer.from(
813813
`HTTP/1.1 431 ${STATUS_CODES[431]}\r\n` +
814814
'Connection: close\r\n\r\n', 'ascii',
815815
);
816+
817+
function warnUnclosedSocket() {
818+
if (warnUnclosedSocket.emitted) {
819+
return;
820+
}
821+
822+
warnUnclosedSocket.emitted = true;
823+
process.emitWarning(
824+
'An error event has already been emitted on the socket. ' +
825+
'Please use the destroy method on the socket while handling ' +
826+
"a 'clientError' event.",
827+
);
828+
}
829+
816830
function socketOnError(e) {
817831
// Ignore further errors
818832
this.removeListener('error', socketOnError);
819-
this.on('error', noop);
833+
834+
if (this.listenerCount('error', noop) === 0) {
835+
this.on('error', noop);
836+
} else {
837+
warnUnclosedSocket();
838+
}
820839

821840
if (!this.server.emit('clientError', e, this)) {
822841
// 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)