Merge pull request #3359 from ZyX-I/fix-msgpack

Fix crash in msgpackparse function

src/nvim/eval.c

@@ -12731,6 +12731,7 @@ static void f_msgpackparse(typval_T *argvars, typval_T *rettv)
 {
   if (argvars[0].v_type != VAR_LIST) {
     EMSG2(_(e_listarg), "msgpackparse()");
+    return;
   }
   list_T *ret_list = rettv_list_alloc(rettv);
   const list_T *list = argvars[0].vval.v_list;

test/functional/eval/msgpack_functions_spec.lua

@@ -3,16 +3,18 @@ local clear, feed, execute = helpers.clear, helpers.feed, helpers.execute
 local eval, eq, neq = helpers.eval, helpers.eq, helpers.neq
 local execute, source = helpers.execute, helpers.source
 local nvim = helpers.nvim
+local exc_exec = helpers.exc_exec
+
 describe('msgpack*() functions', function()
-  before_each(function()
+  before_each(clear)
-    clear()
+
-  end)
   local obj_test = function(msg, obj)
     it(msg, function()
       nvim('set_var', 'obj', obj)
       eq(obj, eval('msgpackparse(msgpackdump(g:obj))'))
     end)
   end
+
   -- Regression test: msgpack_list_write was failing to write buffer with zero 
   -- length.
   obj_test('are able to dump and restore {"file": ""}', {{file=''}})
@@ -327,75 +329,6 @@ describe('msgpack*() functions', function()
 
   obj_test('are able to dump and restore floating-point value', {0.125})
 
-  it('restore nil as special dict', function()
-    execute('let dumped = ["\\xC0"]')
-    execute('let parsed = msgpackparse(dumped)')
-    eq({{_TYPE={}, _VAL=0}}, eval('parsed'))
-    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.nil'))
-  end)
-
-  it('restore boolean false as zero', function()
-    execute('let dumped = ["\\xC2"]')
-    execute('let parsed = msgpackparse(dumped)')
-    eq({{_TYPE={}, _VAL=0}}, eval('parsed'))
-    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.boolean'))
-  end)
-
-  it('restore boolean true as one', function()
-    execute('let dumped = ["\\xC3"]')
-    execute('let parsed = msgpackparse(dumped)')
-    eq({{_TYPE={}, _VAL=1}}, eval('parsed'))
-    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.boolean'))
-  end)
-
-  it('dump string as BIN 8', function()
-    nvim('set_var', 'obj', {'Test'})
-    eq({"\196\004Test"}, eval('msgpackdump(obj)'))
-  end)
-
-  it('restore FIXSTR as special dict', function()
-    execute('let dumped = ["\\xa2ab"]')
-    execute('let parsed = msgpackparse(dumped)')
-    eq({{_TYPE={}, _VAL={'ab'}}}, eval('parsed'))
-    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.string'))
-  end)
-
-  it('restore BIN 8 as string', function()
-    execute('let dumped = ["\\xC4\\x02ab"]')
-    eq({'ab'}, eval('msgpackparse(dumped)'))
-  end)
-
-  it('restore FIXEXT1 as special dictionary', function()
-    execute('let dumped = ["\\xD4\\x10", ""]')
-    execute('let parsed = msgpackparse(dumped)')
-    eq({{_TYPE={}, _VAL={0x10, {"", ""}}}}, eval('parsed'))
-    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.ext'))
-  end)
-
-  it('restore MAP with BIN key as special dictionary', function()
-    execute('let dumped = ["\\x81\\xC4\\x01a\\xC4\\n"]')
-    execute('let parsed = msgpackparse(dumped)')
-    eq({{_TYPE={}, _VAL={{'a', ''}}}}, eval('parsed'))
-    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.map'))
-  end)
-
-  it('restore MAP with duplicate STR keys as special dictionary', function()
-    execute('let dumped = ["\\x82\\xA1a\\xC4\\n\\xA1a\\xC4\\n"]')
-    execute('let parsed = msgpackparse(dumped)')
-    eq({{_TYPE={}, _VAL={{{_TYPE={}, _VAL={'a'}}, ''},
-                         {{_TYPE={}, _VAL={'a'}}, ''}}}}, eval('parsed'))
-    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.map'))
-    eq(1, eval('g:parsed[0]._VAL[0][0]._TYPE is v:msgpack_types.string'))
-    eq(1, eval('g:parsed[0]._VAL[1][0]._TYPE is v:msgpack_types.string'))
-  end)
-
-  it('restore MAP with MAP key as special dictionary', function()
-    execute('let dumped = ["\\x81\\x80\\xC4\\n"]')
-    execute('let parsed = msgpackparse(dumped)')
-    eq({{_TYPE={}, _VAL={{{}, ''}}}}, eval('parsed'))
-    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.map'))
-  end)
-
   it('can restore and dump UINT64_MAX', function()
     execute('let dumped = ["\\xCF" . repeat("\\xFF", 8)]')
     execute('let parsed = msgpackparse(dumped)')
@@ -449,6 +382,127 @@ describe('msgpack*() functions', function()
     eq({"\n"}, eval('parsed'))
     eq(1, eval('dumped ==# dumped2'))
   end)
+end)
+
+describe('msgpackparse() function', function()
+  before_each(clear)
+
+  it('restores nil as special dict', function()
+    execute('let dumped = ["\\xC0"]')
+    execute('let parsed = msgpackparse(dumped)')
+    eq({{_TYPE={}, _VAL=0}}, eval('parsed'))
+    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.nil'))
+  end)
+
+  it('restores boolean false as zero', function()
+    execute('let dumped = ["\\xC2"]')
+    execute('let parsed = msgpackparse(dumped)')
+    eq({{_TYPE={}, _VAL=0}}, eval('parsed'))
+    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.boolean'))
+  end)
+
+  it('restores boolean true as one', function()
+    execute('let dumped = ["\\xC3"]')
+    execute('let parsed = msgpackparse(dumped)')
+    eq({{_TYPE={}, _VAL=1}}, eval('parsed'))
+    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.boolean'))
+  end)
+
+  it('restores FIXSTR as special dict', function()
+    execute('let dumped = ["\\xa2ab"]')
+    execute('let parsed = msgpackparse(dumped)')
+    eq({{_TYPE={}, _VAL={'ab'}}}, eval('parsed'))
+    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.string'))
+  end)
+
+  it('restores BIN 8 as string', function()
+    execute('let dumped = ["\\xC4\\x02ab"]')
+    eq({'ab'}, eval('msgpackparse(dumped)'))
+  end)
+
+  it('restores FIXEXT1 as special dictionary', function()
+    execute('let dumped = ["\\xD4\\x10", ""]')
+    execute('let parsed = msgpackparse(dumped)')
+    eq({{_TYPE={}, _VAL={0x10, {"", ""}}}}, eval('parsed'))
+    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.ext'))
+  end)
+
+  it('restores MAP with BIN key as special dictionary', function()
+    execute('let dumped = ["\\x81\\xC4\\x01a\\xC4\\n"]')
+    execute('let parsed = msgpackparse(dumped)')
+    eq({{_TYPE={}, _VAL={{'a', ''}}}}, eval('parsed'))
+    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.map'))
+  end)
+
+  it('restores MAP with duplicate STR keys as special dictionary', function()
+    execute('let dumped = ["\\x82\\xA1a\\xC4\\n\\xA1a\\xC4\\n"]')
+    execute('let parsed = msgpackparse(dumped)')
+    eq({{_TYPE={}, _VAL={ {{_TYPE={}, _VAL={'a'}}, ''},
+                          {{_TYPE={}, _VAL={'a'}}, ''}}} }, eval('parsed'))
+    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.map'))
+    eq(1, eval('g:parsed[0]._VAL[0][0]._TYPE is v:msgpack_types.string'))
+    eq(1, eval('g:parsed[0]._VAL[1][0]._TYPE is v:msgpack_types.string'))
+  end)
+
+  it('restores MAP with MAP key as special dictionary', function()
+    execute('let dumped = ["\\x81\\x80\\xC4\\n"]')
+    execute('let parsed = msgpackparse(dumped)')
+    eq({{_TYPE={}, _VAL={{{}, ''}}}}, eval('parsed'))
+    eq(1, eval('g:parsed[0]._TYPE is v:msgpack_types.map'))
+  end)
+
+  it('msgpackparse(systemlist(...)) does not segfault. #3135', function()
+    local cmd = "msgpackparse(systemlist('"
+      ..helpers.nvim_prog.." --api-info'))['_TYPE']['_VAL'][0][0]"
+    local api_info = eval(cmd)
+    api_info = eval(cmd) -- do it again (try to force segfault)
+    api_info = eval(cmd) -- do it again
+    eq('functions', api_info)
+  end)
+
+  it('fails when called with no arguments', function()
+    eq('Vim(call):E119: Not enough arguments for function: msgpackparse',
+       exc_exec('call msgpackparse()'))
+  end)
+
+  it('fails when called with two arguments', function()
+    eq('Vim(call):E118: Too many arguments for function: msgpackparse',
+       exc_exec('call msgpackparse(["", ""], 1)'))
+  end)
+
+  it('fails to parse a string', function()
+    eq('Vim(call):E686: Argument of msgpackparse() must be a List',
+       exc_exec('call msgpackparse("abcdefghijklmnopqrstuvwxyz")'))
+  end)
+
+  it('fails to parse a number', function()
+    eq('Vim(call):E686: Argument of msgpackparse() must be a List',
+       exc_exec('call msgpackparse(127)'))
+  end)
+
+  it('fails to parse a dictionary', function()
+    eq('Vim(call):E686: Argument of msgpackparse() must be a List',
+       exc_exec('call msgpackparse({})'))
+  end)
+
+  it('fails to parse a funcref', function()
+    eq('Vim(call):E686: Argument of msgpackparse() must be a List',
+       exc_exec('call msgpackparse(function("tr"))'))
+  end)
+
+  it('fails to parse a float', function()
+    eq('Vim(call):E686: Argument of msgpackparse() must be a List',
+       exc_exec('call msgpackparse(0.0)'))
+  end)
+end)
+
+describe('msgpackdump() function', function()
+  before_each(clear)
+
+  it('dumps string as BIN 8', function()
+    nvim('set_var', 'obj', {'Test'})
+    eq({"\196\004Test"}, eval('msgpackdump(obj)'))
+  end)
 
   it('can dump generic mapping with generic mapping keys and values', function()
     execute('let todump = {"_TYPE": v:msgpack_types.map, "_VAL": []}')
@@ -487,129 +541,91 @@ describe('msgpack*() functions', function()
 
   it('fails to dump a function reference', function()
     execute('let Todump = function("tr")')
-    execute([[
+    eq('Vim(call):E475: Invalid argument: attempt to dump function reference',
-    try
+       exc_exec('call msgpackdump([Todump])'))
-      let dumped = msgpackdump([Todump])
-      let exception = 0
-    catch
-      let exception = v:exception
-    endtry
-    ]])
-    eq('Vim(let):E475: Invalid argument: attempt to dump function reference',
-       eval('exception'))
   end)
 
   it('fails to dump a function reference in a list', function()
     execute('let todump = [function("tr")]')
-    execute([[
+    eq('Vim(call):E475: Invalid argument: attempt to dump function reference',
-    try
+       exc_exec('call msgpackdump([todump])'))
-      let dumped = msgpackdump([todump])
-      let exception = 0
-    catch
-      let exception = v:exception
-    endtry
-    ]])
-    eq('Vim(let):E475: Invalid argument: attempt to dump function reference',
-       eval('exception'))
   end)
 
   it('fails to dump a recursive list', function()
     execute('let todump = [[[]]]')
     execute('call add(todump[0][0], todump)')
-    execute([[
+    eq('Vim(call):E475: Invalid argument: container references itself',
-    try
+       exc_exec('call msgpackdump([todump])'))
-      let dumped = msgpackdump([todump])
-      let exception = 0
-    catch
-      let exception = v:exception
-    endtry
-    ]])
-    eq('Vim(let):E475: Invalid argument: container references itself',
-       eval('exception'))
   end)
 
   it('fails to dump a recursive dict', function()
     execute('let todump = {"d": {"d": {}}}')
     execute('call extend(todump.d.d, {"d": todump})')
-    execute([[
+    eq('Vim(call):E475: Invalid argument: container references itself',
-    try
+       exc_exec('call msgpackdump([todump])'))
-      let dumped = msgpackdump([todump])
-      let exception = 0
-    catch
-      let exception = v:exception
-    endtry
-    ]])
-    eq('Vim(let):E475: Invalid argument: container references itself',
-       eval('exception'))
   end)
 
   it('fails to dump a recursive list in a special dict', function()
     execute('let todump = {"_TYPE": v:msgpack_types.array, "_VAL": []}')
     execute('call add(todump._VAL, todump)')
-    execute([[
+    eq('Vim(call):E475: Invalid argument: container references itself',
-    try
+       exc_exec('call msgpackdump([todump])'))
-      let dumped = msgpackdump([todump])
-      let exception = 0
-    catch
-      let exception = v:exception
-    endtry
-    ]])
-    eq('Vim(let):E475: Invalid argument: container references itself',
-       eval('exception'))
   end)
 
   it('fails to dump a recursive (key) map in a special dict', function()
     execute('let todump = {"_TYPE": v:msgpack_types.array, "_VAL": []}')
     execute('call add(todump._VAL, [todump, 0])')
-    execute([[
+    eq('Vim(call):E475: Invalid argument: container references itself',
-    try
+       exc_exec('call msgpackdump([todump])'))
-      let dumped = msgpackdump([todump])
-      let exception = 0
-    catch
-      let exception = v:exception
-    endtry
-    ]])
-    eq('Vim(let):E475: Invalid argument: container references itself',
-       eval('exception'))
   end)
 
   it('fails to dump a recursive (val) map in a special dict', function()
     execute('let todump = {"_TYPE": v:msgpack_types.array, "_VAL": []}')
     execute('call add(todump._VAL, [0, todump])')
-    execute([[
+    eq('Vim(call):E475: Invalid argument: container references itself',
-    try
+       exc_exec('call msgpackdump([todump])'))
-      let dumped = msgpackdump([todump])
-      let exception = 0
-    catch
-      let exception = v:exception
-    endtry
-    ]])
-    eq('Vim(let):E475: Invalid argument: container references itself',
-       eval('exception'))
   end)
 
   it('fails to dump a recursive (val) special list in a special dict',
   function()
     execute('let todump = {"_TYPE": v:msgpack_types.array, "_VAL": []}')
     execute('call add(todump._VAL, [0, todump._VAL])')
-    execute([[
+    eq('Vim(call):E475: Invalid argument: container references itself',
-    try
+       exc_exec('call msgpackdump([todump])'))
-      let dumped = msgpackdump([todump])
-      let exception = 0
-    catch
-      let exception = v:exception
-    endtry
-    ]])
-    eq('Vim(let):E475: Invalid argument: container references itself',
-       eval('exception'))
   end)
 
-  it('msgpackparse(systemlist(...)) does not segfault. #3135', function()
+  it('fails when called with no arguments', function()
-    local cmd = "msgpackparse(systemlist('"
+    eq('Vim(call):E119: Not enough arguments for function: msgpackdump',
-      ..helpers.nvim_prog.." --api-info'))['_TYPE']['_VAL'][0][0]"
+       exc_exec('call msgpackdump()'))
-    local api_info = eval(cmd)
+  end)
-    api_info = eval(cmd) -- do it again (try to force segfault)
+
-    api_info = eval(cmd) -- do it again
+  it('fails when called with two arguments', function()
-    eq('functions', api_info)
+    eq('Vim(call):E118: Too many arguments for function: msgpackdump',
+       exc_exec('call msgpackdump(["", ""], 1)'))
+  end)
+
+  it('fails to dump a string', function()
+    eq('Vim(call):E686: Argument of msgpackdump() must be a List',
+       exc_exec('call msgpackdump("abcdefghijklmnopqrstuvwxyz")'))
+  end)
+
+  it('fails to dump a number', function()
+    eq('Vim(call):E686: Argument of msgpackdump() must be a List',
+       exc_exec('call msgpackdump(127)'))
+  end)
+
+  it('fails to dump a dictionary', function()
+    eq('Vim(call):E686: Argument of msgpackdump() must be a List',
+       exc_exec('call msgpackdump({})'))
+  end)
+
+  it('fails to dump a funcref', function()
+    eq('Vim(call):E686: Argument of msgpackdump() must be a List',
+       exc_exec('call msgpackdump(function("tr"))'))
+  end)
+
+  it('fails to dump a float', function()
+    eq('Vim(call):E686: Argument of msgpackdump() must be a List',
+       exc_exec('call msgpackdump(0.0)'))
   end)
 end)

test/functional/helpers.lua

@@ -324,6 +324,19 @@ local function rmdir(path)
   return ret
 end
 
+local exc_exec = function(cmd)
+  nvim_command(([[
+    try
+      execute "%s"
+    catch
+      let g:__exception = v:exception
+    endtry
+  ]]):format(cmd:gsub('\n', '\\n'):gsub('[\\"]', '\\%0')))
+  local ret = nvim_eval('get(g:, "__exception", 0)')
+  nvim_command('unlet! g:__exception')
+  return ret
+end
+
 return {
   clear = clear,
   spawn = spawn,
@@ -358,5 +371,6 @@ return {
   wait = wait,
   set_session = set_session,
   write_file = write_file,
-  rmdir = rmdir
+  rmdir = rmdir,
+  exc_exec = exc_exec,
 }