From 9fc35f4e9351efdad55db53f571cc98b7ee8c92a Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 6 Jan 2016 17:00:27 -0500 Subject: [PATCH] http: overridable `clientError` Make default `clientError` behavior (close socket immediately) overridable. With this APIs it is possible to write a custom error handler, and to send, for example, a 400 HTTP response. http.createServer(...).on('clientError', function(err, socket) { socket.end('HTTP/1.1 400 Bad Request\r\n\r\n'); socket.destroy(); }); Fix: #4543 --- doc/api/http.markdown | 4 ++++ lib/_http_server.js | 7 ++----- lib/https.js | 5 +++-- test/parallel/test-http-client-abort.js | 6 ------ test/parallel/test-https-timeout-server.js | 1 + 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/doc/api/http.markdown b/doc/api/http.markdown index aedc35208f6db1..e1b76b6f0317d9 100644 --- a/doc/api/http.markdown +++ b/doc/api/http.markdown @@ -421,6 +421,10 @@ 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 destroying socket, and, for example, +can do it it gracefully by sending 400 HTTP response. + +Default behavior is destroy socket immediately. `socket` is the [`net.Socket`][] object that the error originated from. diff --git a/lib/_http_server.js b/lib/_http_server.js index f524790fb2b13a..7d73d6ea80a3e7 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,8 @@ function connectionListener(socket) { // TODO(isaacs): Move all these functions out of here function socketOnError(e) { - self.emit('clientError', e, this); + if (!self.emit('clientError', e, this)) + this.destroy(e); } function socketOnData(d) { diff --git a/lib/https.js b/lib/https.js index 8e79d295bf5ea6..ca7bb499829849 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(); }); 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-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() {