Skip to content

Commit 6baab7e

Browse files
galpeterhaesik
authored andcommitted
Improve WebSocket server handshake handling (#1921)
The WebSocket server incorrectly used http headers: * The 'Connection' header field value can have multiple elements, not just 'Upgrade'. * The header field names should be handled in a case-insensitive way. * If there is an error reporting a 400 error should also terminate the connection and correctly add http header termination lines. IoT.js-DCO-1.0-Signed-off-by: Peter Gal pgal.u-szeged@partner.samsung.com
1 parent 26cfc25 commit 6baab7e

File tree

2 files changed

+69
-11
lines changed

2 files changed

+69
-11
lines changed

src/js/websocket.js

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -88,33 +88,37 @@ function parseServerHandshakeData(data, client, server) {
8888
var res = data.split('\r\n');
8989
var method = res[0].split(' ');
9090

91-
var headers = { 'Connection': '',
92-
'Upgrade': '',
93-
'Host': '',
94-
'Sec-WebSocket-Key': '',
95-
'Sec-WebSocket-Version': -1,
91+
// All header keys are converted to lower case
92+
// to ease the processing of the values.
93+
// Based on the HTTP/1.1 RFC (https://tools.ietf.org/html/rfc7230#section-3.2)
94+
// this conversion is ok as the header field names are case-insensitive.
95+
var headers = { 'connection': '',
96+
'upgrade': '',
97+
'host': '',
98+
'sec-websocket-key': '',
99+
'sec-websocket-version': -1,
96100
};
97101

98102
for (var i = 1; i < res.length; i++) {
99103
var temp = res[i].split(': ');
100-
headers[temp[0]] = temp[1];
104+
headers[temp[0].toLowerCase()] = temp[1];
101105
}
102106

103107
var response = '';
104108
if (method[0] === 'GET' &&
105109
method[2] === 'HTTP/1.1' &&
106110
method[1] === server.path &&
107-
headers['Connection'] === 'Upgrade' &&
108-
headers['Upgrade'] === 'websocket' &&
109-
headers['Sec-WebSocket-Version'] === '13') {
111+
headers['connection'].toLowerCase().indexOf('upgrade') !== -1 &&
112+
headers['upgrade'].toLowerCase() === 'websocket' &&
113+
headers['sec-websocket-version'] === '13') {
110114
response = native.ReceiveHandshakeData(
111-
headers['Sec-WebSocket-Key']
115+
headers['sec-websocket-key']
112116
).toString();
113117
client.readyState = 'OPEN';
114118
client._socket.write(response);
115119
server.emit('open', client);
116120
} else {
117-
response = method[2] + ' 400 Bad Request';
121+
response = method[2] + ' 400 Bad Request\r\nConnection: Closed\r\n\r\n';
118122
client._socket.write(response);
119123
}
120124
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/* Copyright 2019-present Samsung Electronics Co., Ltd. and other contributors
2+
*
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software
10+
* distributed under the License is distributed on an "AS IS" BASIS
11+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
* See the License for the specific language governing permissions and
13+
* limitations under the License.
14+
*/
15+
16+
var assert = require('assert');
17+
var ws = require('websocket');
18+
var http = require('http');
19+
20+
var websocket = new ws.Websocket();
21+
22+
var test_connected = false;
23+
var test_statuscode = -1;
24+
25+
var server = new ws.Server({ port: 8001 }, function(srv) {
26+
console.log("Connected!");
27+
test_connected = true;
28+
server.close()
29+
});
30+
31+
var client = http.request({
32+
method: 'GET',
33+
port: 8001,
34+
headers: {
35+
// Test if multiple values for the Connection header is accepted
36+
'Connection': 'keep-alive, Upgrade',
37+
'Upgrade': 'websocket',
38+
'Sec-WebSocket-Key': 'r3UXMybFKTPGuT2CK5cYGw==',
39+
'Sec-WebSocket-Version': 13,
40+
}
41+
}, function(response) {
42+
// 101 Switching Protocols
43+
test_statuscode = response.statusCode;
44+
server.close();
45+
});
46+
47+
client.end();
48+
49+
50+
process.on('exit', function () {
51+
assert(test_connected, 'WebScoket server did not received connection event');
52+
assert.equal(test_statusCode, 101,
53+
'GET with multiple Connection value should return 101 status');
54+
});

0 commit comments

Comments
 (0)