OSSForks/node
1
0
Fork 0
mirror of https://github.com/zebrajr/node.git synced 2025-12-06 12:20:27 +01:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

Revert "http: do not leak error listeners"

Browse Source 
This reverts commit 736a7d8d60.

The patch attempted to fix an issue, signaled by a warning, caused by
an invalid API usage. However it introduced a behavior change that is
breaking userland modules.

It is a user error, so restore the original behavior and have the user
investigate and fix the issue in their code.

Refs: https://github.com/nodejs/node/pull/43587#issuecomment-1272092166
Refs: https://github.com/nodejs/node/pull/43587#issuecomment-1272094883
PR-URL: https://github.com/nodejs/node/pull/44921
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
This commit is contained in:
Luigi Pinca 2022-10-12 14:16:56 +02:00 committed by GitHub
parent 3fdf6cfad9
commit 7c06eab1dc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 1 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
lib/_http_server.js
View File

@ -809,10 +809,7 @@ const requestHeaderFieldsTooLargeResponse = Buffer.from(
function socketOnError(e) {
// Ignore further errors
this.removeListener('error', socketOnError);
if (this.listenerCount('error') === 0) {
this.on('error', noop);
}
this.on('error', noop);
if (!this.server.emit('clientError', e, this)) {
// Caution must be taken to avoid corrupting the remote peer.

44
test/parallel/test-http-socket-listeners.js
View File

@ -1,44 +0,0 @@
'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.mustNotCall());
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();
});
});