From c9763beaea2d0cab179d2831a6902f2ee56fa264 Mon Sep 17 00:00:00 2001 From: Peter Hodge Date: Mon, 9 Jul 2018 16:17:41 +1000 Subject: [PATCH 1/5] msgpack_rpc_stream: copy of hexdump() helper from neovim project --- nvim/msgpack_rpc_stream.lua | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/nvim/msgpack_rpc_stream.lua b/nvim/msgpack_rpc_stream.lua index 667130b..aa8d08a 100644 --- a/nvim/msgpack_rpc_stream.lua +++ b/nvim/msgpack_rpc_stream.lua @@ -11,6 +11,31 @@ local Tabpage = {} Tabpage.__index = Tabpage function Tabpage.new(id) return setmetatable({id=id}, Tabpage) end +local function hexdump(str) + local len = string.len(str) + local dump = "" + local hex = "" + local asc = "" + + for i = 1, len do + if 1 == i % 8 then + dump = dump .. hex .. asc .. "\n" + hex = string.format("%04x: ", i - 1) + asc = "" + end + + local ord = string.byte(str, i) + hex = hex .. string.format("%02x ", ord) + if ord >= 32 and ord <= 126 then + asc = asc .. string.char(ord) + else + asc = asc .. "." + end + end + + return dump .. hex .. string.rep(" ", 8 - len % 8) .. asc +end + local Response = {} Response.__index = Response From e3ba2e3b0e9146a757e277ab0eef337593ac6d7e Mon Sep 17 00:00:00 2001 From: Peter Hodge Date: Mon, 9 Jul 2018 16:18:00 +1000 Subject: [PATCH 2/5] msgpack_rpc_stream: additional debug output for failed unpack --- nvim/msgpack_rpc_stream.lua | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/nvim/msgpack_rpc_stream.lua b/nvim/msgpack_rpc_stream.lua index aa8d08a..ec720f0 100644 --- a/nvim/msgpack_rpc_stream.lua +++ b/nvim/msgpack_rpc_stream.lua @@ -101,12 +101,27 @@ function MsgpackRpcStream:read_start(request_cb, notification_cb, eof_cb) if not data then return eof_cb() end - local type, id_or_cb + local status, type, id_or_cb local pos = 1 local len = #data while pos <= len do - type, id_or_cb, method_or_error, args_or_result, pos = - self._session:receive(data, pos) + -- grab a copy of pos since pcall() will set it to nil on error + local oldpos = pos + status, type, id_or_cb, method_or_error, args_or_result, pos = pcall( + self._session.receive, self._session, data, pos) + if not status then + -- write the full blob of bad data to a specific file + local outfile = io.open('./msgpack-invalid-data', 'w') + outfile:write(data) + outfile:close() + + -- build a printable representation of the bad part of the string + local printable = hexdump(data:sub(oldpos, oldpos + 8 * 10)) + + print(string.format("Error deserialising msgpack data stream at pos %d:\n%s\n", + oldpos, printable)) + error(type) + end if type == 'request' or type == 'notification' then if type == 'request' then request_cb(method_or_error, args_or_result, Response.new(self, From e3434778119fad9a9051ad94e0481021a9df49b5 Mon Sep 17 00:00:00 2001 From: Peter Hodge Date: Mon, 9 Jul 2018 16:15:03 +1000 Subject: [PATCH 3/5] msgpack_rpc_stream: provide context for invalid data --- nvim/msgpack_rpc_stream.lua | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nvim/msgpack_rpc_stream.lua b/nvim/msgpack_rpc_stream.lua index ec720f0..92a2b69 100644 --- a/nvim/msgpack_rpc_stream.lua +++ b/nvim/msgpack_rpc_stream.lua @@ -64,6 +64,7 @@ MsgpackRpcStream.__index = MsgpackRpcStream function MsgpackRpcStream.new(stream) return setmetatable({ _stream = stream, + _previous_chunk = nil, _pack = mpack.Packer({ ext = { [Buffer] = function(o) return 0, mpack.pack(o.id) end, @@ -120,9 +121,11 @@ function MsgpackRpcStream:read_start(request_cb, notification_cb, eof_cb) print(string.format("Error deserialising msgpack data stream at pos %d:\n%s\n", oldpos, printable)) + print(string.format("... occurred after %s", self._previous_chunk)) error(type) end if type == 'request' or type == 'notification' then + self._previous_chunk = string.format('%s<%s>', type, method_or_error) if type == 'request' then request_cb(method_or_error, args_or_result, Response.new(self, id_or_cb)) @@ -130,6 +133,7 @@ function MsgpackRpcStream:read_start(request_cb, notification_cb, eof_cb) notification_cb(method_or_error, args_or_result) end elseif type == 'response' then + self._previous_chunk = string.format('response<%s,%s>', id_or_cb, args_or_result) if method_or_error == mpack.NIL then method_or_error = nil else From 659d082933bb7e8e5b345976c34047cb5ed37ea9 Mon Sep 17 00:00:00 2001 From: Peter Hodge Date: Mon, 9 Jul 2018 16:39:32 +1000 Subject: [PATCH 4/5] msgpack_rpc_stream: don't shadow the 'type' function --- nvim/msgpack_rpc_stream.lua | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/nvim/msgpack_rpc_stream.lua b/nvim/msgpack_rpc_stream.lua index 92a2b69..bc4e797 100644 --- a/nvim/msgpack_rpc_stream.lua +++ b/nvim/msgpack_rpc_stream.lua @@ -102,13 +102,13 @@ function MsgpackRpcStream:read_start(request_cb, notification_cb, eof_cb) if not data then return eof_cb() end - local status, type, id_or_cb + local status, type_, id_or_cb local pos = 1 local len = #data while pos <= len do -- grab a copy of pos since pcall() will set it to nil on error local oldpos = pos - status, type, id_or_cb, method_or_error, args_or_result, pos = pcall( + status, type_, id_or_cb, method_or_error, args_or_result, pos = pcall( self._session.receive, self._session, data, pos) if not status then -- write the full blob of bad data to a specific file @@ -122,17 +122,17 @@ function MsgpackRpcStream:read_start(request_cb, notification_cb, eof_cb) print(string.format("Error deserialising msgpack data stream at pos %d:\n%s\n", oldpos, printable)) print(string.format("... occurred after %s", self._previous_chunk)) - error(type) + error(type_) end - if type == 'request' or type == 'notification' then - self._previous_chunk = string.format('%s<%s>', type, method_or_error) - if type == 'request' then + if type_ == 'request' or type_ == 'notification' then + self._previous_chunk = string.format('%s<%s>', type_, method_or_error) + if type_ == 'request' then request_cb(method_or_error, args_or_result, Response.new(self, id_or_cb)) else notification_cb(method_or_error, args_or_result) end - elseif type == 'response' then + elseif type_ == 'response' then self._previous_chunk = string.format('response<%s,%s>', id_or_cb, args_or_result) if method_or_error == mpack.NIL then method_or_error = nil From 3dc2c4822a341a2fa55163f00a63c792e473c9ef Mon Sep 17 00:00:00 2001 From: Peter Hodge Date: Mon, 9 Jul 2018 16:40:45 +1000 Subject: [PATCH 5/5] msgpack_rpc_stream: use a string.format() that's valid Functions and tables can't be formatted using %s --- nvim/msgpack_rpc_stream.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nvim/msgpack_rpc_stream.lua b/nvim/msgpack_rpc_stream.lua index bc4e797..8f61d32 100644 --- a/nvim/msgpack_rpc_stream.lua +++ b/nvim/msgpack_rpc_stream.lua @@ -133,7 +133,7 @@ function MsgpackRpcStream:read_start(request_cb, notification_cb, eof_cb) notification_cb(method_or_error, args_or_result) end elseif type_ == 'response' then - self._previous_chunk = string.format('response<%s,%s>', id_or_cb, args_or_result) + self._previous_chunk = string.format('response<%s>', type(args_or_result)) if method_or_error == mpack.NIL then method_or_error = nil else