From 160e271bcc653c312f751b3cfe61f4531d798942 Mon Sep 17 00:00:00 2001 From: Sukera Date: Mon, 17 Oct 2022 21:48:43 +0200 Subject: [PATCH 1/3] Fix missing response on unknown method --- src/typed.jl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/typed.jl b/src/typed.jl index 34cfe3bc..717c9536 100644 --- a/src/typed.jl +++ b/src/typed.jl @@ -59,6 +59,7 @@ function dispatch_msg(x::JSONRPCEndpoint, dispatcher::MsgDispatcher, msg) dispatcher._currentlyHandlingMsg = true try method_name = msg["method"] + is_request = haskey(msg, "id") handler = get(dispatcher._handlers, method_name, nothing) if handler !== nothing param_type = get_param_type(handler.message_type) @@ -78,6 +79,10 @@ function dispatch_msg(x::JSONRPCEndpoint, dispatcher::MsgDispatcher, msg) end end else + if is_request # even invalid requests MUST get a response - see Section 5 of the JSON RPC specification. + send_error_response(x, msg, -32601, "Unknown method.", nothing) + end + # TODO: ignoring notifications is legal, so there's no need to crash here on unknown things. However, removing this requires downstream support. error("Unknown method $method_name.") end finally From 698e9050788c30dae36d2a6d43b4cb3f6896a721 Mon Sep 17 00:00:00 2001 From: Sukera Date: Mon, 17 Oct 2022 22:46:44 +0200 Subject: [PATCH 2/3] Name unsupported methods in errors --- src/typed.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/typed.jl b/src/typed.jl index 717c9536..160bac21 100644 --- a/src/typed.jl +++ b/src/typed.jl @@ -80,10 +80,10 @@ function dispatch_msg(x::JSONRPCEndpoint, dispatcher::MsgDispatcher, msg) end else if is_request # even invalid requests MUST get a response - see Section 5 of the JSON RPC specification. - send_error_response(x, msg, -32601, "Unknown method.", nothing) + send_error_response(x, msg, -32601, "Unknown method '$method_name'.", nothing) end # TODO: ignoring notifications is legal, so there's no need to crash here on unknown things. However, removing this requires downstream support. - error("Unknown method $method_name.") + error("Unknown method '$method_name'.") end finally dispatcher._currentlyHandlingMsg = false From 9682a9302c1404822e734dda3c3968aff2c55785 Mon Sep 17 00:00:00 2001 From: Sukera Date: Mon, 17 Oct 2022 22:47:27 +0200 Subject: [PATCH 3/3] Add test for errors on wrongly requested methods --- test/test_typed.jl | 79 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 59 insertions(+), 20 deletions(-) diff --git a/test/test_typed.jl b/test/test_typed.jl index c2994222..7fb5dd0c 100644 --- a/test/test_typed.jl +++ b/test/test_typed.jl @@ -1,12 +1,16 @@ -@testset "Message dispatcher" begin - +function socket_path(id) if Sys.iswindows() - global_socket_name1 = "\\\\.\\pipe\\jsonrpc-testrun1" + return "\\\\.\\pipe\\jsonrpc-testrun$id" elseif Sys.isunix() - global_socket_name1 = joinpath(tempdir(), "jsonrpc-testrun1") + return joinpath(mktempdir(), "jsonrpc-testrun$id") else error("Unknown operating system.") end +end + +@testset "Message dispatcher" begin + + global_socket_name1 = socket_path(1) request1_type = JSONRPC.RequestType("request1", Foo, String) request2_type = JSONRPC.RequestType("request2", Nothing, String) @@ -14,14 +18,15 @@ global g_var = "" - server_is_up = Base.Condition() + server_is_up1 = Base.Condition() server_task = @async try server = listen(global_socket_name1) - notify(server_is_up) + notify(server_is_up1) + yield() # don't want to deadlock sock = accept(server) global conn = JSONRPC.JSONRPCEndpoint(sock, sock) - global msg_dispatcher = JSONRPC.MsgDispatcher() + msg_dispatcher = JSONRPC.MsgDispatcher() msg_dispatcher[request1_type] = (conn, params) -> begin @test JSONRPC.is_currently_handling_msg(msg_dispatcher) @@ -39,7 +44,7 @@ Base.display_error(stderr, err, catch_backtrace()) end - wait(server_is_up) + wait(server_is_up1) sock2 = connect(global_socket_name1) conn2 = JSONRPCEndpoint(sock2, sock2) @@ -63,22 +68,17 @@ # Now we test a faulty server - if Sys.iswindows() - global_socket_name2 = "\\\\.\\pipe\\jsonrpc-testrun2" - elseif Sys.isunix() - global_socket_name2 = joinpath(tempdir(), "jsonrpc-testrun2") - else - error("Unknown operating system.") - end + global_socket_name2 = socket_path(2) - server_is_up = Base.Condition() + server_is_up2 = Base.Condition() server_task2 = @async try server = listen(global_socket_name2) - notify(server_is_up) + notify(server_is_up2) + yield() # don't want to deadlock sock = accept(server) global conn = JSONRPC.JSONRPCEndpoint(sock, sock) - global msg_dispatcher = JSONRPC.MsgDispatcher() + msg_dispatcher = JSONRPC.MsgDispatcher() msg_dispatcher[request2_type] = (conn, params)->34 # The request type requires a `String` return, so this tests whether we get an error. @@ -91,7 +91,7 @@ Base.display_error(stderr, err, catch_backtrace()) end - wait(server_is_up) + wait(server_is_up2) sock2 = connect(global_socket_name2) conn2 = JSONRPCEndpoint(sock2, sock2) @@ -104,6 +104,45 @@ close(sock2) close(conn) - fetch(server_task) + fetch(server_task2) + + + # Now we test a wrongly requested method + + global_socket_name3 = socket_path(3) + + server_is_up3 = Base.Condition() + + server_task3 = @async try + server = listen(global_socket_name3) + notify(server_is_up3) + yield() # don't want to deadlock + sock = accept(server) + global conn = JSONRPC.JSONRPCEndpoint(sock, sock) + msg_dispatcher = JSONRPC.MsgDispatcher() + + run(conn) + + for msg in conn + @test_throws ErrorException("Unknown method 'request2'.") JSONRPC.dispatch_msg(conn, msg_dispatcher, msg) + flush(conn) + end + catch err + Base.display_error(stderr, err, catch_backtrace()) + end + + wait(server_is_up3) + + sock3 = connect(global_socket_name3) + conn3 = JSONRPCEndpoint(sock3, sock3) + + run(conn3) + + @test_throws JSONRPC.JSONRPCError(-32601, "Unknown method 'request2'.", nothing) JSONRPC.send(conn3, request2_type, nothing) + + close(conn3) + close(sock3) + close(conn) + fetch(server_task3) end