From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A60E8469719 for ; Tue, 11 Feb 2020 17:50:59 +0300 (MSK) Date: Tue, 11 Feb 2020 17:50:58 +0300 From: Nikita Pettik Message-ID: <20200211145058.GD5168@tarantool.org> References: <4e5891d426667d49189c3a96fdbce5d315b604c2.1581414644.git.imeevma@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <4e5891d426667d49189c3a96fdbce5d315b604c2.1581414644.git.imeevma@gmail.com> Subject: Re: [Tarantool-patches] [PATCH v1 1/1] sql: limit blob size during CAST AS INTEGER List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: imeevma@tarantool.org Cc: tarantool-patches@dev.tarantool.org On 11 Feb 12:52, imeevma@tarantool.org wrote: > This patch limits the number of decoded bytes during CAST from > BLOB to INTEGER according to the specified BLOB size. > > Closes #4766 Note that issue has no assigned milestone. As far as I remember it is not recommended to work on such issues (personally I do not care, but I guess Kirill does). What is more, according to our new policy, each patch should come with change-log: see 'Support ChangeLogs' chapter in SOP. > > diff --git a/src/box/sql/util.c b/src/box/sql/util.c > index f908e9c..8f70ce6 100644 > --- a/src/box/sql/util.c > +++ b/src/box/sql/util.c > @@ -467,14 +467,21 @@ sql_atoi64(const char *z, int64_t *val, bool *is_neg, int length) > if (*z == '-') > *is_neg = true; > > + /* > + * BLOB data may not end with '\0'. This leads to an error > + * in the strtoll() and strtoull() functions. Nit: does strtoll() raise an error, or return wrong result? > To fix this, > + * let's copy the value for decoding into static memory > + * and add '\0' to it. > + */ > + const char *str_value = tt_cstr(z, length); > char *end = NULL; > errno = 0; > - if (*z == '-') { > + if (*str_value == '-') { > *is_neg = true; > - *val = strtoll(z, &end, 10); > + *val = strtoll(str_value, &end, 10); > } else { > *is_neg = false; > - uint64_t u_val = strtoull(z, &end, 10); > + uint64_t u_val = strtoull(str_value, &end, 10); > *val = u_val; > } > /* Overflow and underflow errors. */ > diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua > index 9c937a0..a579f24 100755 > --- a/test/sql-tap/cast.test.lua > +++ b/test/sql-tap/cast.test.lua > @@ -1,6 +1,6 @@ > #!/usr/bin/env tarantool > test = require("sqltester") > -test:plan(85) > +test:plan(87) > > --!./tcltestrunner.lua > -- 2005 June 25 > @@ -905,4 +905,34 @@ test:do_execsql_test( > -- > }) > > +-- > +-- gh-4766: Make sure that a blob as part of a tuple can be cast > +-- to NUMBER, INTEGER and UNSIGNED. I'd outline the fact that bug appears due to blob may not be terminated with \0 while stored in space. So make sure that function providing conversion from blob to numbers does account blob's lenght. > +-- > +test:do_execsql_test( > + "cast-6.1", > + [[ > + CREATE TABLE t (a VARBINARY PRIMARY KEY); > + INSERT INTO t VALUES (X'33'), (X'372020202020'); > + SELECT a, CAST(a AS NUMBER), CAST(a AS INTEGER), CAST(a AS UNSIGNED) FROM t; > + DROP TABLE t; > + ]], { > + -- > + '3', 3, 3, 3, '7 ', 7, 7, 7 > + -- > + }) > + > +test:do_execsql_test( > + "cast-6.2", > + [[ > + CREATE TABLE t (a VARBINARY PRIMARY KEY, i INT); > + INSERT INTO t VALUES (X'33', 1); > + SELECT a, CAST(a AS NUMBER), CAST(a AS INTEGER), CAST(a AS UNSIGNED) FROM t; > + DROP TABLE t; What's the (in functional sense) difference between 6.1 and 6.2? > + ]], { > + -- > + '3', 3, 3, 3 > + -- > + }) > + > test:finish_test() > -- > 2.7.4 >