From 6a0571cd726df4cd03e42ab0936c4f0611270e94 Mon Sep 17 00:00:00 2001 From: Fedor Indutny 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 --- 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 108b38891aabb1..04740d5dcb1348 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -465,6 +465,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 5c24180371e4fb..66d4d1fb6942b9 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -77,6 +77,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 @@ -142,6 +156,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 @@ -175,6 +193,7 @@ function freeParser(parser, req, socket) { parser.socket.parser = null; parser.socket = null; parser.incoming = null; + parser.outgoing = null; if (parsers.free(parser) === false) parser.close(); parser = null; 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) {