From 28102edbc8a0ec20e5ba846bb0599ca3b4013b35 Mon Sep 17 00:00:00 2001
From: Roee Kasher <roee@fire.glass>
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 <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 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();
+}));