From 411d8133233cc7e4a246f58a92b068bedecfd26c Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor@indutny.com>
Date: Thu, 17 Dec 2015 17:23:46 -0500
Subject: [PATCH] http: do not emit `upgrade` on advertisement

Do not emit `upgrade` if the server is just advertising its protocols
support as per RFC 7230 Section 6.7.

    A server MAY send an Upgrade header field in any other response
    to advertise that it implements support for upgrading to the
    listed protocols, in order of descending preference, when
    appropriate for a future request.

Fix: https://github.com/nodejs/node/issues/4334
PR-URL: https://github.com/nodejs/node/pull/4337
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
---
 lib/_http_client.js                          |  1 +
 lib/_http_common.js                          | 19 +++++++
 test/parallel/test-http-upgrade-advertise.js | 54 ++++++++++++++++++++
 test/parallel/test-http-upgrade-agent.js     |  1 +
 test/parallel/test-http-upgrade-client.js    |  8 ++-
 5 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-http-upgrade-advertise.js

diff --git a/lib/_http_client.js b/lib/_http_client.js
index 51bc5862c6703d..2253b47c91288f 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -471,6 +471,7 @@ function tickOnSocket(req, socket) {
   parser.reinitialize(HTTPParser.RESPONSE);
   parser.socket = socket;
   parser.incoming = null;
+  parser.outgoing = req;
   req.parser = parser;
 
   socket.parser = parser;
diff --git a/lib/_http_common.js b/lib/_http_common.js
index 1d190cef6b4c2b..328b6eea8affba 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -78,6 +78,20 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,
     parser.incoming.statusMessage = statusMessage;
   }
 
+  // The client made non-upgrade request, and server is just advertising
+  // supported protocols.
+  //
+  // See RFC7230 Section 6.7
+  //
+  // NOTE: RegExp below matches `upgrade` in `Connection: abc, upgrade, def`
+  // header.
+  if (upgrade &&
+      parser.outgoing !== null &&
+      (parser.outgoing._headers.upgrade === undefined ||
+       !/(^|\W)upgrade(\W|$)/i.test(parser.outgoing._headers.connection))) {
+    upgrade = false;
+  }
+
   parser.incoming.upgrade = upgrade;
 
   var skipBody = false; // response to HEAD or CONNECT
@@ -143,6 +157,10 @@ var parsers = new FreeList('parsers', 1000, function() {
   parser._url = '';
   parser._consumed = false;
 
+  parser.socket = null;
+  parser.incoming = null;
+  parser.outgoing = null;
+
   // Only called in the slow case where slow means
   // that the request headers were either fragmented
   // across multiple TCP packets or too large to be
@@ -177,6 +195,7 @@ function freeParser(parser, req, socket) {
       parser.socket.parser = null;
     parser.socket = null;
     parser.incoming = null;
+    parser.outgoing = null;
     parser[kOnExecute] = null;
     if (parsers.free(parser) === false)
       parser.close();
diff --git a/test/parallel/test-http-upgrade-advertise.js b/test/parallel/test-http-upgrade-advertise.js
new file mode 100644
index 00000000000000..fbc183f84b2ada
--- /dev/null
+++ b/test/parallel/test-http-upgrade-advertise.js
@@ -0,0 +1,54 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const tests = [
+  { headers: {}, expected: 'regular' },
+  { headers: { upgrade: 'h2c' }, expected: 'regular' },
+  { headers: { connection: 'upgrade' }, expected: 'regular' },
+  { headers: { connection: 'upgrade', upgrade: 'h2c' }, expected: 'upgrade' }
+];
+
+function fire() {
+  if (tests.length === 0)
+    return server.close();
+
+  const test = tests.shift();
+
+  const done = common.mustCall(function done(result) {
+    assert.equal(result, test.expected);
+
+    fire();
+  });
+
+  const req = http.request({
+    port: common.PORT,
+    path: '/',
+    headers: test.headers
+  }, function onResponse(res) {
+    res.resume();
+    done('regular');
+  });
+
+  req.on('upgrade', function onUpgrade(res, socket) {
+    socket.destroy();
+    done('upgrade');
+  });
+
+  req.end();
+}
+
+const server = http.createServer(function(req, res) {
+  res.writeHead(200, {
+    Connection: 'upgrade, keep-alive',
+    Upgrade: 'h2c'
+  });
+  res.end('hello world');
+}).on('upgrade', function(req, socket) {
+  socket.end('HTTP/1.1 101 Switching protocols\r\n' +
+             'Connection: upgrade\r\n' +
+             'Upgrade: h2c\r\n\r\n' +
+             'ohai');
+}).listen(common.PORT, fire);
diff --git a/test/parallel/test-http-upgrade-agent.js b/test/parallel/test-http-upgrade-agent.js
index 83f783a0e34925..7590354f50b393 100644
--- a/test/parallel/test-http-upgrade-agent.js
+++ b/test/parallel/test-http-upgrade-agent.js
@@ -36,6 +36,7 @@ srv.listen(common.PORT, '127.0.0.1', function() {
     port: common.PORT,
     host: '127.0.0.1',
     headers: {
+      'connection': 'upgrade',
       'upgrade': 'websocket'
     }
   };
diff --git a/test/parallel/test-http-upgrade-client.js b/test/parallel/test-http-upgrade-client.js
index a86b0e317f706b..a45c06b44e04f0 100644
--- a/test/parallel/test-http-upgrade-client.js
+++ b/test/parallel/test-http-upgrade-client.js
@@ -32,7 +32,13 @@ var gotUpgrade = false;
 
 srv.listen(common.PORT, '127.0.0.1', function() {
 
-  var req = http.get({ port: common.PORT });
+  var req = http.get({
+    port: common.PORT,
+    headers: {
+      connection: 'upgrade',
+      upgrade: 'websocket'
+    }
+  });
   req.on('upgrade', function(res, socket, upgradeHead) {
     var recvData = upgradeHead;
     socket.on('data', function(d) {