From 46b6859a4f0298b63b93fe07612a593b44d99ea6 Mon Sep 17 00:00:00 2001 From: atusy <30277794+atusy@users.noreply.github.com> Date: Fri, 24 Apr 2026 05:41:59 +0900 Subject: [PATCH] fix(lsp): handle null id in JSON-RPC responses #38340 Problem: LSP spec allows response message to have a null request-id. This may happen when for example client sends unparseable request. https://github.com/microsoft/language-server-protocol/issues/196 Solution: Guard the server response branches against id=vim.NIL (json null), and handle error responses with null id by logging a warning and dispatching on error. Problem: CI (ubuntu asan, ubuntu tsan, windows) reports `uv_loop_close() hang?` from the two new null-id response tests. The leaked handle is the server-side accepted TCP socket created inside `server:listen` callback. The tests closed only the listener but not the accepted socket, so libuv could not finish shutting down the loop and each test session took ~2s extra to exit. Solution: Hoist the accepted socket to the outer `exec_lua` scope and close it at teardown before closing the listener. The close runs synchronously inside `exec_lua`, so the loop has time to dispose the handle before the session exits. * test(lsp): close accepted socket on read-loop exit/error Match the precedent in the handler test ("handler can return false as response") and the shared `_create_tcp_server` helper in `test/functional/plugin/lsp/testutil.lua`: close the accepted socket from inside the `create_read_loop` exit/error callbacks. The teardown close added in the previous commit remains as belt-and-suspenders, so the socket is disposed whether the server goes away first or the client does. --- runtime/lua/vim/lsp/rpc.lua | 8 +- test/functional/plugin/lsp_spec.lua | 154 ++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 1 deletion(-) diff --git a/runtime/lua/vim/lsp/rpc.lua b/runtime/lua/vim/lsp/rpc.lua index 3e94767086..234f895f2b 100644 --- a/runtime/lua/vim/lsp/rpc.lua +++ b/runtime/lua/vim/lsp/rpc.lua @@ -453,7 +453,9 @@ function Client:handle_body(body) log.debug('rpc.receive', decoded) -- Received a request. - if type(decoded.method) == 'string' and decoded.id then + if type(decoded.method) == 'string' and decoded.id and decoded.id ~= vim.NIL then + local id_type = type(decoded.id) + assert(id_type == 'number' or id_type == 'string', 'Request id must be a number or a string') -- Schedule here so that the users functions don't trigger an error and -- we can still use the result. vim.schedule(coroutine.wrap(function() @@ -501,6 +503,7 @@ function Client:handle_body(body) -- * If 'error' is nil, then 'result' must be present. -- * If 'result' is nil, then 'error' must be present (and not vim.NIL). decoded.id + and decoded.id ~= vim.NIL and ( (decoded.error == nil and decoded.result ~= nil) or (decoded.result == nil and decoded.error ~= nil and decoded.error ~= vim.NIL) @@ -549,6 +552,9 @@ function Client:handle_body(body) self:on_error(M.client_errors.NO_RESULT_CALLBACK_FOUND, decoded) log.error('No callback found for server response id ' .. result_id) end + elseif decoded.id == vim.NIL then + log.warn('Server sent response with null id', decoded.error) + self:on_error(M.client_errors.INVALID_SERVER_MESSAGE, decoded) elseif type(decoded.method) == 'string' then -- Received a notification. self:try_call( diff --git a/test/functional/plugin/lsp_spec.lua b/test/functional/plugin/lsp_spec.lua index 84eb63beea..cd6629b378 100644 --- a/test/functional/plugin/lsp_spec.lua +++ b/test/functional/plugin/lsp_spec.lua @@ -3040,6 +3040,160 @@ describe('LSP', function() } eq(expected, result) end) + + it('does not crash on error response with null id (JSON-RPC 2.0 parse error)', function() + local result = exec_lua(function() + local server = assert(vim.uv.new_tcp()) + local accepted + local messages = {} + server:bind('127.0.0.1', 0) + server:listen(127, function(err) + assert(not err, err) + accepted = assert(vim.uv.new_tcp()) + server:accept(accepted) + accepted:read_start(require('vim.lsp.rpc').create_read_loop(function(body) + local payload = vim.json.decode(body) + if payload.method then + table.insert(messages, payload.method) + if payload.method == 'initialize' then + -- Send a valid initialize response first + local msg = vim.json.encode({ + id = payload.id, + jsonrpc = '2.0', + result = { + capabilities = {}, + }, + }) + accepted:write( + table.concat({ 'Content-Length: ', tostring(#msg), '\r\n\r\n', msg }) + ) + elseif payload.method == 'initialized' then + -- Then send an error response with null id (parse error per JSON-RPC 2.0 ยง5) + local msg = + '{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":null}' + accepted:write( + table.concat({ 'Content-Length: ', tostring(#msg), '\r\n\r\n', msg }) + ) + end + end + end, function() + if accepted and not accepted:is_closing() then + accepted:close() + end + end, function() + if accepted and not accepted:is_closing() then + accepted:close() + end + end)) + end) + local port = server:getsockname().port + local on_error_called = false + local client_id = assert(vim.lsp.start({ + name = 'null-id-test', + cmd = vim.lsp.rpc.connect('127.0.0.1', port), + on_error = function(_code, _err) + on_error_called = true + end, + })) + vim.lsp.get_client_by_id(client_id) + -- Wait for the initialized notification to be sent and the null-id error to be received + vim.wait(1000, function() + return #messages >= 2 and on_error_called + end) + if accepted and not accepted:is_closing() then + accepted:close() + end + server:close() + server:shutdown() + return { + messages = messages, + on_error_called = on_error_called, + } + end) + -- The key assertion: Neovim should not crash, and the error handler should be called + eq(true, result.on_error_called) + eq(true, #result.messages >= 2) + end) + + it('does not misclassify server request with null id as notification', function() + local result = exec_lua(function() + local server = assert(vim.uv.new_tcp()) + local accepted + local messages = {} + server:bind('127.0.0.1', 0) + server:listen(127, function(err) + assert(not err, err) + accepted = assert(vim.uv.new_tcp()) + server:accept(accepted) + accepted:read_start(require('vim.lsp.rpc').create_read_loop(function(body) + local payload = vim.json.decode(body) + if payload.method then + table.insert(messages, payload.method) + if payload.method == 'initialize' then + local msg = vim.json.encode({ + id = payload.id, + jsonrpc = '2.0', + result = { + capabilities = {}, + }, + }) + accepted:write( + table.concat({ 'Content-Length: ', tostring(#msg), '\r\n\r\n', msg }) + ) + elseif payload.method == 'initialized' then + -- Send a server request with null id (invalid per JSON-RPC 2.0) + local msg = + '{"jsonrpc":"2.0","method":"workspace/configuration","params":{"items":[]},"id":null}' + accepted:write( + table.concat({ 'Content-Length: ', tostring(#msg), '\r\n\r\n', msg }) + ) + end + end + end, function() + if accepted and not accepted:is_closing() then + accepted:close() + end + end, function() + if accepted and not accepted:is_closing() then + accepted:close() + end + end)) + end) + local port = server:getsockname().port + local on_error_called = false + local notification_received = false + local client_id = assert(vim.lsp.start({ + name = 'null-id-request-test', + cmd = vim.lsp.rpc.connect('127.0.0.1', port), + on_error = function(_code, _err) + on_error_called = true + end, + handlers = { + ['workspace/configuration'] = function() + notification_received = true + return {} + end, + }, + })) + vim.lsp.get_client_by_id(client_id) + vim.wait(1000, function() + return #messages >= 2 and (on_error_called or notification_received) + end) + if accepted and not accepted:is_closing() then + accepted:close() + end + server:close() + server:shutdown() + return { + messages = messages, + on_error_called = on_error_called, + notification_received = notification_received, + } + end) + -- Should be dispatched as an error, NOT silently handled as a notification + eq(true, result.on_error_called) + eq(false, result.notification_received) + end) end) describe('#dynamic vim.lsp._dynamic', function()