feat(shada): restore Blob globals properly

As Strings and Blobs are encoded as msgpack BINs, the current ShaDa
implementation will restore global Blob variables as Strings (or msgpack
special dicts if they contain NULs).

Encode an additional element with Blob globals to differentiate them
from Strings so that we can restore them with the correct type.

Adjust variables_spec.lua's autotest() to also check for proper type.

3 files changed, 46 insertions, 7 deletions

diff --git a/src/nvim/shada.c b/src/nvim/shada.c
index c0e787380f..7d277fe5c8 100644
--- a/src/nvim/shada.c
+++ b/src/nvim/shada.c
@@ -1653,6 +1653,13 @@ static ShaDaWriteResult shada_pack_entry(msgpack_packer *const packer,
       break;
     }
     case kSDItemVariable: {
+      if (entry.data.global_var.value.v_type == VAR_TYPE_BLOB) {
+        // Strings and Blobs both pack as msgpack BINs; differentiate them by
+        // storing an additional VAR_TYPE_BLOB element alongside Blobs
+        list_T *const list = tv_list_alloc(1);
+        tv_list_append_number(list, VAR_TYPE_BLOB);
+        entry.data.global_var.additional_elements = list;
+      }
       const size_t arr_size = 2 + (size_t)(
           tv_list_len(entry.data.global_var.additional_elements));
       msgpack_pack_array(spacker, arr_size);
@@ -3937,15 +3944,38 @@ shada_read_next_item_start:
       entry->data.global_var.name =
           xmemdupz(unpacked.data.via.array.ptr[0].via.bin.ptr,
                    unpacked.data.via.array.ptr[0].via.bin.size);
-      if (msgpack_to_vim(unpacked.data.via.array.ptr[1],
-                         &(entry->data.global_var.value)) == FAIL) {
+      SET_ADDITIONAL_ELEMENTS(unpacked.data.via.array, 2,
+                              entry->data.global_var.additional_elements,
+                              "variable");
+      bool is_blob = false;
+      // A msgpack BIN could be a String or Blob; an additional VAR_TYPE_BLOB
+      // element is stored with Blobs which can be used to differentiate them
+      if (unpacked.data.via.array.ptr[1].type == MSGPACK_OBJECT_BIN) {
+        const listitem_T *type_item
+            = tv_list_first(entry->data.global_var.additional_elements);
+        if (type_item != NULL) {
+          const typval_T *type_tv = TV_LIST_ITEM_TV(type_item);
+          if (type_tv->v_type != VAR_NUMBER
+              || type_tv->vval.v_number != VAR_TYPE_BLOB) {
+            emsgf(_(READERR("variable", "has wrong variable type")),
+                  initial_fpos);
+            goto shada_read_next_item_error;
+          }
+          is_blob = true;
+        }
+      }
+      if (is_blob) {
+        const msgpack_object_bin *const bin
+          = &unpacked.data.via.array.ptr[1].via.bin;
+        blob_T *const blob = tv_blob_alloc();
+        ga_concat_len(&blob->bv_ga, bin->ptr, (size_t)bin->size);
+        tv_blob_set_ret(&entry->data.global_var.value, blob);
+      } else if (msgpack_to_vim(unpacked.data.via.array.ptr[1],
+                                &(entry->data.global_var.value)) == FAIL) {
         emsgf(_(READERR("variable", "has value that cannot "
                         "be converted to the VimL value")), initial_fpos);
         goto shada_read_next_item_error;
       }
-      SET_ADDITIONAL_ELEMENTS(unpacked.data.via.array, 2,
-                              entry->data.global_var.additional_elements,
-                              "variable");
       break;
     }
     case kSDItemSubString: {
diff --git a/test/functional/shada/errors_spec.lua b/test/functional/shada/errors_spec.lua
index 77a41caec7..ebfd73cf85 100644
--- a/test/functional/shada/errors_spec.lua
+++ b/test/functional/shada/errors_spec.lua
@@ -342,6 +342,11 @@ describe('ShaDa error handling', function()
     eq('Vim(rshada):E575: Error while reading ShaDa file: variable entry at position 0 has wrong variable name type', exc_exec(sdrcmd()))
   end)
 
+  it('fails on variable item with BIN value and type value != VAR_TYPE_BLOB', function()
+    wshada('\006\000\007\147\196\001\065\196\000\000')
+    eq('Vim(rshada):E575: Error while reading ShaDa file: variable entry at position 0 has wrong variable type', exc_exec(sdrcmd()))
+  end)
+
   it('fails on replacement item with NIL value', function()
     wshada('\003\000\001\192')
     eq('Vim(rshada):E575: Error while reading ShaDa file: sub string entry at position 0 is not an array', exc_exec(sdrcmd()))
diff --git a/test/functional/shada/variables_spec.lua b/test/functional/shada/variables_spec.lua
index cc0e7fa537..854add1363 100644
--- a/test/functional/shada/variables_spec.lua
+++ b/test/functional/shada/variables_spec.lua
@@ -1,7 +1,7 @@
 -- ShaDa variables saving/reading support
 local helpers = require('test.functional.helpers')(after_each)
-local meths, funcs, nvim_command, eq =
-  helpers.meths, helpers.funcs, helpers.command, helpers.eq
+local meths, funcs, nvim_command, eq, eval =
+  helpers.meths, helpers.funcs, helpers.command, helpers.eq, helpers.eval
 
 local shada_helpers = require('test.functional.shada.helpers')
 local reset, clear = shada_helpers.reset, shada_helpers.clear
@@ -30,10 +30,12 @@ describe('ShaDa support code', function()
       else
         meths.set_var(varname, varval)
       end
+      local vartype = eval('type(g:' .. varname .. ')')
       -- Exit during `reset` is not a regular exit: it does not write shada
       -- automatically
       nvim_command('qall')
       reset('set shada+=!')
+      eq(vartype, eval('type(g:' .. varname .. ')'))
       eq(varval, meths.get_var(varname))
     end)
   end
@@ -47,6 +49,8 @@ describe('ShaDa support code', function()
   autotest('false', 'FALSEVAR', false)
   autotest('null', 'NULLVAR', 'v:null', true)
   autotest('ext', 'EXTVAR', '{"_TYPE": v:msgpack_types.ext, "_VAL": [2, ["", ""]]}', true)
+  autotest('blob', 'BLOBVAR', '0z12ab34cd', true)
+  autotest('blob (with NULs)', 'BLOBVARNULS', '0z004e554c7300', true)
 
   it('does not read back variables without `!` in &shada', function()
     meths.set_var('STRVAR', 'foo')