[tarantool-patches] [PATCH v3 0/2] lua-yaml null/boolean fixes

[tarantool-patches] [PATCH v3 0/2] lua-yaml null/boolean fixes

[Alexander Turenko]

The patchset improves validation of arguments in yaml.encode() and
yaml.decode() functions and fixes encoding of string values like 'null',
which should be quoted.

The main change from the previous version is dropping the last patch,
which was about encoding an empty document as null to better fit the
YAML standard and corresponding decoding changes, but breaks backward
compatibility in some sense. Also I have added API test cases for the
first patch and have fixed Vlad's comments about the code.

AKhatskevich (1):
  lua-yaml: verify arguments count

Alexander Turenko (1):
  lua-yaml: fix strings literals encoding in yaml

 test/app-tap/console.test.lua | 21 ++++++--
 test/app-tap/yaml.test.lua    | 74 ++++++++++++++++++++++++++-
 third_party/lua-yaml/lyaml.cc | 95 +++++++++++++++++++++++++++--------
 3 files changed, 162 insertions(+), 28 deletions(-)

-- 
2.20.1

[tarantool-patches] [PATCH v3 1/2] lua-yaml: verify arguments count

[Alexander Turenko]

From: AKhatskevich <avkhatskevich@tarantool.org>

Added arguments count check for yaml.encode() and yaml.decode()
functions.

Without these checks the functions could read garbage outside of a Lua
stack when called w/o arguments.
---
 test/app-tap/yaml.test.lua    | 74 ++++++++++++++++++++++++++++++++++-
 third_party/lua-yaml/lyaml.cc |  7 ++--
 2 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/test/app-tap/yaml.test.lua b/test/app-tap/yaml.test.lua
index 4784bb27d..b9bb10720 100755
--- a/test/app-tap/yaml.test.lua
+++ b/test/app-tap/yaml.test.lua
@@ -130,9 +130,80 @@ local function test_tagged(test, s)
             "tag prefix on multiple documents")
 end
 
