msgpack_rpc: Deal with deserialization failures

There seems to be no way to deal with failures when calling
`msgpack_unpacker_next`, so this reimplements that function as
`msgpack_rpc_unpack`, which has an additional result for detecting failures.

On top of that, we make use of the new function to properly return msgpack-rpc
errors when something bad happens.

3 files changed, 73 insertions, 2 deletions

diff --git a/src/nvim/os/channel.c b/src/nvim/os/channel.c
index 9a692cf9fe..b1e2a8a287 100644
--- a/src/nvim/os/channel.c
+++ b/src/nvim/os/channel.c
@@ -183,11 +183,13 @@ static void parse_msgpack(RStream *rstream, void *data, bool eof)
 
   msgpack_unpacked unpacked;
   msgpack_unpacked_init(&unpacked);
+  UnpackResult result;
+  msgpack_packer response;
 
   // Deserialize everything we can.
-  while (msgpack_unpacker_next(channel->unpacker, &unpacked)) {
+  while ((result = msgpack_rpc_unpack(channel->unpacker, &unpacked))
+      == kUnpackResultOk) {
     // Each object is a new msgpack-rpc request and requires an empty response
-    msgpack_packer response;
     msgpack_packer_init(&response, channel->sbuffer, msgpack_sbuffer_write);
     // Perform the call
     msgpack_rpc_call(channel->id, &unpacked.data, &response);
@@ -199,6 +201,29 @@ static void parse_msgpack(RStream *rstream, void *data, bool eof)
     // Clear the buffer for future calls
     msgpack_sbuffer_clear(channel->sbuffer);
   }
+
+  if (result == kUnpackResultFail) {
+    // See src/msgpack/unpack_template.h in msgpack source tree for
+    // causes for this error(search for 'goto _failed')
+    //
+    // A not so uncommon cause for this might be deserializing objects with
+    // a high nesting level: msgpack will break when it's internal parse stack
+    // size exceeds MSGPACK_EMBED_STACK_SIZE(defined as 32 by default)
+    msgpack_packer_init(&response, channel->sbuffer, msgpack_sbuffer_write);
+    msgpack_pack_array(&response, 4);
+    msgpack_pack_int(&response, 1);
+    msgpack_pack_int(&response, 0);
+    msgpack_rpc_error("Invalid msgpack payload. "
+                      "This error can also happen when deserializing "
+                      "an object with high level of nesting",
+                      &response);
+    wstream_write(channel->data.streams.write,
+                  wstream_new_buffer(channel->sbuffer->data,
+                                     channel->sbuffer->size,
+                                     true));
+    // Clear the buffer for future calls
+    msgpack_sbuffer_clear(channel->sbuffer);
+  }
 }
 
 static void send_event(Channel *channel, char *type, typval_T *data)
diff --git a/src/nvim/os/msgpack_rpc.c b/src/nvim/os/msgpack_rpc.c
index 932a7717fd..63e1245028 100644
--- a/src/nvim/os/msgpack_rpc.c
+++ b/src/nvim/os/msgpack_rpc.c
@@ -79,11 +79,13 @@ void msgpack_rpc_call(uint64_t id, msgpack_object *req, msgpack_packer *res)
              "Request array size is %u, it should be 4",
              req->via.array.size);
     msgpack_rpc_error(error_msg, res);
+    return;
   }
 
   if (req->via.array.ptr[1].type != MSGPACK_OBJECT_POSITIVE_INTEGER) {
     msgpack_pack_int(res, 0);  // no message id yet
     msgpack_rpc_error("Id must be a positive integer", res);
+    return;
   }
 
   // Set the response id, which is the same as the request
@@ -398,6 +400,31 @@ void msgpack_rpc_free_dictionary(Dictionary value)
   free(value.items);
 }
 
+UnpackResult msgpack_rpc_unpack(msgpack_unpacker* unpacker,
+                                msgpack_unpacked* result)
+{
+  if (result->zone != NULL) {
+    msgpack_zone_free(result->zone);
+  }
+
+  int res = msgpack_unpacker_execute(unpacker);
+
+  if (res > 0) {
+    result->zone = msgpack_unpacker_release_zone(unpacker);
+    result->data = msgpack_unpacker_data(unpacker);
+    msgpack_unpacker_reset(unpacker);
+    return kUnpackResultOk;
+  }
+
+  if (res < 0) {
+    // Since we couldn't parse it, destroy the data consumed so far
+    msgpack_unpacker_reset(unpacker);
+    return kUnpackResultFail;
+  }
+
+  return kUnpackResultNeedMore;
+}
+
 REMOTE_FUNCS_IMPL(Buffer, buffer)
 REMOTE_FUNCS_IMPL(Window, window)
 REMOTE_FUNCS_IMPL(Tabpage, tabpage)
diff --git a/src/nvim/os/msgpack_rpc.h b/src/nvim/os/msgpack_rpc.h
index c8f243e2cf..baabff20aa 100644
--- a/src/nvim/os/msgpack_rpc.h
+++ b/src/nvim/os/msgpack_rpc.h
@@ -9,6 +9,12 @@
 #include "nvim/func_attr.h"
 #include "nvim/api/private/defs.h"
 
+typedef enum {
+  kUnpackResultOk,        /// Successfully parsed a document
+  kUnpackResultFail,      /// Got unexpected input
+  kUnpackResultNeedMore   /// Need more data
+} UnpackResult;
+
 /// Validates the basic structure of the msgpack-rpc call and fills `res`
 /// with the basic response structure.
 ///
@@ -40,6 +46,19 @@ void msgpack_rpc_dispatch(uint64_t id,
                           msgpack_packer *res)
   FUNC_ATTR_NONNULL_ARG(2) FUNC_ATTR_NONNULL_ARG(3);
 
+/// Try to unpack a msgpack document from the data in the unpacker buffer. This
+/// function is a replacement to msgpack.h `msgpack_unpack_next` that lets
+/// the called know if the unpacking failed due to bad input or due to missing
+/// data.
+///
+/// @param unpacker The unpacker containing the parse buffer
+/// @param result The result which will contain the parsed object
+/// @return kUnpackResultOk      : An object was parsed
+///         kUnpackResultFail    : Got bad input
+///         kUnpackResultNeedMore: Need more data
+UnpackResult msgpack_rpc_unpack(msgpack_unpacker* unpacker,
+                                msgpack_unpacked* result);
+
 /// Finishes the msgpack-rpc call with an error message.
 ///
 /// @param msg The error message