diff --git a/lsp-server/src/core/dispatcher.cppm b/lsp-server/src/core/dispatcher.cppm index 21b5343..d6c653c 100644 --- a/lsp-server/src/core/dispatcher.cppm +++ b/lsp-server/src/core/dispatcher.cppm @@ -233,13 +233,22 @@ namespace lsp::core protocol::ErrorCodes code, std::string_view message) { - protocol::ResponseMessage response; - response.id = std::move(id); - response.error = protocol::ResponseError{ - .jsonrpc = "2.0", - .code = static_cast(code), - .message = std::string(message), - .data = std::nullopt, + protocol::LSPAny response_id(std::nullptr_t{}); + if (id) + { + response_id = std::visit( + [](const auto& value) -> protocol::LSPAny { return value; }, *id); + } + + protocol::LSPObject error{ + { "code", static_cast(code) }, + { "message", protocol::string(message) }, + }; + + protocol::LSPObject response{ + { "jsonrpc", protocol::string("2.0") }, + { "id", std::move(response_id) }, + { "error", std::move(error) }, }; return transform::Serialize(response).value(); } diff --git a/lsp-server/src/core/server.cppm b/lsp-server/src/core/server.cppm index 5189704..6b7d06f 100644 --- a/lsp-server/src/core/server.cppm +++ b/lsp-server/src/core/server.cppm @@ -65,6 +65,9 @@ export namespace lsp::core void SendError(const protocol::RequestMessage& request, protocol::ErrorCodes code, std::string_view message); + void SendError(std::optional id, + protocol::ErrorCodes code, + std::string_view message); std::istream& input_; std::ostream& output_; @@ -82,6 +85,32 @@ export namespace lsp::core namespace lsp::core { + namespace + { + std::optional ExtractRequestId( + const protocol::LSPObject& object) + { + const auto id = object.find("id"); + if (id == object.end()) + return std::nullopt; + if (id->second.Is()) + return protocol::RequestId{ id->second.Get() }; + if (id->second.Is()) + { + const auto value = id->second.Get(); + if (value <= static_cast( + std::numeric_limits::max())) + { + return protocol::RequestId{ static_cast(value) }; + } + return std::nullopt; + } + if (id->second.Is()) + return protocol::RequestId{ id->second.Get() }; + return std::nullopt; + } + } + LspServer::LspServer(std::istream& input, std::ostream& output, ProviderRegistrar registrar, @@ -190,13 +219,30 @@ namespace lsp::core void LspServer::HandleMessage(const std::string& raw_message) { auto any = transform::Deserialize(raw_message); - if (!any || !any->Is()) + if (!any) { - spdlog::warn("Failed to parse message: {}", raw_message); + SendError(std::nullopt, protocol::ErrorCodes::ParseError, "Parse error"); + return; + } + if (!any->Is()) + { + SendError(std::nullopt, + protocol::ErrorCodes::InvalidRequest, + "Invalid Request"); return; } const auto& object = any->Get(); + const auto id = ExtractRequestId(object); + const auto jsonrpc = object.find("jsonrpc"); + if (jsonrpc == object.end() || + !jsonrpc->second.Is() || + jsonrpc->second.Get() != "2.0") + { + SendError(id, protocol::ErrorCodes::InvalidRequest, "Invalid Request"); + return; + } + const bool has_id = object.contains("id"); const bool has_method = object.contains("method"); const bool has_result = object.contains("result"); @@ -204,15 +250,33 @@ namespace lsp::core if (has_method && has_id) { + const auto method = object.find("method"); + if (!id || !method->second.Is()) + { + SendError(id, protocol::ErrorCodes::InvalidRequest, "Invalid Request"); + return; + } if (auto request = transform::Deserialize(raw_message)) + { HandleRequest(*request); + } else - spdlog::warn("Failed to parse request message"); + { + SendError(id, protocol::ErrorCodes::InvalidRequest, "Invalid Request"); + } return; } if (has_method) { + const auto method = object.find("method"); + if (!method->second.Is()) + { + SendError(std::nullopt, + protocol::ErrorCodes::InvalidRequest, + "Invalid Request"); + return; + } if (auto notification = transform::Deserialize(raw_message)) { @@ -220,12 +284,14 @@ namespace lsp::core } else { - spdlog::warn("Failed to parse notification message"); + SendError(std::nullopt, + protocol::ErrorCodes::InvalidRequest, + "Invalid Request"); } return; } - if (has_id && (has_result || has_error)) + if (has_id && (has_result != has_error)) { if (auto response = transform::Deserialize(raw_message)) HandleResponse(*response); @@ -234,7 +300,13 @@ namespace lsp::core return; } - spdlog::warn("Unrecognized message: {}", raw_message); + if (has_result || has_error) + { + spdlog::warn("Ignoring invalid response-like message"); + return; + } + + SendError(id, protocol::ErrorCodes::InvalidRequest, "Invalid Request"); } void LspServer::SendMessage(const std::string& message) @@ -484,6 +556,13 @@ namespace lsp::core protocol::ErrorCodes code, std::string_view message) { - SendMessage(BuildErrorResponseMessage(request, code, message)); + SendError(request.id, code, message); + } + + void LspServer::SendError(std::optional id, + protocol::ErrorCodes code, + std::string_view message) + { + SendMessage(BuildErrorResponseMessage(std::move(id), code, message)); } } diff --git a/lsp-server/src/protocol/common/message.cppm b/lsp-server/src/protocol/common/message.cppm index bfebc60..93b0ec3 100644 --- a/lsp-server/src/protocol/common/message.cppm +++ b/lsp-server/src/protocol/common/message.cppm @@ -39,7 +39,6 @@ export namespace lsp::protocol struct ResponseError { - string jsonrpc = "2.0"; integer code; string message; std::optional data; diff --git a/lsp-server/src/protocol/protocol.cppm b/lsp-server/src/protocol/protocol.cppm index 10bd576..40165d6 100644 --- a/lsp-server/src/protocol/protocol.cppm +++ b/lsp-server/src/protocol/protocol.cppm @@ -50,7 +50,7 @@ namespace glz struct meta { using T = lsp::protocol::ResponseError; - static constexpr auto value = glz::object(&T::jsonrpc, &T::code, &T::message, &T::data); + static constexpr auto value = glz::object(&T::code, &T::message, &T::data); }; template<> diff --git a/lsp-server/test/test_core_server.py b/lsp-server/test/test_core_server.py index 3dbad83..7fcd2ee 100644 --- a/lsp-server/test/test_core_server.py +++ b/lsp-server/test/test_core_server.py @@ -14,6 +14,10 @@ def frame(message: dict) -> bytes: return f"Content-Length: {len(body)}\r\n\r\n".encode("ascii") + body +def raw_frame(body: bytes) -> bytes: + return f"Content-Length: {len(body)}\r\n\r\n".encode("ascii") + body + + def read_messages(data: bytes) -> list[dict]: messages = [] offset = 0 @@ -169,12 +173,48 @@ def assert_lifecycle(server: Path) -> None: client.kill() +def assert_json_rpc_errors(server: Path) -> None: + payload = b"".join([ + raw_frame(b'{"jsonrpc":"2.0",'), + raw_frame(b"[]"), + raw_frame(b'{"jsonrpc":"1.0","id":7,"method":"initialize"}'), + frame({ + "jsonrpc": "2.0", + "id": 8, + "method": "initialize", + "params": {}, + }), + frame({"jsonrpc": "2.0", "id": 9, "method": "missing/method"}), + frame({"jsonrpc": "2.0", "id": 10, "method": "shutdown"}), + frame({"jsonrpc": "2.0", "method": "exit"}), + ]) + result = run_raw(server, payload) + if result.returncode != 0: + raise RuntimeError("JSON-RPC error sequence should shut down cleanly") + + responses = read_messages(result.stdout) + if responses[0].get("id", "missing") is not None: + raise RuntimeError( + f"ParseError response should use a null id: {responses[0]!r}") + if responses[0].get("error", {}).get("code") != -32700: + raise RuntimeError("Malformed JSON should return ParseError") + if responses[1].get("id", "missing") is not None: + raise RuntimeError("InvalidRequest response should use a null id") + if responses[1].get("error", {}).get("code") != -32600: + raise RuntimeError("A non-object JSON value should return InvalidRequest") + if response_by_id(result.stdout, 7).get("error", {}).get("code") != -32600: + raise RuntimeError("jsonrpc other than 2.0 should return InvalidRequest") + if response_by_id(result.stdout, 9).get("error", {}).get("code") != -32601: + raise RuntimeError("Unknown request method should return MethodNotFound") + + def main() -> int: parser = argparse.ArgumentParser() parser.add_argument("--server", type=Path, required=True) args = parser.parse_args() assert_lifecycle(args.server) + assert_json_rpc_errors(args.server) return 0