From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id A406B25097 for ; Mon, 21 Jan 2019 21:13:04 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id z4yDna77mFRG for ; Mon, 21 Jan 2019 21:13:04 -0500 (EST) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 549AE24E49 for ; Mon, 21 Jan 2019 21:13:04 -0500 (EST) From: Alexander Turenko Subject: [tarantool-patches] [PATCH v2 2/3] lua-yaml: fix boolean/null representation in yaml Date: Tue, 22 Jan 2019 05:12:53 +0300 Message-Id: <15b711cb55d34fff1f010fd6df1aa32248f65735.1548123025.git.alexander.turenko@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Vladislav Shpilevoy Cc: Alexander Turenko , tarantool-patches@freelists.org Encode changes: Wrap the following string literals into single quotes: 'false', 'no', 'true', 'yes', '~', 'null', 'Null', 'NULL'. The content of these strings is equals to null or boolean values representation in YAML. Reverted the a2d7643c commit to use single quotes instead of multiline encoding for 'true' and 'false' string literals. '~', 'null', 'Null', 'NULL', 'no' and 'yes' were encoded w/o quotes before, so '~', 'null', 'no', 'yes' were decoded with a wrong type then. Multiline encoding was used for 'true' and 'false'. Decode changes: Treat 'Null', 'NULL' as null in addition to '~' and 'null'. Note: this commit leaves an empty document/value treatment as an empty string, which is not standard (should be treated as null). This is fixed in the following commit. Follows up #3476 Closes #3662 Closes #3583 --- test/app-tap/console.test.lua | 23 +++++++--- test/box/misc.result | 2 +- third_party/lua-yaml/lyaml.cc | 80 ++++++++++++++++++++++++++--------- 3 files changed, 80 insertions(+), 25 deletions(-) diff --git a/test/app-tap/console.test.lua b/test/app-tap/console.test.lua index 4f915afd7..68f273234 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(68) -- Start console and connect to it local server = console.listen(CONSOLE_SOCKET) @@ -77,11 +77,24 @@ 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) +test:is(yaml.decode('Null'), nil) +test:is(yaml.decode('NULL'), nil) box.cfg{ listen=IPROTO_SOCKET; diff --git a/test/box/misc.result b/test/box/misc.result index c3cabcc8a..94ee11b63 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -55,7 +55,7 @@ t = {} for n in pairs(box) do table.insert(t, tostring(n)) end table.sort(t) ... t --- -- - NULL +- - 'NULL' - atomic - backup - begin diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc index 9b07992d8..73e5d2c31 100644 --- a/third_party/lua-yaml/lyaml.cc +++ b/third_party/lua-yaml/lyaml.cc @@ -94,6 +94,46 @@ struct lua_yaml_dumper { luaL_Buffer yamlbuf; }; +enum yaml_type { + YAML_NO_MATCH = 0, + YAML_FALSE, + YAML_TRUE, + YAML_NULL +}; + +/** + * 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. + */ +static yaml_type +yaml_get_bool(const char *str, const size_t len) +{ + if (len > 5) + return YAML_NO_MATCH; + if (strcmp(str, "false") == 0 || strcmp(str, "no") == 0) + return YAML_FALSE; + if (strcmp(str, "true") == 0 || strcmp(str, "yes") == 0) + return YAML_TRUE; + return YAML_NO_MATCH; +} + +/** + * Verify whether a string represents a null literal in YAML. + * + * Non-standard: don't match an empty string as null. + */ +static yaml_type +yaml_get_null(const char *str, const size_t len){ + if (len == 1 && str[0] == '~') + return YAML_NULL; + if (len == 4 && (strcmp(str, "null") == 0 || strcmp(str, "Null") == 0 || + strcmp(str, "NULL") == 0)) + return YAML_NULL; + return YAML_NO_MATCH; +} + static void generate_error_message(struct lua_yaml_loader *loader) { char buf[256]; luaL_Buffer b; @@ -209,7 +249,7 @@ 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")); + lua_pushboolean(loader->L, yaml_get_bool(str, length) == YAML_TRUE); return; } else if (!strcmp(tag, "binary")) { frombase64(loader->L, (const unsigned char *)str, length); @@ -218,20 +258,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); + yaml_type type; + 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_get_null(str, length) == YAML_NULL) { luaL_pushnull(loader->L); return; - } else if (!length) { - lua_pushliteral(loader->L, ""); + } else if ((type = yaml_get_bool(str, length)) != YAML_NO_MATCH) { + lua_pushboolean(loader->L, type == YAML_TRUE); return; } @@ -548,7 +588,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 +636,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_get_null(str, len) == YAML_NULL) + || (yaml_get_bool(str, len) != YAML_NO_MATCH) + || (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 +650,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