http: free the parser before emitting 'upgrade'

Ensure that the parser is freed before emitting the 'connect' or 'upgrade' event. PR-URL: #18209 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jon Moss <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
nodejs · Jan 30, 2018 · 0250e1b · 0250e1b
1 parent 01599e2
commit 0250e1b
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 1 deletion.
diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -452,6 +452,7 @@ function socketOnData(d) {
     socket.removeListener('data', socketOnData);
     socket.removeListener('end', socketOnEnd);
     parser.finish();
+    freeParser(parser, req, socket);
 
     var bodyHead = d.slice(bytesParsed, d.length);
 
@@ -472,7 +473,6 @@ function socketOnData(d) {
       // Got Upgrade header or CONNECT method, but have no handler.
       socket.destroy();
     }
-    freeParser(parser, req, socket);
   } else if (parser.incoming && parser.incoming.complete &&
              // When the status code is 100 (Continue), the server will
              // send a final response after this client sends a request

diff --git a/test/parallel/test-http-parser-freed-before-upgrade.js b/test/parallel/test-http-parser-freed-before-upgrade.js
@@ -0,0 +1,33 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const server = http.createServer();
+
+server.on('upgrade', common.mustCall((request, socket) => {
+  assert.strictEqual(socket.parser, null);
+  socket.write([
+    'HTTP/1.1 101 Switching Protocols',
+    'Connection: Upgrade',
+    'Upgrade: WebSocket',
+    '\r\n'
+  ].join('\r\n'));
+}));
+
+server.listen(common.mustCall(() => {
+  const request = http.get({
+    port: server.address().port,
+    headers: {
+      Connection: 'Upgrade',
+      Upgrade: 'WebSocket'
+    }
+  });
+
+  request.on('upgrade', common.mustCall((response, socket) => {
+    assert.strictEqual(socket.parser, null);
+    socket.destroy();
+    server.close();
+  }));
+}));