diff options
author | Paolo Insogna <paolo@cowtech.it> | 2023-03-08 08:09:21 +0100 |
---|---|---|
committer | Danielle Adams <adamzdanielle@gmail.com> | 2023-04-10 20:55:11 -0400 |
commit | 80b4e6da53e44d10afee9921312595a6a07a174e (patch) | |
tree | a7d60b42db4b6c20c8c92e399cc632bb1b72bedc | |
parent | 3b077a688576a312c665dde6a751f78c5f162c46 (diff) | |
download | node-new-80b4e6da53e44d10afee9921312595a6a07a174e.tar.gz |
http: use listenerCount when adding noop event
PR-URL: https://github.com/nodejs/node/pull/46769
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
-rw-r--r-- | lib/_http_server.js | 21 | ||||
-rw-r--r-- | test/parallel/test-http-socket-error-listeners.js | 47 |
2 files changed, 67 insertions, 1 deletions
diff --git a/lib/_http_server.js b/lib/_http_server.js index c8b43a9301..028ea1557c 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -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. diff --git a/test/parallel/test-http-socket-error-listeners.js b/test/parallel/test-http-socket-error-listeners.js new file mode 100644 index 0000000000..f0d220a5d9 --- /dev/null +++ b/test/parallel/test-http-socket-error-listeners.js @@ -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(); + }); + }); +} |