GeezerVison is showing github.com — http: use listenerCount when adding noop event · nodejs/node@80b4e6d
File tree
Expand file treeCollapse file tree
Open diff view settings
Expand file treeCollapse file tree
Open diff view settings
Collapse file
Copy file name to clipboardExpand all lines: lib/_http_server.js +20-1Lines changed: 20 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -817,10 +817,29 @@ const requestHeaderFieldsTooLargeResponse = Buffer.from( |
817 | 817 | `HTTP/1.1 431 ${STATUS_CODES[431]}\r\n` + |
818 | 818 | 'Connection: close\r\n\r\n', 'ascii', |
819 | 819 | ); |
| 820 | + |
| 821 | +function warnUnclosedSocket() { |
| 822 | + if (warnUnclosedSocket.emitted) { |
| 823 | + return; |
| 824 | + } |
| 825 | + |
| 826 | + warnUnclosedSocket.emitted = true; |
| 827 | + process.emitWarning( |
| 828 | + 'An error event has already been emitted on the socket. ' + |
| 829 | + 'Please use the destroy method on the socket while handling ' + |
| 830 | + "a 'clientError' event.", |
| 831 | + ); |
| 832 | +} |
| 833 | + |
820 | 834 | function socketOnError(e) { |
821 | 835 | // Ignore further errors |
822 | 836 | this.removeListener('error', socketOnError); |
823 | | - this.on('error', noop); |
| 837 | + |
| 838 | + if (this.listenerCount('error', noop) === 0) { |
| 839 | + this.on('error', noop); |
| 840 | + } else { |
| 841 | + warnUnclosedSocket(); |
| 842 | + } |
824 | 843 |
|
825 | 844 | if (!this.server.emit('clientError', e, this)) { |
826 | 845 | // Caution must be taken to avoid corrupting the remote peer. |
|
Collapse file
Copy file name to clipboard +47Lines changed: 47 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
|
| 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 | +} |