From 7cb636fdaffebb21a7a817b033986cb187fe472f Mon Sep 17 00:00:00 2001 From: David Murdoch Date: Wed, 28 Nov 2018 10:35:30 -0500 Subject: [PATCH 1/3] Allow binary data in websocket and fix crash caused by malformed data --- lib/webSocketServer.js | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/webSocketServer.js b/lib/webSocketServer.js index d6c9e62c7d..1df6cee9fa 100644 --- a/lib/webSocketServer.js +++ b/lib/webSocketServer.js @@ -43,10 +43,25 @@ ConnectionManager.prototype.manageConnection = function(connection) { }; connection.on("message", function(message) { + let payload; try { - var payload = JSON.parse(message.utf8Data); + if (message.type === "utf8") { + payload = JSON.parse(message.utf8Data); + } else if (message.type === "binary") { + payload = JSON.parse(message.binaryData.toString("utf8").trim()); + } else { + throw new Error("Invalid message type"); + } } catch (e) { - connection.reject(400, "Bad Request"); + var error = { + jsonrpc: "2.0", + error: { + code: -32600, + message: e.message + } + }; + connection.send(JSON.stringify(error)); + return; } self._logPayload(payload); From e395388fcf2f662aa17b83544bf8e5e0ef5ef99f Mon Sep 17 00:00:00 2001 From: David Murdoch Date: Tue, 9 Apr 2019 18:15:43 -0400 Subject: [PATCH 2/3] Add test and fix error response message, as we can't respond with jsonrpc since we don't have the id to respond to --- lib/webSocketServer.js | 12 +++-------- test/requests.js | 47 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/lib/webSocketServer.js b/lib/webSocketServer.js index 9aab79187e..6a6142a114 100644 --- a/lib/webSocketServer.js +++ b/lib/webSocketServer.js @@ -1,4 +1,5 @@ -var WebSocketServer = require("websocket").server; +var websocket = require("websocket"); +var WebSocketServer = websocket.server; module.exports = function(httpServer, provider, logger) { var connectionManager = new ConnectionManager(provider, logger); @@ -53,14 +54,7 @@ ConnectionManager.prototype.manageConnection = function(connection) { throw new Error("Invalid message type"); } } catch (e) { - var error = { - jsonrpc: "2.0", - error: { - code: -32600, - message: e.message - } - }; - connection.send(JSON.stringify(error)); + connection.close(websocket.connection.CLOSE_REASON_UNPROCESSABLE_INPUT, e.message); return; } diff --git a/test/requests.js b/test/requests.js index 4e637f3163..51bfb3d654 100644 --- a/test/requests.js +++ b/test/requests.js @@ -1618,6 +1618,53 @@ describe("WebSockets Server:", function() { web3.setProvider(provider); }); + it("Can also handle binary websocket data", async() => { + // Python web3 only sends binary over websockets and we should + // be able to handle it. + + // web3.eth.getAccounts transmits over utf8, so use that as our baseline. + const accounts = await web3.eth.getAccounts(); + + // Listen for messages: + const pendingMessage = new Promise((resolve, reject) => { + function message(result) { + cleanup(); + resolve(JSON.parse(result.data)); + } + function close(err) { + cleanup(); + reject(err.reason); + } + function cleanup() { + web3.currentProvider.connection.removeEventListener("message", message); + web3.currentProvider.connection.removeEventListener("close", close); + } + web3.currentProvider.connection.addEventListener("message", message); + web3.currentProvider.connection.addEventListener("close", close); + }); + + // generate a binary jsonrpc message: + const jsonRpc = Buffer.from( + JSON.stringify({ + jsonrpc: "2.0", + id: 9999, + method: "eth_accounts", + params: [] + }) + ); + + // send the binary data: + web3.currentProvider.connection.send(jsonRpc); + const result = await pendingMessage; + + // And compare + assert.deepStrictEqual( + result.result, + accounts.map((a) => a.toLocaleLowerCase()), + "Accounts don't match between binary and utf8 websocket requests!" + ); + }).timeout(500); // fail quick if our hacked-together websocket handler fails. + tests(web3); after("Shutdown server", async function() { From 3a8a4c38f605aed14e64db0c11127513dfac9af8 Mon Sep 17 00:00:00 2001 From: David Murdoch Date: Tue, 9 Apr 2019 18:31:33 -0400 Subject: [PATCH 3/3] Move the new test so it makes a litte more sense --- test/requests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/requests.js b/test/requests.js index 51bfb3d654..b29af9b262 100644 --- a/test/requests.js +++ b/test/requests.js @@ -1618,6 +1618,8 @@ describe("WebSockets Server:", function() { web3.setProvider(provider); }); + tests(web3); + it("Can also handle binary websocket data", async() => { // Python web3 only sends binary over websockets and we should // be able to handle it. @@ -1665,8 +1667,6 @@ describe("WebSockets Server:", function() { ); }).timeout(500); // fail quick if our hacked-together websocket handler fails. - tests(web3); - after("Shutdown server", async function() { let provider = web3._provider; web3.setProvider();