Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 774eb19

Browse filesBrowse 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
Copy full SHA for 774eb19

File tree

Expand file treeCollapse file tree

2 files changed

+67
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+67
-1
lines changed
Open diff view settings
Collapse file

‎lib/_http_server.js‎

Copy file name to clipboardExpand all lines: lib/_http_server.js
+20-1Lines changed: 20 additions & 1 deletion
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.
Collapse file
+47Lines changed: 47 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.