From 7f2d5d6883b5738682aa18ff84a20a632ffa9790 Mon Sep 17 00:00:00 2001 From: benarcher2691 Date: Mon, 1 Dec 2025 19:16:44 +0100 Subject: [PATCH] fix(msgpack): use fixstr encoding for strings of length 20-31 #36737 Problem: MessagePack fixstr format supports string lengths 0-31, but mpack_str() only used fixstr for lengths < 20. Strings of 20-31 bytes were incorrectly encoded as str8 (2-byte header) instead of fixstr (1-byte header). Solution: Change the condition from `len < 20` to `len < 32` to match the MessagePack specification. This fix affects message timing which exposed a pre-existing race condition in the channels_spec PTY test. The test now uses a helper function to accumulate partial PTY reads, making it more robust. Fixes #32784 --- src/nvim/msgpack_rpc/packer.c | 2 +- test/functional/core/channels_spec.lua | 18 +++++++++++++++++- test/functional/lua/mpack_spec.lua | 14 ++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/nvim/msgpack_rpc/packer.c b/src/nvim/msgpack_rpc/packer.c index 264e2157ef..6889fd885d 100644 --- a/src/nvim/msgpack_rpc/packer.c +++ b/src/nvim/msgpack_rpc/packer.c @@ -76,7 +76,7 @@ void mpack_float8(char **ptr, double i) void mpack_str(String str, PackerBuffer *packer) { const size_t len = str.size; - if (len < 20) { + if (len < 32) { mpack_w(&packer->ptr, 0xa0 | len); } else if (len < 0xff) { mpack_w(&packer->ptr, 0xd9); diff --git a/test/functional/core/channels_spec.lua b/test/functional/core/channels_spec.lua index c26f4b0fea..9e72975b29 100644 --- a/test/functional/core/channels_spec.lua +++ b/test/functional/core/channels_spec.lua @@ -196,6 +196,22 @@ describe('channels', function() end end + -- Helper to accumulate PTY stdout data until expected string is received. + -- PTY reads are non-atomic and may deliver data in chunks. + local function expect_stdout(id, expected) + local accumulated = '' + while #accumulated < #expected do + local msg = next_msg() + eq('notification', msg[1]) + eq('stdout', msg[2]) + eq(id, msg[3][1]) + for _, chunk in ipairs(msg[3][2]) do + accumulated = accumulated .. chunk + end + end + eq(expected, accumulated) + end + it('can use stdio channel with pty', function() skip(is_os('win')) source([[ @@ -229,7 +245,7 @@ describe('channels', function() expect_twoline(id, 'stdout', 'Blobs!\r', "[1, ['Blobs!', ''], 'stdin']") command("call chansend(id, 'neovan')") - eq({ 'notification', 'stdout', { id, { 'neovan' } } }, next_msg()) + expect_stdout(id, 'neovan') command("call chansend(id, '\127\127im\n')") expect_twoline(id, 'stdout', '\b \b\b \bim\r', "[1, ['neovim', ''], 'stdin']") diff --git a/test/functional/lua/mpack_spec.lua b/test/functional/lua/mpack_spec.lua index ebede26936..039911aa62 100644 --- a/test/functional/lua/mpack_spec.lua +++ b/test/functional/lua/mpack_spec.lua @@ -27,4 +27,18 @@ describe('lua vim.mpack', function() end) ) end) + + it('encodes dict keys of length 20-31 as fixstr #32784', function() + -- MessagePack fixstr format: 0xa0 | length (for lengths 0-31) + -- Before #36737, strings 20-31 bytes were incorrectly encoded as str8 (0xd9, len) + for len = 20, 31 do + local expected_header = string.char(0xa0 + len) -- fixstr header + local result = exec_lua(function(keylen) + local key = string.rep('x', keylen) + return vim.mpack.encode({ [key] = 1 }) + end, len) + -- Byte 1 is fixmap header (0x81), byte 2 should be fixstr header for the key + eq(expected_header, result:sub(2, 2), 'dict key length ' .. len .. ' should use fixstr') + end + end) end)