Skip to content

Commit 01fc2e9

Browse files
committed
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: nodejs#4334
1 parent 0b43c08 commit 01fc2e9

File tree

5 files changed

+85
-1
lines changed

5 files changed

+85
-1
lines changed

lib/_http_client.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,7 @@ function tickOnSocket(req, socket) {
466466
parser.reinitialize(HTTPParser.RESPONSE);
467467
parser.socket = socket;
468468
parser.incoming = null;
469+
parser.outgoing = req;
469470
req.parser = parser;
470471

471472
socket.parser = parser;

lib/_http_common.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,17 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,
7777
parser.incoming.statusMessage = statusMessage;
7878
}
7979

80+
// The client made non-upgrade request, and server is just advertising
81+
// supported protocols.
82+
//
83+
// See RFC7230 Section 6.7
84+
if (upgrade &&
85+
parser.outgoing !== null &&
86+
(parser.outgoing._headers.upgrade === undefined ||
87+
!/(^|\W)upgrade(\W|$)/i.test(parser.outgoing._headers.connection))) {
88+
upgrade = false;
89+
}
90+
8091
parser.incoming.upgrade = upgrade;
8192

8293
var skipBody = false; // response to HEAD or CONNECT
@@ -142,6 +153,10 @@ var parsers = new FreeList('parsers', 1000, function() {
142153
parser._url = '';
143154
parser._consumed = false;
144155

156+
parser.socket = null;
157+
parser.incoming = null;
158+
parser.outgoing = null;
159+
145160
// Only called in the slow case where slow means
146161
// that the request headers were either fragmented
147162
// across multiple TCP packets or too large to be
@@ -175,6 +190,7 @@ function freeParser(parser, req, socket) {
175190
parser.socket.parser = null;
176191
parser.socket = null;
177192
parser.incoming = null;
193+
parser.outgoing = null;
178194
if (parsers.free(parser) === false)
179195
parser.close();
180196
parser = null;
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const http = require('http');
6+
7+
const tests = [
8+
{ headers: {}, expected: 'regular' },
9+
{ headers: { upgrade: 'h2c' }, expected: 'regular' },
10+
{ headers: { connection: 'upgrade' }, expected: 'regular' },
11+
{ headers: { connection: 'upgrade', upgrade: 'h2c' }, expected: 'upgrade' }
12+
];
13+
14+
function fire() {
15+
if (tests.length === 0)
16+
return server.close();
17+
18+
const test = tests.shift();
19+
20+
var once = false;
21+
22+
const done = common.mustCall(function done(result) {
23+
assert(!once);
24+
once = true;
25+
assert.equal(result, test.expected);
26+
27+
fire();
28+
});
29+
30+
const req = http.request({
31+
port: common.PORT,
32+
path: '/',
33+
headers: test.headers
34+
}, function onResponse(res) {
35+
res.resume();
36+
done('regular');
37+
});
38+
39+
req.on('upgrade', function onUpgrade(res, socket) {
40+
socket.destroy();
41+
done('upgrade');
42+
});
43+
44+
req.end();
45+
}
46+
47+
const server = http.createServer(function(req, res) {
48+
res.writeHead(200, {
49+
Connection: 'upgrade, keep-alive',
50+
Upgrade: 'h2c'
51+
});
52+
res.end('hello world');
53+
}).on('upgrade', function(req, socket) {
54+
socket.end('HTTP/1.1 101 Switching protocols\r\n' +
55+
'Connection: upgrade\r\n' +
56+
'Upgrade: h2c\r\n\r\n' +
57+
'ohai');
58+
}).listen(common.PORT, function() {
59+
fire();
60+
});

test/parallel/test-http-upgrade-agent.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ srv.listen(common.PORT, '127.0.0.1', function() {
3636
port: common.PORT,
3737
host: '127.0.0.1',
3838
headers: {
39+
'connection': 'upgrade',
3940
'upgrade': 'websocket'
4041
}
4142
};

test/parallel/test-http-upgrade-client.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,13 @@ var gotUpgrade = false;
3232

3333
srv.listen(common.PORT, '127.0.0.1', function() {
3434

35-
var req = http.get({ port: common.PORT });
35+
var req = http.get({
36+
port: common.PORT,
37+
headers: {
38+
connection: 'upgrade',
39+
upgrade: 'websocket'
40+
}
41+
});
3642
req.on('upgrade', function(res, socket, upgradeHead) {
3743
// XXX: This test isn't fantastic, as it assumes that the entire response
3844
// from the server will arrive in a single data callback

0 commit comments

Comments
 (0)