diff --git a/doc/api/http.markdown b/doc/api/http.markdown index aedc35208f6db1..34e3417b518d63 100644 --- a/doc/api/http.markdown +++ b/doc/api/http.markdown @@ -421,6 +421,11 @@ not be emitted. `function (exception, socket) { }` If a client connection emits an `'error'` event, it will be forwarded here. +Listener of this event is responsible for closing/destroying the underlying +socket. For example, one may wish to more gracefully close the socket with an +HTTP '400 Bad Request' response instead of abruptly severing the connection. + +Default behavior is to destroy the socket immediately on malformed request. `socket` is the [`net.Socket`][] object that the error originated from. diff --git a/doc/api/tls.markdown b/doc/api/tls.markdown index d3e5cd132733e7..d128d08099017e 100644 --- a/doc/api/tls.markdown +++ b/doc/api/tls.markdown @@ -169,7 +169,7 @@ This class is a subclass of `net.Server` and has the same methods on it. Instead of accepting just raw TCP connections, this accepts encrypted connections using TLS or SSL. -### Event: 'clientError' +### Event: 'tlsClientError' `function (exception, tlsSocket) { }` diff --git a/lib/_http_server.js b/lib/_http_server.js index f524790fb2b13a..7534ceba95e9e1 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -237,10 +237,6 @@ function Server(requestListener) { this.addListener('connection', connectionListener); - this.addListener('clientError', function(err, conn) { - conn.destroy(err); - }); - this.timeout = 2 * 60 * 1000; this._pendingResponseData = 0; @@ -353,7 +349,12 @@ function connectionListener(socket) { // TODO(isaacs): Move all these functions out of here function socketOnError(e) { - self.emit('clientError', e, this); + // Ignore further errors + this.removeListener('error', socketOnError); + this.on('error', () => {}); + + if (!self.emit('clientError', e, this)) + this.destroy(e); } function socketOnData(d) { @@ -372,7 +373,7 @@ function connectionListener(socket) { function onParserExecuteCommon(ret, d) { if (ret instanceof Error) { debug('parse error'); - socket.destroy(ret); + socketOnError.call(socket, ret); } else if (parser.incoming && parser.incoming.upgrade) { // Upgrade or CONNECT var bytesParsed = ret; @@ -418,7 +419,7 @@ function connectionListener(socket) { if (ret instanceof Error) { debug('parse error'); - socket.destroy(ret); + socketOnError.call(socket, ret); return; } diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 68bacd8c049ce3..d3df48a4c59fc7 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -809,14 +809,14 @@ function Server(/* [options], listener */) { errorEmitted = true; var connReset = new Error('socket hang up'); connReset.code = 'ECONNRESET'; - self.emit('clientError', connReset, socket); + self.emit('tlsClientError', connReset, socket); } }); socket.on('_tlsError', function(err) { if (!socket._controlReleased && !errorEmitted) { errorEmitted = true; - self.emit('clientError', err, socket); + self.emit('tlsClientError', err, socket); } }); }); diff --git a/lib/https.js b/lib/https.js index 8e79d295bf5ea6..7008a79131c663 100644 --- a/lib/https.js +++ b/lib/https.js @@ -29,8 +29,9 @@ function Server(opts, requestListener) { this.addListener('request', requestListener); } - this.addListener('clientError', function(err, conn) { - conn.destroy(); + this.addListener('tlsClientError', function(err, conn) { + if (!this.emit('clientError', err, conn)) + conn.destroy(err); }); this.timeout = 2 * 60 * 1000; diff --git a/test/parallel/test-http-client-abort.js b/test/parallel/test-http-client-abort.js index c3353bb72201b6..28998c70500be7 100644 --- a/test/parallel/test-http-client-abort.js +++ b/test/parallel/test-http-client-abort.js @@ -21,12 +21,6 @@ var server = http.Server(function(req, res) { server.close(); } }); - - // since there is already clientError, maybe that would be appropriate, - // since "error" is magical - req.on('clientError', function() { - console.log('Got clientError'); - }); }); var responses = 0; diff --git a/test/parallel/test-http-server-client-error.js b/test/parallel/test-http-server-client-error.js new file mode 100644 index 00000000000000..619a4c45175995 --- /dev/null +++ b/test/parallel/test-http-server-client-error.js @@ -0,0 +1,39 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +const http = require('http'); +const net = require('net'); + +const server = http.createServer(common.mustCall(function(req, res) { + res.end(); +})); + +server.on('clientError', common.mustCall(function(err, socket) { + socket.end('HTTP/1.1 400 Bad Request\r\n\r\n'); + + server.close(); +})); + +server.listen(common.PORT, function() { + function next() { + // Invalid request + const client = net.connect(common.PORT); + client.end('Oopsie-doopsie\r\n'); + + var chunks = ''; + client.on('data', function(chunk) { + chunks += chunk; + }); + client.once('end', function() { + assert.equal(chunks, 'HTTP/1.1 400 Bad Request\r\n\r\n'); + }); + } + + // Normal request + http.get({ port: common.PORT, path: '/' }, function(res) { + assert.equal(res.statusCode, 200); + res.resume(); + res.once('end', next); + }); +}); diff --git a/test/parallel/test-https-timeout-server.js b/test/parallel/test-https-timeout-server.js index f6d5d75a88abbe..0cfc9a6eec33c1 100644 --- a/test/parallel/test-https-timeout-server.js +++ b/test/parallel/test-https-timeout-server.js @@ -32,6 +32,7 @@ server.on('clientError', function(err, conn) { assert.equal(conn._secureEstablished, false); server.close(); clientErrors++; + conn.destroy(); }); server.listen(common.PORT, function() { diff --git a/test/parallel/test-tls-econnreset.js b/test/parallel/test-tls-econnreset.js index 6ef629a159a948..78cfbaddb3a530 100644 --- a/test/parallel/test-tls-econnreset.js +++ b/test/parallel/test-tls-econnreset.js @@ -48,7 +48,7 @@ var connectError = null; var server = tls.createServer({ ca: ca, cert: cert, key: key }, function(conn) { throw 'unreachable'; -}).on('clientError', function(err, conn) { +}).on('tlsClientError', function(err, conn) { assert(!clientError && conn); clientError = err; }).listen(common.PORT, function() { diff --git a/test/parallel/test-tls-no-sslv3.js b/test/parallel/test-tls-no-sslv3.js index 442fc3b91c25af..cc307625745844 100644 --- a/test/parallel/test-tls-no-sslv3.js +++ b/test/parallel/test-tls-no-sslv3.js @@ -48,7 +48,7 @@ server.listen(common.PORT, '127.0.0.1', function() { })); }); -server.on('clientError', err => errors.push(err)); +server.on('tlsClientError', err => errors.push(err)); process.on('exit', function() { if (/unknown option -ssl3/.test(stderr)) { diff --git a/test/parallel/test-tls-sni-option.js b/test/parallel/test-tls-sni-option.js index 84b2f506156818..cbd46c46fef291 100644 --- a/test/parallel/test-tls-sni-option.js +++ b/test/parallel/test-tls-sni-option.js @@ -110,7 +110,7 @@ var server = tls.createServer(serverOptions, function(c) { serverResults.push({ sni: c.servername, authorized: c.authorized }); }); -server.on('clientError', function(err) { +server.on('tlsClientError', function(err) { serverResults.push(null); serverError = err.message; }); diff --git a/test/parallel/test-tls-timeout-server.js b/test/parallel/test-tls-timeout-server.js index e3ed246386647f..036d480b5ea039 100644 --- a/test/parallel/test-tls-timeout-server.js +++ b/test/parallel/test-tls-timeout-server.js @@ -25,7 +25,7 @@ var options = { var server = tls.createServer(options, common.fail); -server.on('clientError', function(err, conn) { +server.on('tlsClientError', function(err, conn) { conn.destroy(); server.close(); clientErrors++;