Skip to content

Commit

Permalink
http: use listenerCount when adding noop event
Browse files Browse the repository at this point in the history
PR-URL: #46769
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
ShogunPanda authored and danielleadams committed Apr 11, 2023
1 parent 3b077a6 commit 80b4e6d
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 1 deletion.
21 changes: 20 additions & 1 deletion lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -817,10 +817,29 @@ const requestHeaderFieldsTooLargeResponse = Buffer.from(
`HTTP/1.1 431 ${STATUS_CODES[431]}\r\n` +
'Connection: close\r\n\r\n', 'ascii',
);

function warnUnclosedSocket() {
if (warnUnclosedSocket.emitted) {
return;
}

warnUnclosedSocket.emitted = true;
process.emitWarning(
'An error event has already been emitted on the socket. ' +
'Please use the destroy method on the socket while handling ' +
"a 'clientError' event.",
);
}

function socketOnError(e) {
// Ignore further errors
this.removeListener('error', socketOnError);
this.on('error', noop);

if (this.listenerCount('error', noop) === 0) {
this.on('error', noop);
} else {
warnUnclosedSocket();
}

if (!this.server.emit('clientError', e, this)) {
// Caution must be taken to avoid corrupting the remote peer.
Expand Down
47 changes: 47 additions & 0 deletions test/parallel/test-http-socket-error-listeners.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Flags: --no-warnings

'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');
const net = require('net');

// This test sends an invalid character to a HTTP server and purposely
// does not handle clientError (even if it sets an event handler).
//
// The idea is to let the server emit multiple errors on the socket,
// mostly due to parsing error, and make sure they don't result
// in leaking event listeners.

{
let i = 0;
let socket;
process.on('warning', common.mustCall());

const server = http.createServer(common.mustNotCall());

server.on('clientError', common.mustCallAtLeast((err) => {
assert.strictEqual(err.code, 'HPE_INVALID_METHOD');
assert.strictEqual(err.rawPacket.toString(), '*');

if (i === 20) {
socket.end();
} else {
socket.write('*');
i++;
}
}, 1));

server.listen(0, () => {
socket = net.createConnection({ port: server.address().port });

socket.on('connect', () => {
socket.write('*');
});

socket.on('close', () => {
server.close();
});
});
}

0 comments on commit 80b4e6d

Please sign in to comment.