From ed31a0cf15ca804b8e00eb3fad3309fa92578846 Mon Sep 17 00:00:00 2001 From: Nick O'Leary Date: Tue, 8 Jan 2019 16:21:36 +0000 Subject: [PATCH] Update to WS 6.x and fix all it broke Significant update to the ws module to get it completely up to date. The jump from 1.x to 6.x has required a rewrite of our WS handling. Most specifically the means by which you can have multiple ws servers on a single http server has completely changed; we now have to handle the 'upgrade' event on the server ourselves. --- package.json | 2 +- .../@node-red/editor-api/lib/editor/comms.js | 30 +++--- .../@node-red/nodes/core/io/22-websocket.js | 99 +++++++++++-------- .../nodes/locales/en-US/messages.json | 3 +- 4 files changed, 79 insertions(+), 55 deletions(-) diff --git a/package.json b/package.json index f7b5087f5..590140ada 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,7 @@ "sentiment": "2.1.0", "uglify-js": "3.4.9", "when": "3.7.8", - "ws": "1.1.5", + "ws": "6.1.2", "xml2js": "0.4.19" }, "optionalDependencies": { diff --git a/packages/node_modules/@node-red/editor-api/lib/editor/comms.js b/packages/node_modules/@node-red/editor-api/lib/editor/comms.js index 8326ffa3b..95e11dcb0 100644 --- a/packages/node_modules/@node-red/editor-api/lib/editor/comms.js +++ b/packages/node_modules/@node-red/editor-api/lib/editor/comms.js @@ -15,6 +15,7 @@ **/ var ws = require("ws"); +var url = require("url"); var log = require("@node-red/util").log; // TODO: separate module var Tokens; @@ -78,6 +79,7 @@ function CommsConnection(ws) { addActiveConnection(self); } ws.on('close',function() { + console.log(arguments); log.audit({event: "comms.close",user:self.user, session: self.session}); log.trace("comms.close "+self.session); removeActiveConnection(self); @@ -187,27 +189,27 @@ function start() { Users.default().then(function(_anonymousUser) { anonymousUser = _anonymousUser; var webSocketKeepAliveTime = settings.webSocketKeepAliveTime || 15000; - var path = settings.httpAdminRoot || "/"; - path = (path.slice(0,1) != "/" ? "/":"") + path + (path.slice(-1) == "/" ? "":"/") + "comms"; - wsServer = new ws.Server({ - server:server, - path:path, - // Disable the deflate option due to this issue - // https://github.com/websockets/ws/pull/632 - // that is fixed in the 1.x release of the ws module - // that we cannot currently pickup as it drops node 0.10 support - //perMessageDeflate: false - }); - + var commsPath = settings.httpAdminRoot || "/"; + commsPath = (commsPath.slice(0,1) != "/" ? "/":"") + commsPath + (commsPath.slice(-1) == "/" ? "":"/") + "comms"; + wsServer = new ws.Server({ noServer: true }); wsServer.on('connection',function(ws) { var commsConnection = new CommsConnection(ws); }); - - wsServer.on('error', function(err) { log.warn(log._("comms.error-server",{message:err.toString()})); }); + server.on('upgrade', function upgrade(request, socket, head) { + const pathname = url.parse(request.url).pathname; + if (pathname === commsPath) { + wsServer.handleUpgrade(request, socket, head, function done(ws) { + wsServer.emit('connection', ws, request); + }); + } + // Don't destroy the socket as other listeners may want to handle the + // event. + }); + lastSentTime = Date.now(); heartbeatTimer = setInterval(function() { diff --git a/packages/node_modules/@node-red/nodes/core/io/22-websocket.js b/packages/node_modules/@node-red/nodes/core/io/22-websocket.js index 2abef30a5..920d81caa 100644 --- a/packages/node_modules/@node-red/nodes/core/io/22-websocket.js +++ b/packages/node_modules/@node-red/nodes/core/io/22-websocket.js @@ -18,6 +18,23 @@ module.exports = function(RED) { "use strict"; var ws = require("ws"); var inspect = require("util").inspect; + var url = require("url"); + + var serverUpgradeAdded = false; + function handleServerUpgrade(request, socket, head) { + const pathname = url.parse(request.url).pathname; + if (listenerNodes.hasOwnProperty(pathname)) { + listenerNodes[pathname].server.handleUpgrade(request, socket, head, function done(ws) { + listenerNodes[pathname].server.emit('connection', ws, request); + }); + } else { + // Don't destroy the socket as other listeners may want to handle the + // event. + } + } + var listenerNodes = {}; + var activeListenerNodes = 0; + // A node red node that sets up a local websocket server function WebSocketListenerNode(n) { @@ -53,13 +70,22 @@ module.exports = function(RED) { function handleConnection(/*socket*/socket) { var id = (1+Math.random()*4294967295).toString(16); - if (node.isServer) { node._clients[id] = socket; node.emit('opened',Object.keys(node._clients).length); } + if (node.isServer) { + node._clients[id] = socket; + node.emit('opened',Object.keys(node._clients).length); + } socket.on('open',function() { - if (!node.isServer) { node.emit('opened',''); } + if (!node.isServer) { + node.emit('opened',''); + } }); socket.on('close',function() { - if (node.isServer) { delete node._clients[id]; node.emit('closed',Object.keys(node._clients).length); } - else { node.emit('closed'); } + if (node.isServer) { + delete node._clients[id]; + node.emit('closed',Object.keys(node._clients).length); + } else { + node.emit('closed'); + } if (!node.closing && !node.isServer) { clearTimeout(node.tout); node.tout = setTimeout(function() { startconn(); }, 3000); // try to reconnect every 3 secs... bit fast ? @@ -78,34 +104,29 @@ module.exports = function(RED) { } if (node.isServer) { - var path = RED.settings.httpNodeRoot || "/"; - path = path + (path.slice(-1) == "/" ? "":"/") + (node.path.charAt(0) == "/" ? node.path.substring(1) : node.path); - - // Workaround https://github.com/einaros/ws/pull/253 - // Listen for 'newListener' events from RED.server - node._serverListeners = {}; - - var storeListener = function(/*String*/event,/*function*/listener) { - if (event == "error" || event == "upgrade" || event == "listening") { - node._serverListeners[event] = listener; - } + activeListenerNodes++; + if (!serverUpgradeAdded) { + RED.server.on('upgrade', handleServerUpgrade); + serverUpgradeAdded = true } - RED.server.addListener('newListener',storeListener); + var path = RED.settings.httpNodeRoot || "/"; + path = path + (path.slice(-1) == "/" ? "":"/") + (node.path.charAt(0) == "/" ? node.path.substring(1) : node.path); + node.fullPath = path; + if (listenerNodes.hasOwnProperty(path)) { + node.error(RED._("websocket.errors.duplicate-path",{path: node.path})); + return; + } + listenerNodes[node.fullPath] = node; var serverOptions = { - server:RED.server, - path:path + noServer: true } if (RED.settings.webSocketNodeVerifyClient) { serverOptions.verifyClient = RED.settings.webSocketNodeVerifyClient; } // Create a WebSocket Server node.server = new ws.Server(serverOptions); - - // Workaround https://github.com/einaros/ws/pull/253 - // Stop listening for new listener events - RED.server.removeListener('newListener',storeListener); node.server.setMaxListeners(0); node.server.on('connection', handleConnection); } @@ -115,21 +136,17 @@ module.exports = function(RED) { } node.on("close", function() { - // Workaround https://github.com/einaros/ws/pull/253 - // Remove listeners from RED.server if (node.isServer) { - var listener = null; - for (var event in node._serverListeners) { - if (node._serverListeners.hasOwnProperty(event)) { - listener = node._serverListeners[event]; - if (typeof listener === "function") { - RED.server.removeListener(event,listener); - } - } - } - node._serverListeners = {}; + delete listenerNodes[node.fullPath]; node.server.close(); node._inputNodes = []; + activeListenerNodes--; + if (activeListenerNodes === 0 && serverUpgradeAdded) { + RED.server.removeListener('upgrade', handleServerUpgrade); + serverUpgradeAdded = false; + } + + } else { node.closing = true; @@ -177,11 +194,12 @@ module.exports = function(RED) { } WebSocketListenerNode.prototype.broadcast = function(data) { - var i; try { if (this.isServer) { - for (i = 0; i < this.server.clients.length; i++) { - this.server.clients[i].send(data); + for (let client in this._clients) { + if (this._clients.hasOwnProperty(client)) { + this._clients[client].send(data); + } } } else { @@ -215,8 +233,11 @@ module.exports = function(RED) { this.serverConfig.on('opened', function(n) { node.status({fill:"green",shape:"dot",text:RED._("websocket.status.connected",{count:n})}); }); this.serverConfig.on('erro', function() { node.status({fill:"red",shape:"ring",text:"common.status.error"}); }); this.serverConfig.on('closed', function(n) { - if (n > 0) { node.status({fill:"green",shape:"dot",text:RED._("websocket.status.connected",{count:n})}); } - else { node.status({fill:"red",shape:"ring",text:"common.status.disconnected"}); } + if (n > 0) { + node.status({fill:"green",shape:"dot",text:RED._("websocket.status.connected",{count:n})}); + } else { + node.status({fill:"red",shape:"ring",text:"common.status.disconnected"}); + } }); } else { this.error(RED._("websocket.errors.missing-conf")); diff --git a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json index cc9b06d41..59b39160d 100644 --- a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json +++ b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json @@ -432,7 +432,8 @@ "errors": { "connect-error": "An error occured on the ws connection: ", "send-error": "An error occurred while sending: ", - "missing-conf": "Missing server configuration" + "missing-conf": "Missing server configuration", + "duplicate-path": "Cannot have two WebSocket listeners on the same path: __path__" } }, "watch": {