Skip to content

Commit

Permalink
http: do not emit upgrade on advertisement
Browse files Browse the repository at this point in the history
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: #4334
PR-URL: #4337
Reviewed-By: Ben Noordhuis <[email protected]>
  • Loading branch information
indutny authored and rvagg committed Feb 9, 2016
1 parent 6d8eba0 commit 411d813
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 1 deletion.
1 change: 1 addition & 0 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 19 additions & 0 deletions lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
54 changes: 54 additions & 0 deletions test/parallel/test-http-upgrade-advertise.js
Original file line number Diff line number Diff line change
@@ -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);
1 change: 1 addition & 0 deletions test/parallel/test-http-upgrade-agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
};
Expand Down
8 changes: 7 additions & 1 deletion test/parallel/test-http-upgrade-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 411d813

Please sign in to comment.