* [tarantool-patches] [PATCH v3 1/2] lua-yaml: verify arguments count
2019-02-05 3:30 [tarantool-patches] [PATCH v3 0/2] lua-yaml null/boolean fixes Alexander Turenko
@ 2019-02-05 3:30 ` Alexander Turenko
2019-02-05 3:30 ` [tarantool-patches] [PATCH v3 2/2] lua-yaml: fix strings literals encoding in yaml Alexander Turenko
1 sibling, 0 replies; 3+ messages in thread
From: Alexander Turenko @ 2019-02-05 3:30 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: AKhatskevich, tarantool-patches
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [tarantool-patches] [PATCH v3 2/2] lua-yaml: fix strings literals encoding in yaml
2019-02-05 3:30 [tarantool-patches] [PATCH v3 0/2] lua-yaml null/boolean fixes Alexander Turenko
2019-02-05 3:30 ` [tarantool-patches] [PATCH v3 1/2] lua-yaml: verify arguments count Alexander Turenko
@ 2019-02-05 3:30 ` Alexander Turenko
1 sibling, 0 replies; 3+ messages in thread
From: Alexander Turenko @ 2019-02-05 3:30 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: Alexander Turenko, tarantool-patches
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
^ permalink raw reply [flat|nested] 3+ messages in thread