From b4aff8fec55195d035188210f85ae540b600a777 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Tue, 8 Nov 2022 14:39:29 +0100 Subject: [PATCH] remove `news` support (#155) `news` has several resource leaks and other security issues - replaced by `nim-websock` which is actively maintained. --- json_rpc.nimble | 6 +- json_rpc/clients/config.nim | 7 -- json_rpc/clients/websocketclient.nim | 134 +++++++++------------------ tests/all.nim | 15 ++- tests/ethprocs.nim | 1 - tests/helpers.nim | 2 +- tests/testethcalls.nim | 2 +- tests/testhook.nim | 4 +- tests/testhttp.nim | 2 +- tests/testhttps.nim | 2 +- tests/testproxy.nim | 8 +- tests/testrpcmacro.nim | 2 +- tests/testserverclient.nim | 11 +-- 13 files changed, 64 insertions(+), 132 deletions(-) delete mode 100644 json_rpc/clients/config.nim diff --git a/json_rpc.nimble b/json_rpc.nimble index 432d4c6..73d36d0 100644 --- a/json_rpc.nimble +++ b/json_rpc.nimble @@ -13,7 +13,6 @@ requires "nim >= 1.2.0", "chronos", "httputils", "chronicles", - "https://github.com/status-im/news#status", "websock", "json_serialization" @@ -30,7 +29,4 @@ proc buildBinary(name: string, srcDir = "./", params = "", cmdParams = "") = task test, "run tests": buildBinary "all", "tests/", - params = "-d:json_rpc_websocket_package=websock" - - buildBinary "all", "tests/", - params = "-d:json_rpc_websocket_package=news" + params = "" diff --git a/json_rpc/clients/config.nim b/json_rpc/clients/config.nim deleted file mode 100644 index 6a51a4a..0000000 --- a/json_rpc/clients/config.nim +++ /dev/null @@ -1,7 +0,0 @@ -const - json_rpc_websocket_package {.strdefine.} = "news" - useNews* = json_rpc_websocket_package == "news" - -when json_rpc_websocket_package notin ["websock", "news"]: - {.fatal: "json_rpc_websocket_package should be set to either 'websock' or 'news'".} - diff --git a/json_rpc/clients/websocketclient.nim b/json_rpc/clients/websocketclient.nim index 1b41cd6..f2365df 100644 --- a/json_rpc/clients/websocketclient.nim +++ b/json_rpc/clients/websocketclient.nim @@ -1,37 +1,24 @@ import pkg/[chronos, chronos/apps/http/httptable, chronicles], stew/byteutils, - ../client, ./config + ../client export client -# TODO needs fixes in news -# {.push raises: [Defect].} +{.push raises: [Defect].} logScope: topics = "JSONRPC-WS-CLIENT" -when useNews: - const newsUseChronos = true - include pkg/news +import std/[uri, strutils] +import pkg/websock/[websock, extensions/compression/deflate] - type - RpcWebSocketClient* = ref object of RpcClient - transport*: WebSocket - uri*: string - loop*: Future[void] - getHeaders*: GetJsonRpcRequestHeaders - -else: - import std/[uri, strutils] - import pkg/websock/[websock, extensions/compression/deflate] - - type - RpcWebSocketClient* = ref object of RpcClient - transport*: WSSession - uri*: Uri - loop*: Future[void] - getHeaders*: GetJsonRpcRequestHeaders +type + RpcWebSocketClient* = ref object of RpcClient + transport*: WSSession + uri*: Uri + loop*: Future[void] + getHeaders*: GetJsonRpcRequestHeaders proc new*( T: type RpcWebSocketClient, getHeaders: GetJsonRpcRequestHeaders = nil): T = @@ -62,34 +49,20 @@ method call*(self: RpcWebSocketClient, name: string, proc processData(client: RpcWebSocketClient) {.async.} = var error: ref CatchableError - when useNews: - try: - while true: - var value = await client.transport.receiveString() - if value == "": - # transmission ends - break + let ws = client.transport + try: + while ws.readyState != ReadyState.Closed: + var value = await ws.recvMsg() - client.processMessage(value) - except CatchableError as e: - error = e + if value.len == 0: + # transmission ends + break - client.transport.close() - else: - let ws = client.transport - try: - while ws.readyState != ReadyState.Closed: - var value = await ws.recvMsg() + client.processMessage(string.fromBytes(value)) + except CatchableError as e: + error = e - if value.len == 0: - # transmission ends - break - - client.processMessage(string.fromBytes(value)) - except CatchableError as e: - error = e - - await client.transport.close() + await client.transport.close() client.transport = nil @@ -118,53 +91,30 @@ proc addExtraHeaders( # Apply default origin discard headers.hasKeyOrPut("Origin", "http://localhost") -when useNews: - func toStringTable(headersTable: HttpTable): StringTableRef = - let res = newStringTable(modeCaseInsensitive) - for header in headersTable: - res[header.key] = header.value.join(",") - res - - proc connect*( - client: RpcWebSocketClient, - uri: string, - extraHeaders: HttpTable = default(HttpTable), - compression = false) {.async.} = - if compression: - warn "compression is not supported with the news back-end" - var headers = HttpTable.init() +proc connect*( + client: RpcWebSocketClient, + uri: string, + extraHeaders: HttpTable = default(HttpTable), + compression = false, + hooks: seq[Hook] = @[], + flags: set[TLSFlags] = {}) {.async.} = + proc headersHook(ctx: Hook, headers: var HttpTable): Result[void, string] = headers.addExtraHeaders(client, extraHeaders) - client.transport = await newWebSocket(uri, headers.toStringTable()) - client.uri = uri - client.loop = processData(client) -else: - proc connect*( - client: RpcWebSocketClient, - uri: string, - extraHeaders: HttpTable = default(HttpTable), - compression = false, - hooks: seq[Hook] = @[], - flags: set[TLSFlags] = {}) {.async.} = - proc headersHook(ctx: Hook, headers: var HttpTable): Result[void, string] = - headers.addExtraHeaders(client, extraHeaders) - ok() - var ext: seq[ExtFactory] = if compression: @[deflateFactory()] - else: @[] - let uri = parseUri(uri) - let ws = await WebSocket.connect( - uri=uri, - factories=ext, - hooks=hooks & Hook(append: headersHook), - flags=flags) - client.transport = ws - client.uri = uri - client.loop = processData(client) + ok() + var ext: seq[ExtFactory] = if compression: @[deflateFactory()] + else: @[] + let uri = parseUri(uri) + let ws = await WebSocket.connect( + uri=uri, + factories=ext, + hooks=hooks & Hook(append: headersHook), + flags=flags) + client.transport = ws + client.uri = uri + client.loop = processData(client) method close*(client: RpcWebSocketClient) {.async.} = await client.loop.cancelAndWait() if not client.transport.isNil: - when useNews: - client.transport.close() - else: - await client.transport.close() + await client.transport.close() client.transport = nil diff --git a/tests/all.nim b/tests/all.nim index 38e23f2..eb70851 100644 --- a/tests/all.nim +++ b/tests/all.nim @@ -1,12 +1,9 @@ {. warning[UnusedImport]:off .} import - ../json_rpc/clients/config - -import - testrpcmacro, testethcalls, testhttp, testserverclient - -when not useNews: - # The proxy implementation is based on websock - import testproxy - import testhook + testrpcmacro, + testethcalls, + testhttp, + testserverclient, + testproxy, + testhook diff --git a/tests/ethprocs.nim b/tests/ethprocs.nim index 8202b91..5f5a1ee 100644 --- a/tests/ethprocs.nim +++ b/tests/ethprocs.nim @@ -1,5 +1,4 @@ import - json, nimcrypto, stint, ethtypes, ethhexstrings, stintjson, ../json_rpc/rpcserver diff --git a/tests/helpers.nim b/tests/helpers.nim index 590fd33..3113973 100644 --- a/tests/helpers.nim +++ b/tests/helpers.nim @@ -1,5 +1,5 @@ import - json, ../json_rpc/router + ../json_rpc/router template `==`*(a, b: distinct (string|StringOfJson)): bool = string(a) == string(b) diff --git a/tests/testethcalls.nim b/tests/testethcalls.nim index ed9acd6..38791c5 100644 --- a/tests/testethcalls.nim +++ b/tests/testethcalls.nim @@ -1,5 +1,5 @@ import - unittest, json, tables, + unittest, tables, stint, ethtypes, ethprocs, stintjson, chronicles, ../json_rpc/[rpcclient, rpcserver], ./helpers diff --git a/tests/testhook.nim b/tests/testhook.nim index 326f11c..9ec25c4 100644 --- a/tests/testhook.nim +++ b/tests/testhook.nim @@ -1,7 +1,7 @@ import - unittest, json, chronicles, + unittest, websock/websock, - ../json_rpc/[rpcclient, rpcserver, clients/config] + ../json_rpc/[rpcclient, rpcserver] const serverHost = "localhost" diff --git a/tests/testhttp.nim b/tests/testhttp.nim index 4aabe8d..2c11dd5 100644 --- a/tests/testhttp.nim +++ b/tests/testhttp.nim @@ -1,4 +1,4 @@ -import unittest, json, strutils +import unittest, strutils import httputils import ../json_rpc/[rpcserver, rpcclient] diff --git a/tests/testhttps.nim b/tests/testhttps.nim index 0a7202e..633c224 100644 --- a/tests/testhttps.nim +++ b/tests/testhttps.nim @@ -1,4 +1,4 @@ -import unittest, json, strutils +import unittest, strutils import httputils import ../json_rpc/[rpcsecureserver, rpcclient] import chronos/[streams/tlsstream, apps/http/httpcommon] diff --git a/tests/testproxy.nim b/tests/testproxy.nim index 04ec16f..bac116d 100644 --- a/tests/testproxy.nim +++ b/tests/testproxy.nim @@ -1,11 +1,11 @@ import - unittest, json, chronicles, + unittest, chronicles, ../json_rpc/[rpcclient, rpcserver, rpcproxy] let srvAddress = initTAddress("127.0.0.1", Port(8545)) let proxySrvAddress = "localhost:8546" let proxySrvAddressForClient = "http://"&proxySrvAddress - + template registerMethods(srv: RpcServer, proxy: RpcProxy) = srv.rpc("myProc") do(input: string, data: array[0..3, int]): return %("Hello " & input & " data: " & $data) @@ -39,7 +39,7 @@ suite "Proxy RPC through http": test "Method missing on server and proxy server": expect(CatchableError): discard waitFor client.call("missingMethod", %[%"abc"]) - + waitFor srv.stop() waitFor srv.closeWait() waitFor proxy.stop() @@ -68,7 +68,7 @@ suite "Proxy RPC through websockets": test "Method missing on server and proxy server": expect(CatchableError): discard waitFor client.call("missingMethod", %[%"abc"]) - + srv.stop() waitFor srv.closeWait() waitFor proxy.stop() diff --git a/tests/testrpcmacro.nim b/tests/testrpcmacro.nim index 051b242..f0a6c8d 100644 --- a/tests/testrpcmacro.nim +++ b/tests/testrpcmacro.nim @@ -1,4 +1,4 @@ -import unittest, json, chronicles, options +import unittest, chronicles, options import ../json_rpc/rpcserver, ./helpers type diff --git a/tests/testserverclient.nim b/tests/testserverclient.nim index 2f012c9..033c06b 100644 --- a/tests/testserverclient.nim +++ b/tests/testserverclient.nim @@ -1,9 +1,6 @@ import - unittest, json, chronicles, - ../json_rpc/[rpcclient, rpcserver, clients/config] - -const - compressionSupported = useNews + unittest, chronicles, + ../json_rpc/[rpcclient, rpcserver] # Create RPC on server proc setupServer*(srv: RpcServer) = @@ -78,13 +75,13 @@ suite "Websocket Server/Client RPC": suite "Websocket Server/Client RPC with Compression": var srv = newRpcWebSocketServer("127.0.0.1", Port(8545), - compression = compressionSupported) + compression = true) var client = newRpcWebSocketClient() srv.setupServer() srv.start() waitFor client.connect("ws://127.0.0.1:8545/", - compression = compressionSupported) + compression = true) test "Successful RPC call": let r = waitFor client.call("myProc", %[%"abc", %[1, 2, 3, 4]])