<HTML><BODY>After discussion we decided that this patch will serve us<br>part of patch-set that should close newly filed bug:<br>https://github.com/tarantool/tarantool/issues/3832<br><br>The main Idea behind this ticket is that any statement<br>should return column type for when it returns column name.<br>And this ticket allow us to find an error without falling<br>into SEGMENTATION FAULT or returning less metadata.<br><br>Четверг, 22 ноября 2018, 21:31 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:<br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
<br>
<div id="">
<div class="js-helper js-readmsg-msg">
<style type="text/css"></style>
<div>
<base target="_self" href="https://e.mail.ru/">
<div id="style_15429114790000000440_BODY">It was just a question, "why did you decide to do not<br>
send type". It was not necessary to start sending UNKNOWN. I<br>
think, that unknown type should be sent never.<br>
<br>
On 22/11/2018 21:08, Imeev Mergen wrote:<br>
> Hi! Thank you for review! Diff between versions and new patch<br>
> below.<br>
> <br>
> On 11/19/18 8:27 PM, n.pettik wrote:<br>
> <br>
>>> EXPLAIN envokes SEGMENTATION FAULT when being executed through<br>
>> Typo: invokes.<br>
>><br>
>>> net.box. It happens due to column type of the result of this<br>
>>> function being NULL.<br>
>> I’ve checked and not only EXPLAIN results, but also EXPLAIN QUERY PLAN,<br>
>> several pragmas (for instance, pragma table_info()) etc. So, initially problem<br>
>> was not only in EXPLAIN statement. Please exposed commit message and<br>
>> add tests on other cases.<br>
>><br>
>> Also, why did you decide to avoid sending type at all, instead of sending<br>
>> UNKNOWN for example?<br>
> <br>
> *Diff between versions:*<br>
> <br>
> diff --git a/src/box/execute.c b/src/box/execute.c<br>
> index 5b747cf..cd809f8 100644<br>
> --- a/src/box/execute.c<br>
> +++ b/src/box/execute.c<br>
> @@ -529,6 +529,9 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,<br>
> return -1;<br>
> <br>
> for (int i = 0; i < column_count; ++i) {<br>
> + size_t size = mp_sizeof_map(2) +<br>
> + mp_sizeof_uint(IPROTO_FIELD_NAME) +<br>
> + mp_sizeof_uint(IPROTO_FIELD_TYPE);<br>
> const char *name = sqlite3_column_name(stmt, i);<br>
> const char *type = sqlite3_column_datatype(stmt, i);<br>
> /*<br>
> @@ -537,27 +540,20 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,<br>
> * column_name simply returns them.<br>
> */<br>
> assert(name != NULL);<br>
> - size_t size = mp_sizeof_uint(IPROTO_FIELD_NAME) +<br>
> - mp_sizeof_str(strlen(name));<br>
> - if (type != NULL) {<br>
> - size += mp_sizeof_map(2) +<br>
> - mp_sizeof_uint(IPROTO_FIELD_TYPE) +<br>
> - mp_sizeof_str(strlen(type));<br>
> - } else {<br>
> - size += mp_sizeof_map(1);<br>
> - }<br>
> + if (type == NULL)<br>
> + type = "UNKNOWN";<br>
> + size += mp_sizeof_str(strlen(name));<br>
> + size += mp_sizeof_str(strlen(type));<br>
> char *pos = (char *) obuf_alloc(out, size);<br>
> if (pos == NULL) {<br>
> diag_set(OutOfMemory, size, "obuf_alloc", "pos");<br>
> return -1;<br>
> }<br>
> - pos = mp_encode_map(pos, 1 + (type != NULL));<br>
> + pos = mp_encode_map(pos, 2);<br>
> pos = mp_encode_uint(pos, IPROTO_FIELD_NAME);<br>
> pos = mp_encode_str(pos, name, strlen(name));<br>
> - if (type != NULL) {<br>
> - pos = mp_encode_uint(pos, IPROTO_FIELD_TYPE);<br>
> - pos = mp_encode_str(pos, type, strlen(type));<br>
> - }<br>
> + pos = mp_encode_uint(pos, IPROTO_FIELD_TYPE);<br>
> + pos = mp_encode_str(pos, type, strlen(type));<br>
> }<br>
> return 0;<br>
> }<br>
> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c<br>
> index 342c6a7..c7063d9 100644<br>
> --- a/src/box/lua/net_box.c<br>
> +++ b/src/box/lua/net_box.c<br>
> @@ -644,7 +644,8 @@ netbox_decode_metadata(struct lua_State *L, const char **data)<br>
> lua_createtable(L, count, 0);<br>
> for (uint32_t i = 0; i < count; ++i) {<br>
> uint32_t map_size = mp_decode_map(data);<br>
> - assert(map_size == 1 || map_size == 2);<br>
> + assert(map_size == 2);<br>
> + (void) map_size;<br>
> uint32_t key = mp_decode_uint(data);<br>
> assert(key == IPROTO_FIELD_NAME);<br>
> (void) key;<br>
> @@ -653,13 +654,11 @@ netbox_decode_metadata(struct lua_State *L, const char **data)<br>
> const char *str = mp_decode_str(data, &len);<br>
> lua_pushlstring(L, str, len);<br>
> lua_setfield(L, -2, "name");<br>
> - if (map_size == 2) {<br>
> - key = mp_decode_uint(data);<br>
> - assert(key == IPROTO_FIELD_TYPE);<br>
> - const char *type = mp_decode_str(data, &len);<br>
> - lua_pushlstring(L, type, len);<br>
> - lua_setfield(L, -2, "type");<br>
> - }<br>
> + key = mp_decode_uint(data);<br>
> + assert(key == IPROTO_FIELD_TYPE);<br>
> + const char *type = mp_decode_str(data, &len);<br>
> + lua_pushlstring(L, type, len);<br>
> + lua_setfield(L, -2, "type");<br>
> lua_rawseti(L, -2, i + 1);<br>
> }<br>
> }<br>
> diff --git a/test/sql/iproto.result b/test/sql/iproto.result<br>
> index 31e5d2e..f229427 100644<br>
> --- a/test/sql/iproto.result<br>
> +++ b/test/sql/iproto.result<br>
> @@ -774,9 +774,9 @@ cn:close()<br>
> ---<br>
> ...<br>
> --<br>
> --- EXPLAIN could lead to segfault because it does not send column<br>
> --- data type, but looks like DQL. Netbox had been trying to decode<br>
> --- type for any DQL.<br>
> +-- Some commands, e.g. EXPLAIN, could lead to segfault because it<br>
> +-- does not send column data type, but looks like DQL. Netbox had<br>
> +-- been trying to decode type for any DQL.<br>
> --<br>
> cn = remote.connect(box.cfg.listen)<br>
> ---<br>
> @@ -785,6 +785,32 @@ cn = remote.connect(box.cfg.listen)<br>
> _ = cn:execute("EXPLAIN SELECT 1;")<br>
> ---<br>
> ...<br>
> +-- Segmentation fault when PRAGMA TABLE_INFO() executed using<br>
> +-- net.box.<br>
> +box.sql.execute('CREATE TABLE test (id INT PRIMARY KEY)')<br>
> +---<br>
> +...<br>
> +cn:execute("PRAGMA TABLE_INFO(test);")<br>
> +---<br>
> +- metadata:<br>
> + - name: cid<br>
> + type: UNKNOWN<br>
> + - name: name<br>
> + type: UNKNOWN<br>
> + - name: type<br>
> + type: UNKNOWN<br>
> + - name: notnull<br>
> + type: UNKNOWN<br>
> + - name: dflt_value<br>
> + type: UNKNOWN<br>
> + - name: pk<br>
> + type: UNKNOWN<br>
> + rows:<br>
> + - [0, 'ID', 'integer', 1, null, 1]<br>
> +...<br>
> +box.sql.execute('DROP TABLE test')<br>
> +---<br>
> +...<br>
> cn:close()<br>
> ---<br>
> ...<br>
> diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua<br>
> index a0f0f15..9bcc7ef 100644<br>
> --- a/test/sql/iproto.test.lua<br>
> +++ b/test/sql/iproto.test.lua<br>
> @@ -248,15 +248,21 @@ cn:execute('drop table test')<br>
> cn:close()<br>
> <br>
> --<br>
> --- EXPLAIN could lead to segfault because it does not send column<br>
> --- data type, but looks like DQL. Netbox had been trying to decode<br>
> --- type for any DQL.<br>
> +-- Some commands, e.g. EXPLAIN, could lead to segfault because it<br>
> +-- does not send column data type, but looks like DQL. Netbox had<br>
> +-- been trying to decode type for any DQL.<br>
> --<br>
> cn = remote.connect(box.cfg.listen)<br>
> <br>
> -- Segmentation fault when EXPLAIN executed using net.box.<br>
> _ = cn:execute("EXPLAIN SELECT 1;")<br>
> <br>
> +-- Segmentation fault when PRAGMA TABLE_INFO() executed using<br>
> +-- net.box.<br>
> +box.sql.execute('CREATE TABLE test (id INT PRIMARY KEY)')<br>
> +cn:execute("PRAGMA TABLE_INFO(test);")<br>
> +box.sql.execute('DROP TABLE test')<br>
> +<br>
> cn:close()<br>
> <br>
> box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
> <br>
> <br>
> *New version:*<br>
> <br>
> commit 29dde0365922bfcec0d159b36c6454854adf34f7<br>
> Author: Mergen Imeev <<a href="mailto:imeevma@gmail.com">imeevma@gmail.com</a>><br>
> Date: Fri Nov 16 13:34:46 2018 +0300<br>
> <br>
> sql: Print column type even if it is NULL<br>
> <br>
> Some commands, e.g. EXPLAIN, invokes SEGMENTATION FAULT when being<br>
> executed through net.box. It happens due to column type of the<br>
> result of this function being NULL. This patch adds checks for<br>
> received column type.<br>
> <br>
> diff --git a/src/box/execute.c b/src/box/execute.c<br>
> index e450444..cd809f8 100644<br>
> --- a/src/box/execute.c<br>
> +++ b/src/box/execute.c<br>
> @@ -540,6 +540,8 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,<br>
> * column_name simply returns them.<br>
> */<br>
> assert(name != NULL);<br>
> + if (type == NULL)<br>
> + type = "UNKNOWN";<br>
> size += mp_sizeof_str(strlen(name));<br>
> size += mp_sizeof_str(strlen(type));<br>
> char *pos = (char *) obuf_alloc(out, size);<br>
> diff --git a/test/sql/iproto.result b/test/sql/iproto.result<br>
> index d077ee8..f229427 100644<br>
> --- a/test/sql/iproto.result<br>
> +++ b/test/sql/iproto.result<br>
> @@ -773,6 +773,47 @@ cn:execute('drop table test')<br>
> cn:close()<br>
> ---<br>
> ...<br>
> +--<br>
> +-- Some commands, e.g. EXPLAIN, could lead to segfault because it<br>
> +-- does not send column data type, but looks like DQL. Netbox had<br>
> +-- been trying to decode type for any DQL.<br>
> +--<br>
> +cn = remote.connect(box.cfg.listen)<br>
> +---<br>
> +...<br>
> +-- Segmentation fault when EXPLAIN executed using net.box.<br>
> +_ = cn:execute("EXPLAIN SELECT 1;")<br>
> +---<br>
> +...<br>
> +-- Segmentation fault when PRAGMA TABLE_INFO() executed using<br>
> +-- net.box.<br>
> +box.sql.execute('CREATE TABLE test (id INT PRIMARY KEY)')<br>
> +---<br>
> +...<br>
> +cn:execute("PRAGMA TABLE_INFO(test);")<br>
> +---<br>
> +- metadata:<br>
> + - name: cid<br>
> + type: UNKNOWN<br>
> + - name: name<br>
> + type: UNKNOWN<br>
> + - name: type<br>
> + type: UNKNOWN<br>
> + - name: notnull<br>
> + type: UNKNOWN<br>
> + - name: dflt_value<br>
> + type: UNKNOWN<br>
> + - name: pk<br>
> + type: UNKNOWN<br>
> + rows:<br>
> + - [0, 'ID', 'integer', 1, null, 1]<br>
> +...<br>
> +box.sql.execute('DROP TABLE test')<br>
> +---<br>
> +...<br>
> +cn:close()<br>
> +---<br>
> +...<br>
> box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
> ---<br>
> ...<br>
> diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua<br>
> index 1470eda..9bcc7ef 100644<br>
> --- a/test/sql/iproto.test.lua<br>
> +++ b/test/sql/iproto.test.lua<br>
> @@ -247,10 +247,27 @@ cn:execute('drop table test')<br>
> <br>
> cn:close()<br>
> <br>
> +--<br>
> +-- Some commands, e.g. EXPLAIN, could lead to segfault because it<br>
> +-- does not send column data type, but looks like DQL. Netbox had<br>
> +-- been trying to decode type for any DQL.<br>
> +--<br>
> +cn = remote.connect(box.cfg.listen)<br>
> +<br>
> +-- Segmentation fault when EXPLAIN executed using net.box.<br>
> +_ = cn:execute("EXPLAIN SELECT 1;")<br>
> +<br>
> +-- Segmentation fault when PRAGMA TABLE_INFO() executed using<br>
> +-- net.box.<br>
> +box.sql.execute('CREATE TABLE test (id INT PRIMARY KEY)')<br>
> +cn:execute("PRAGMA TABLE_INFO(test);")<br>
> +box.sql.execute('DROP TABLE test')<br>
> +<br>
> +cn:close()<br>
> +<br>
> box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
> box.schema.user.revoke('guest', 'create', 'space')<br>
> space = nil<br>
> <br>
> -- Cleanup xlog<br>
> box.snapshot()<br>
> -<br>
> <br>
</div>
<base target="_self" href="https://e.mail.ru/">
</div>
</div>
</div>
</blockquote>
<br>
<br>-- <br/>Мерген Имеев<br></BODY></HTML>