From ba2f615cd40d5d809d1a141c7b098e3bd22ff7bb Mon Sep 17 00:00:00 2001
From: ZyX <kp-pav@yandex.ru>
Date: Sat, 16 Jul 2016 02:26:04 +0300
Subject: functests: Test for error conditions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

During testing found the following bugs:

1. msgpack-gen.lua script is completely unprepared for Float values either in
   return type or in arguments. Specifically:

   1. At the time of writing relevant code FLOAT_OBJ did not exist as well as
      FLOATING_OBJ, but it would be used by msgpack-gen.lua should return type
      be Float. I added FLOATING_OBJ macros later because did not know that
      msgpack-gen.lua uses these _OBJ macros, otherwise it would be FLOAT_OBJ.
   2. msgpack-gen.lua should use .data.floating in place of .data.float. But it
      did not expect that .data subattribute may have name different from
      lowercased type name.

2. vim_replace_termcodes returned its argument as-is if it receives an empty
   string (as well as _vim_id*() functions did). But if something in returned
   argument lives in an allocated memory such action will cause double free:
   once when freeing arguments, then when freeing return value. It did not cause
   problems yet because msgpack bindings return empty string as {NULL, 0} and
   nothing was actually allocated.
3. New code in msgpack-gen.lua popped arguments in reversed order, making lua
   bindings’ signatures be different from API ones.
---
 src/nvim/viml/executor/converter.c | 39 +++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

(limited to 'src/nvim/viml/executor')

diff --git a/src/nvim/viml/executor/converter.c b/src/nvim/viml/executor/converter.c
index a6399500f2..a741d3a752 100644
--- a/src/nvim/viml/executor/converter.c
+++ b/src/nvim/viml/executor/converter.c
@@ -724,16 +724,15 @@ void nlua_push_Object(lua_State *lstate, const Object obj)
 String nlua_pop_String(lua_State *lstate, Error *err)
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
 {
-  String ret;
-
-  ret.data = (char *)lua_tolstring(lstate, -1, &(ret.size));
-
-  if (ret.data == NULL) {
+  if (lua_type(lstate, -1) != LUA_TSTRING) {
     lua_pop(lstate, 1);
-    set_api_error("Expected lua string", err);
+    api_set_error(err, Validation, "Expected lua string");
     return (String) { .size = 0, .data = NULL };
   }
+  String ret;
 
+  ret.data = (char *)lua_tolstring(lstate, -1, &(ret.size));
+  assert(ret.data != NULL);
   ret.data = xmemdupz(ret.data, ret.size);
   lua_pop(lstate, 1);
 
@@ -746,17 +745,19 @@ String nlua_pop_String(lua_State *lstate, Error *err)
 Integer nlua_pop_Integer(lua_State *lstate, Error *err)
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
 {
-  Integer ret = 0;
-
-  if (!lua_isnumber(lstate, -1)) {
+  if (lua_type(lstate, -1) != LUA_TNUMBER) {
     lua_pop(lstate, 1);
-    set_api_error("Expected lua integer", err);
-    return ret;
+    api_set_error(err, Validation, "Expected lua number");
+    return 0;
   }
-  ret = (Integer)lua_tonumber(lstate, -1);
+  const lua_Number n = lua_tonumber(lstate, -1);
   lua_pop(lstate, 1);
-
-  return ret;
+  if (n > (lua_Number)API_INTEGER_MAX || n < (lua_Number)API_INTEGER_MIN
+      || ((lua_Number)((Integer)n)) != n) {
+    api_set_error(err, Exception, "Number is not integral");
+    return 0;
+  }
+  return (Integer)n;
 }
 
 /// Convert lua value to boolean
@@ -765,7 +766,7 @@ Integer nlua_pop_Integer(lua_State *lstate, Error *err)
 Boolean nlua_pop_Boolean(lua_State *lstate, Error *err)
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
 {
-  Boolean ret = lua_toboolean(lstate, -1);
+  const Boolean ret = lua_toboolean(lstate, -1);
   lua_pop(lstate, 1);
   return ret;
 }
@@ -784,7 +785,7 @@ static inline LuaTableProps nlua_check_type(lua_State *const lstate,
 {
   if (lua_type(lstate, -1) != LUA_TTABLE) {
     if (err) {
-      set_api_error("Expected lua table", err);
+      api_set_error(err, Validation, "Expected lua table");
     }
     return (LuaTableProps) { .type = kObjectTypeNil };
   }
@@ -797,7 +798,7 @@ static inline LuaTableProps nlua_check_type(lua_State *const lstate,
 
   if (table_props.type != type) {
     if (err) {
-      set_api_error("Unexpected type", err);
+      api_set_error(err, Validation, "Unexpected type");
     }
   }
 
@@ -1050,7 +1051,7 @@ Object nlua_pop_Object(lua_State *const lstate, Error *const err)
         const lua_Number n = lua_tonumber(lstate, -1);
         if (n > (lua_Number)API_INTEGER_MAX || n < (lua_Number)API_INTEGER_MIN
             || ((lua_Number)((Integer)n)) != n) {
-          *cur.obj = FLOATING_OBJ((Float)n);
+          *cur.obj = FLOAT_OBJ((Float)n);
         } else {
           *cur.obj = INTEGER_OBJ((Integer)n);
         }
@@ -1094,7 +1095,7 @@ Object nlua_pop_Object(lua_State *const lstate, Error *const err)
             break;
           }
           case kObjectTypeFloat: {
-            *cur.obj = FLOATING_OBJ((Float)table_props.val);
+            *cur.obj = FLOAT_OBJ((Float)table_props.val);
             break;
           }
           case kObjectTypeNil: {
-- 
cgit