Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 50a4471

Browse filesBrowse files
mscdexMyles Borins
mscdex
authored and
Myles Borins
committed
http: fix connection upgrade checks
This commit fixes connection upgrade checks, specifically when headers are passed as an array instead of a plain object to http.request() Fixes: #8235 PR-URL: #8238 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 9389572 commit 50a4471
Copy full SHA for 50a4471

File tree

Expand file treeCollapse file tree

3 files changed

+65
-47
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+65
-47
lines changed
Open diff view settings
Collapse file

‎lib/_http_common.js‎

Copy file name to clipboardExpand all lines: lib/_http_common.js
+5-11Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -78,17 +78,11 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,
7878
parser.incoming.statusMessage = statusMessage;
7979
}
8080

81-
// The client made non-upgrade request, and server is just advertising
82-
// supported protocols.
83-
//
84-
// See RFC7230 Section 6.7
85-
//
86-
// NOTE: RegExp below matches `upgrade` in `Connection: abc, upgrade, def`
87-
// header.
88-
if (upgrade &&
89-
parser.outgoing !== null &&
90-
(parser.outgoing._headers.upgrade === undefined ||
91-
!/(^|\W)upgrade(\W|$)/i.test(parser.outgoing._headers.connection))) {
81+
if (upgrade && parser.outgoing !== null && !parser.outgoing.upgrading) {
82+
// The client made non-upgrade request, and server is just advertising
83+
// supported protocols.
84+
//
85+
// See RFC7230 Section 6.7
9286
upgrade = false;
9387
}
9488

Collapse file

‎lib/_http_outgoing.js‎

Copy file name to clipboardExpand all lines: lib/_http_outgoing.js
+18-6Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,18 @@ const Buffer = require('buffer').Buffer;
99
const common = require('_http_common');
1010

1111
const CRLF = common.CRLF;
12-
const chunkExpression = common.chunkExpression;
12+
const trfrEncChunkExpression = common.chunkExpression;
1313
const debug = common.debug;
1414

15-
const connectionExpression = /^Connection$/i;
15+
const upgradeExpression = /^Upgrade$/i;
1616
const transferEncodingExpression = /^Transfer-Encoding$/i;
17-
const closeExpression = /close/i;
1817
const contentLengthExpression = /^Content-Length$/i;
1918
const dateExpression = /^Date$/i;
2019
const expectExpression = /^Expect$/i;
2120
const trailerExpression = /^Trailer$/i;
21+
const connectionExpression = /^Connection$/i;
22+
const connCloseExpression = /(^|\W)close(\W|$)/i;
23+
const connUpgradeExpression = /(^|\W)upgrade(\W|$)/i;
2224
const lenientHttpHeaders = !!process.REVERT_CVE_2016_2216;
2325

2426
const automaticHeaders = {
@@ -62,6 +64,7 @@ function OutgoingMessage() {
6264
this.writable = true;
6365

6466
this._last = false;
67+
this.upgrading = false;
6568
this.chunkedEncoding = false;
6669
this.shouldKeepAlive = true;
6770
this.useChunkedEncodingByDefault = true;
@@ -191,11 +194,13 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
191194
// in the case of response it is: 'HTTP/1.1 200 OK\r\n'
192195
var state = {
193196
sentConnectionHeader: false,
197+
sentConnectionUpgrade: false,
194198
sentContentLengthHeader: false,
195199
sentTransferEncodingHeader: false,
196200
sentDateHeader: false,
197201
sentExpect: false,
198202
sentTrailer: false,
203+
sentUpgrade: false,
199204
messageHeader: firstLine
200205
};
201206

@@ -224,6 +229,10 @@ OutgoingMessage.prototype._storeHeader = function(firstLine, headers) {
224229
}
225230
}
226231

232+
// Are we upgrading the connection?
233+
if (state.sentConnectionUpgrade && state.sentUpgrade)
234+
this.upgrading = true;
235+
227236
// Date header
228237
if (this.sendDate === true && state.sentDateHeader === false) {
229238
state.messageHeader += 'Date: ' + utcDate() + CRLF;
@@ -313,15 +322,16 @@ function storeHeader(self, state, field, value) {
313322

314323
if (connectionExpression.test(field)) {
315324
state.sentConnectionHeader = true;
316-
if (closeExpression.test(value)) {
325+
if (connCloseExpression.test(value)) {
317326
self._last = true;
318327
} else {
319328
self.shouldKeepAlive = true;
320329
}
321-
330+
if (connUpgradeExpression.test(value))
331+
state.sentConnectionUpgrade = true;
322332
} else if (transferEncodingExpression.test(field)) {
323333
state.sentTransferEncodingHeader = true;
324-
if (chunkExpression.test(value)) self.chunkedEncoding = true;
334+
if (trfrEncChunkExpression.test(value)) self.chunkedEncoding = true;
325335

326336
} else if (contentLengthExpression.test(field)) {
327337
state.sentContentLengthHeader = true;
@@ -331,6 +341,8 @@ function storeHeader(self, state, field, value) {
331341
state.sentExpect = true;
332342
} else if (trailerExpression.test(field)) {
333343
state.sentTrailer = true;
344+
} else if (upgradeExpression.test(field)) {
345+
state.sentUpgrade = true;
334346
}
335347
}
336348

Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http-upgrade-client.js
+42-30Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -25,40 +25,52 @@ var srv = net.createServer(function(c) {
2525
});
2626
});
2727

28-
var gotUpgrade = false;
29-
30-
srv.listen(0, '127.0.0.1', function() {
31-
32-
var req = http.get({
33-
port: this.address().port,
34-
headers: {
28+
srv.listen(0, '127.0.0.1', common.mustCall(function() {
29+
var port = this.address().port;
30+
var headers = [
31+
{
3532
connection: 'upgrade',
3633
upgrade: 'websocket'
37-
}
38-
});
39-
req.on('upgrade', function(res, socket, upgradeHead) {
40-
var recvData = upgradeHead;
41-
socket.on('data', function(d) {
42-
recvData += d;
34+
},
35+
[
36+
['Host', 'echo.websocket.org'],
37+
['Connection', 'Upgrade'],
38+
['Upgrade', 'websocket'],
39+
['Origin', 'http://www.websocket.org']
40+
]
41+
];
42+
var left = headers.length;
43+
headers.forEach(function(h) {
44+
var req = http.get({
45+
port: port,
46+
headers: h
4347
});
48+
var sawUpgrade = false;
49+
req.on('upgrade', common.mustCall(function(res, socket, upgradeHead) {
50+
sawUpgrade = true;
51+
var recvData = upgradeHead;
52+
socket.on('data', function(d) {
53+
recvData += d;
54+
});
4455

45-
socket.on('close', common.mustCall(function() {
46-
assert.equal(recvData, 'nurtzo');
47-
}));
48-
49-
console.log(res.headers);
50-
var expectedHeaders = {'hello': 'world',
51-
'connection': 'upgrade',
52-
'upgrade': 'websocket' };
53-
assert.deepEqual(expectedHeaders, res.headers);
56+
socket.on('close', common.mustCall(function() {
57+
assert.equal(recvData, 'nurtzo');
58+
}));
5459

55-
socket.end();
56-
srv.close();
60+
console.log(res.headers);
61+
var expectedHeaders = {
62+
hello: 'world',
63+
connection: 'upgrade',
64+
upgrade: 'websocket'
65+
};
66+
assert.deepStrictEqual(expectedHeaders, res.headers);
5767

58-
gotUpgrade = true;
68+
socket.end();
69+
if (--left == 0)
70+
srv.close();
71+
}));
72+
req.on('close', common.mustCall(function() {
73+
assert.strictEqual(sawUpgrade, true);
74+
}));
5975
});
60-
});
61-
62-
process.on('exit', function() {
63-
assert.ok(gotUpgrade);
64-
});
76+
}));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.