+local function test_api(test, s)
+    local encode_usage = 'Usage: encode(object, {tag_prefix = <string>, ' ..
+        'tag_handle = <string>})'
+    local decode_usage = 'Usage: yaml.decode(document, [{tag_only = boolean}])'
+
+    local cases = {
+        {
+            'encode: no args',
+            func = s.encode,
+            args = {},
+            exp_err = encode_usage,
+        },
+        {
+            'encode: wrong opts',
+            func = s.encode,
+            args = {{}, 1},
+            exp_err = encode_usage,
+        },
+        {
+            'encode: wrong tag_prefix',
+            func = s.encode,
+            args = {{}, {tag_prefix = 1}},
+            exp_err = encode_usage,
+        },
+        {
+            'encode: wrong tag_handle',
+            func = s.encode,
+            args = {{}, {tag_handle = 1}},
+            exp_err = encode_usage,
+        },
+        {
+            'encode: extra args',
+            func = s.encode,
+            args = {{}, {}, {}},
+            exp_err = encode_usage,
+        },
+        {
+            'decode: no args',
+            func = s.decode,
+            args = {},
+            exp_err = decode_usage,
+        },
+        {
+            'decode: wrong input',
+            func = s.decode,
+            args = {true},
+            exp_err = decode_usage,
+        },
+        {
+            'decode: wrong opts',
+            func = s.decode,
+            args = {'', 1},
+            exp_err = decode_usage,
+        },
+        {
+            'decode: extra args',
+            func = s.decode,
+            args = {'', nil, {}},
+            exp_err = decode_usage,
+        },
+    }
+
+    test:plan(#cases)
+
+    for _, case in ipairs(cases) do
+        local args_len = table.maxn(case.args)
+        local ok, err = pcall(case.func, unpack(case.args, 1, args_len))
+        test:is_deeply({ok, err}, {false, case.exp_err}, case[1])
+    end
+end
+
 tap.test("yaml", function(test)
     local serializer = require('yaml')
-    test:plan(11)
+    test:plan(12)
     test:test("unsigned", common.test_unsigned, serializer)
     test:test("signed", common.test_signed, serializer)
     test:test("double", common.test_double, serializer)
@@ -144,4 +215,5 @@ tap.test("yaml", function(test)
     test:test("compact", test_compact, serializer)
     test:test("output", test_output, serializer)
     test:test("tagged", test_tagged, serializer)
+    test:test("api", test_api, serializer)
 end)
diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc
index c6d118a79..354cafe86 100644
--- a/third_party/lua-yaml/lyaml.cc
+++ b/third_party/lua-yaml/lyaml.cc
@@ -400,7 +400,8 @@ static void load(struct lua_yaml_loader *loader) {
  */
 static int l_load(lua_State *L) {
    struct lua_yaml_loader loader;
-   if (! lua_isstring(L, 1)) {
+   int top = lua_gettop(L);
+   if (!(top == 1 || top == 2) || !lua_isstring(L, 1)) {
 usage_error:
       return luaL_error(L, "Usage: yaml.decode(document, "\
                         "[{tag_only = boolean}])");
@@ -416,7 +417,7 @@ usage_error:
       return luaL_error(L, OOM_ERRMSG);
    yaml_parser_set_input_string(&loader.parser, (yaml_char_t *) document, len);
    bool tag_only;
-   if (lua_gettop(L) > 1) {
+   if (lua_gettop(L) == 2 && ! lua_isnil(L, 2)) {
       if (! lua_istable(L, 2))
          goto usage_error;
       lua_getfield(L, 2, "tag_only");
@@ -794,7 +795,7 @@ error:
 static int l_dump(lua_State *L) {
    struct luaL_serializer *serializer = luaL_checkserializer(L);
    int top = lua_gettop(L);
-   if (top > 2) {
+   if (!(top == 1 || top == 2)) {
 usage_error:
       return luaL_error(L, "Usage: encode(object, {tag_prefix = <string>, "\
                         "tag_handle = <string>})");
-- 
2.20.1

[tarantool-patches] [PATCH v3 2/2] lua-yaml: fix strings literals encoding in yaml

[Alexander Turenko]

yaml.encode() now wraps a string literal whose content is equal to a
null or a boolean value representation in YAML into single quotes. Those
literals are: 'false', 'no', 'true', 'yes', '~', 'null'.

Reverted the a2d7643c commit to use single quotes instead of multiline
encoding for 'true' and 'false' string literals.

Follows up #3476
Closes #3662
Closes #3583
---
 test/app-tap/console.test.lua | 21 +++++++--
 third_party/lua-yaml/lyaml.cc | 88 +++++++++++++++++++++++++++--------
 2 files changed, 85 insertions(+), 24 deletions(-)

diff --git a/test/app-tap/console.test.lua b/test/app-tap/console.test.lua
index 4f915afd7..0586dd1b1 100755
--- a/test/app-tap/console.test.lua
+++ b/test/app-tap/console.test.lua
@@ -21,7 +21,7 @@ local EOL = "\n...\n"
 
 test = tap.test("console")
 
-test:plan(59)
+test:plan(66)
 
 -- Start console and connect to it
 local server = console.listen(CONSOLE_SOCKET)
@@ -77,11 +77,22 @@ test:is(yaml.decode(client:read(EOL)), '', "clear delimiter")
 
 --
 -- gh-3476: yaml.encode encodes 'false' and 'true' incorrectly.
+-- gh-3662: yaml.encode encodes booleans with multiline format.
+-- gh-3583: yaml.encode encodes null incorrectly.
 --
-test:is(type(yaml.decode(yaml.encode('false'))), 'string')
-test:is(type(yaml.decode(yaml.encode('true'))), 'string')
-test:is(type(yaml.decode(yaml.encode({a = 'false'})).a), 'string')
-test:is(type(yaml.decode(yaml.encode({a = 'false'})).a), 'string')
+
+test:is(yaml.encode(false), "--- false\n...\n")
+test:is(yaml.encode(true), "--- true\n...\n")
+test:is(yaml.encode('false'), "--- 'false'\n...\n")
+test:is(yaml.encode('true'), "--- 'true'\n...\n")
+test:is(yaml.encode(nil), "--- null\n...\n")
+
+test:is(yaml.decode('false'), false)
+test:is(yaml.decode('no'), false)
+test:is(yaml.decode('true'), true)
+test:is(yaml.decode('yes'), true)
+test:is(yaml.decode('~'), nil)
+test:is(yaml.decode('null'), nil)
 
 box.cfg{
     listen=IPROTO_SOCKET;
diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc
index 354cafe86..5afcace1b 100644
--- a/third_party/lua-yaml/lyaml.cc
+++ b/third_party/lua-yaml/lyaml.cc
@@ -94,6 +94,50 @@ struct lua_yaml_dumper {
    luaL_Buffer yamlbuf;
 };
 
+/**
+ * Verify whether a string represents a boolean literal in YAML.
+ *
+ * Non-standard: only subset of YAML 1.1 boolean literals are
+ * treated as boolean values.
+ *
+ * Return 0 at success, -1 at error.
+ */
+static inline int
+yaml_parse_bool(const char *str, size_t len, bool *out)
+{
+   if ((len == 5 && memcmp(str, "false", 5) == 0) ||
+       (len == 2 && memcmp(str, "no", 2) == 0)) {
+      if (out != NULL)
+         *out = false;
+      return 0;
+   }
+   if ((len == 4 && memcmp(str, "true", 4) == 0) ||
+       (len == 3 && memcmp(str, "yes", 3) == 0)) {
+      if (out != NULL)
+         *out = true;
+      return 0;
+   }
+   return -1;
+}
+
+/**
+ * Verify whether a string represents a null literal in YAML.
+ *
+ * Non-standard: don't match an empty string, 'Null' and 'NULL' as
+ * null.
+ *
+ * Return 0 at success, -1 at error.
+ */
+static inline int
+yaml_parse_null(const char *str, size_t len)
+{
+   if (len == 1 && str[0] == '~')
+      return 0;
+   if (len == 4 && memcmp(str, "null", 4) == 0)
+      return 0;
+   return -1;
+}
+
 static void generate_error_message(struct lua_yaml_loader *loader) {
    char buf[256];
    luaL_Buffer b;
@@ -209,7 +253,11 @@ static void load_scalar(struct lua_yaml_loader *loader) {
          lua_pushnumber(loader->L, dval);
          return;
       } else if (!strcmp(tag, "bool")) {
-         lua_pushboolean(loader->L, !strcmp(str, "true") || !strcmp(str, "yes"));
+         bool value = false;
+         int rc = yaml_parse_bool(str, length, &value);
+         (void) rc;
+         assert(rc == 0);
+         lua_pushboolean(loader->L, value);
          return;
       } else if (!strcmp(tag, "binary")) {
          frombase64(loader->L, (const unsigned char *)str, length);
@@ -218,20 +266,20 @@ static void load_scalar(struct lua_yaml_loader *loader) {
    }
 
    if (loader->event.data.scalar.style == YAML_PLAIN_SCALAR_STYLE) {
-      if (!strcmp(str, "~")) {
-         luaL_pushnull(loader->L);
-         return;
-      } else if (!strcmp(str, "true") || !strcmp(str, "yes")) {
-         lua_pushboolean(loader->L, 1);
-         return;
-      } else if (!strcmp(str, "false") || !strcmp(str, "no")) {
-         lua_pushboolean(loader->L, 0);
+      bool value;
+      if (!length) {
+         /*
+          * Non-standard: an empty value/document is null
+          * according to the standard, but we decode it as an
+          * empty string.
+          */
+         lua_pushliteral(loader->L, "");
          return;
-      } else if (!strcmp(str, "null")) {
+      } else if (yaml_parse_null(str, length) == 0) {
          luaL_pushnull(loader->L);
          return;
-      } else if (!length) {
-         lua_pushliteral(loader->L, "");
+      } else if (yaml_parse_bool(str, length, &value) == 0) {
+         lua_pushboolean(loader->L, value);
          return;
       }
 
@@ -548,7 +596,6 @@ static int yaml_is_flow_mode(struct lua_yaml_dumper *dumper) {
                (evp->type == YAML_MAPPING_START_EVENT &&
                 evp->data.mapping_start.style == YAML_FLOW_MAPPING_STYLE)) {
             return 1;
-            break;
          }
       }
    }
@@ -597,8 +644,13 @@ static int dump_node(struct lua_yaml_dumper *dumper)
       return dump_table(dumper, &field);
    case MP_STR:
       str = lua_tolstring(dumper->L, -1, &len);
-      if (lua_isnumber(dumper->L, -1)) {
-         /* string is convertible to number, quote it to preserve type */
+      if ((yaml_parse_null(str, len) == 0)
+         || (yaml_parse_bool(str, len, NULL) == 0)
+         || (lua_isnumber(dumper->L, -1))) {
+         /*
+          * The string is convertible to a null, a boolean or
+          * a number, quote it to preserve its type.
+          */
          style = YAML_SINGLE_QUOTED_SCALAR_STYLE;
          break;
       }
@@ -606,12 +658,10 @@ static int dump_node(struct lua_yaml_dumper *dumper)
       if (utf8_check_printable(str, len)) {
          if (yaml_is_flow_mode(dumper)) {
             style = YAML_SINGLE_QUOTED_SCALAR_STYLE;
-         } else if (strstr(str, "\n\n") != NULL || strcmp(str, "true") == 0 ||
-		    strcmp(str, "false") == 0) {
+         } else if (strstr(str, "\n\n") != 0) {
             /*
              * Tarantool-specific: use literal style for string
-             * with empty lines and strings representing boolean
-             * types.
+             * with empty lines.
              * Useful for tutorial().
              */
             style = YAML_LITERAL_SCALAR_STYLE;
-- 
2.20.1