From 5293ea876da9b17bc7a2e77b232e87d1ad6df667 Mon Sep 17 00:00:00 2001 From: Roee Kasher Date: Thu, 26 Jan 2017 13:58:51 +0200 Subject: [PATCH] src: unconsume stream fix in internal http impl When emitting a 'connection' event on a httpServer, the function connectionListener is called. Then, a new parser is created, and 'consume' method is called on the socket's externalStream. However, if this stream was already consumed and unconsumed, the process crashes with a cpp assert from the 'Consume' method in stream_base.h. This commit makes sure that no SIGABRT will be raised and the process will stay alive (after emitting the socket). PR-URL: https://github.com/nodejs/node/pull/11015 Reviewed-By: Fedor Indutny Reviewed-By: James M Snell --- lib/_http_server.js | 4 +++- .../test-http-server-unconsume-consume.js | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http-server-unconsume-consume.js diff --git a/lib/_http_server.js b/lib/_http_server.js index fc88e8938c7f40..d036a410f6f40f 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -345,9 +345,11 @@ function connectionListener(socket) { // Override on to unconsume on `data`, `readable` listeners socket.on = socketOnWrap; + // We only consume the socket if it has never been consumed before. var external = socket._handle._externalStream; - if (external) { + if (!socket._handle._consumed && external) { parser._consumed = true; + socket._handle._consumed = true; parser.consume(external); } external = null; diff --git a/test/parallel/test-http-server-unconsume-consume.js b/test/parallel/test-http-server-unconsume-consume.js new file mode 100644 index 00000000000000..d341093303c76f --- /dev/null +++ b/test/parallel/test-http-server-unconsume-consume.js @@ -0,0 +1,22 @@ +'use strict'; +const common = require('../common'); +const http = require('http'); + +const testServer = http.createServer((req, res) => { + common.fail('Should not be called'); + res.end(); +}); +testServer.on('connect', common.mustCall((req, socket, head) => { + socket.write('HTTP/1.1 200 Connection Established' + '\r\n' + + 'Proxy-agent: Node-Proxy' + '\r\n' + + '\r\n'); + // This shouldn't raise an assertion in StreamBase::Consume. + testServer.emit('connection', socket); + testServer.close(); +})); +testServer.listen(0, common.mustCall(() => { + http.request({ + port: testServer.address().port, + method: 'CONNECT' + }, (res) => {}).end(); +}));