From 52eb41c24f27960024a1f441993eb40984be9f3d Mon Sep 17 00:00:00 2001 From: David Glasser Date: Thu, 15 Dec 2016 14:36:49 -0800 Subject: [PATCH] In Node 6, send a 400 on HTTP parse failure Previously we used the Node default behavior of just closing the socket. This made it difficult for load balancers to understand if the backend is buggy or the client request was bad (if Node is validating something more strictly than the load balancer). This only takes effect on Node 6; in older versions the socket is already destroyed before the event is invoked. See https://github.com/nodejs/node/pull/4557/ for details. --- packages/webapp/webapp_server.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/webapp/webapp_server.js b/packages/webapp/webapp_server.js index eed2969d90..7cbdd2fc6a 100644 --- a/packages/webapp/webapp_server.js +++ b/packages/webapp/webapp_server.js @@ -761,6 +761,27 @@ var runWebAppServer = function () { // own. httpServer.on('request', WebApp._timeoutAdjustmentRequestCallback); + // If the client gave us a bad request, tell it instead of just closing the + // socket. This lets load balancers in front of us differentiate between "a + // server is randomly closing sockets for no reason" and "client sent a bad + // request". + // + // This will only work on Node 6; Node 4 destroys the socket before calling + // this event. See https://github.com/nodejs/node/pull/4557/ for details. + httpServer.on('clientError', (err, socket) => { + // Pre-Node-6, do nothing. + if (socket.destroyed) { + return; + } + + if (err.message === 'Parse Error') { + socket.end('HTTP/1.1 400 Bad Request\r\n\r\n'); + } else { + // For other errors, use the default behavior as if we had no clientError + // handler. + socket.destroy(err); + } + }); // start up app _.extend(WebApp, {