Re: [Tarantool-patches] [PATCH v1 1/1] sql: limit blob size during CAST AS INTEGER

[Nikita Pettik <korablev@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(
>          -- </cast-5.1>
>      })
>  
> +--
> +-- 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;
> +    ]], {
> +        -- <cast-6.1>
> +        '3', 3, 3, 3, '7     ', 7, 7, 7
> +        -- </cast-6.1>
> +    })
> +
> +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?

> +    ]], {
> +        -- <cast-6.2>
> +        '3', 3, 3, 3
> +        -- </cast-6.2>
> +    })
> +
>  test:finish_test()
> -- 
> 2.7.4
>