[tarantool-patches] Re: [PATCH] sql: fix bug with BLOB TRIM() when X'00' in char set

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Nikita, please, do first review.

On 15/12/2018 13:57, Roman Khabibov wrote:
> The reason for the bug was that X'00' is a terminal symbol. If the char set
> contained X'00', all other characters after its (inculding itself) are then
> ignored.
> 
> Closes #3543
> 
> Branch: https://github.com/tarantool/tarantool/tree/romankhabibov/gh-3543-trim-terminal
> Issue: https://github.com/tarantool/tarantool/issues/3543
> ---
>   src/box/sql/func.c         |  18 +++++-
>   test/sql-tap/func.test.lua | 126 ++++++++++++++++++++++++++++++++++++-
>   2 files changed, 140 insertions(+), 4 deletions(-)
> 
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index 9667aead5..5beba7bd2 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -1224,8 +1224,16 @@ trimFunc(sqlite3_context * context, int argc, sqlite3_value ** argv)
>   		return;
>   	} else {
>   		const unsigned char *z;
> -		for (z = zCharSet, nChar = 0; *z; nChar++) {
> +		int sizeInChar = sqlite3_value_bytes(argv[1]);
> +		int length = 0;
> +		z = zCharSet;
> +		nChar = 0;
> +		const unsigned char *zStepBack;
> +		while(sizeInChar - length) {
> +			zStepBack = z;
>   			SQLITE_SKIP_UTF8(z);
> +			length += z - zStepBack;
> +			nChar++;
>   		}
>   		if (nChar > 0) {
>   			azChar =
> @@ -1235,10 +1243,16 @@ trimFunc(sqlite3_context * context, int argc, sqlite3_value ** argv)
>   				return;
>   			}
>   			aLen = (unsigned char *)&azChar[nChar];
> -			for (z = zCharSet, nChar = 0; *z; nChar++) {
> +			z = zCharSet;
> +			nChar = 0;
> +			length = 0;
> +			while(sizeInChar - length) {
>   				azChar[nChar] = (unsigned char *)z;
> +				zStepBack = z;
>   				SQLITE_SKIP_UTF8(z);
> +				length += z - zStepBack;
>   				aLen[nChar] = (u8) (z - azChar[nChar]);
> +				nChar++;
>   			}
>   		}
>   	}
> diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
> index 393212968..b7b8e7c4c 100755
> --- a/test/sql-tap/func.test.lua
> +++ b/test/sql-tap/func.test.lua
> @@ -1,6 +1,6 @@
>   #!/usr/bin/env tarantool
>   test = require("sqltester")
> -test:plan(14535)
> +test:plan(14547)
>   
>   --!./tcltestrunner.lua
>   -- 2001 September 15
> @@ -2100,11 +2100,133 @@ test:do_execsql_test(
>           -- </func-22.22>
>       })
>   
> +-- gh-3543 Check trimming of binary string when X'00' in trimming char set.
> +
> +test:do_execsql_test(
> +    "func-22.23",
> +    [[
> +        SELECT TRIM(X'004100', X'00');
> +    ]], {
> +        -- <func-22.23>
> +        "A"
> +        -- </func-22.23>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.24",
> +    [[
> +        SELECT TRIM(X'004100', X'0000');
> +    ]], {
> +        -- <func-22.24>
> +        "A"
> +        -- </func-22.24>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.25",
> +    [[
> +        SELECT TRIM(X'004100', X'0042');
> +    ]], {
> +        -- <func-22.25>
> +        "A"
> +        -- </func-22.25>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.26",
> +    [[
> +        SELECT TRIM(X'00004100420000', X'00');
> +    ]], {
> +        -- <func-22.26>
> +        "A\0B"
> +        -- </func-22.26>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.27",
> +    [[
> +        SELECT LTRIM(X'004100', X'00');
> +    ]], {
> +        -- <func-22.27>
> +        "A\0"
> +        -- </func-22.27>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.28",
> +    [[
> +        SELECT LTRIM(X'004100', X'0000');
> +    ]], {
> +        -- <func-22.28>
> +        "A\0"
> +        -- </func-22.28>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.29",
> +    [[
> +        SELECT LTRIM(X'004100', X'0042');
> +    ]], {
> +        -- <func-22.29>
> +        "A\0"
> +        -- </func-22.29>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.30",
> +    [[
> +        SELECT LTRIM(X'00004100420000', X'00');
> +    ]], {
> +        -- <func-22.30>
> +        "A\0B\0\0"
> +        -- </func-22.30>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.31",
> +    [[
> +        SELECT RTRIM(X'004100', X'00');
> +    ]], {
> +        -- <func-22.31>
> +        "\0A"
> +        -- </func-22.31>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.32",
> +    [[
> +        SELECT RTRIM(X'004100', X'0000');
> +    ]], {
> +        -- <func-22.32>
> +        "\0A"
> +        -- </func-22.32>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.33",
> +    [[
> +        SELECT RTRIM(X'004100', X'0042');
> +    ]], {
> +        -- <func-22.33>
> +        "\0A"
> +        -- </func-22.33>
> +    })
> +
> +test:do_execsql_test(
> +    "func-22.34",
> +    [[
> +        SELECT RTRIM(X'00004100420000', X'00');
> +    ]], {
> +        -- <func-22.34>
> +        "\0\0A\0B"
> +        -- </func-22.34>
> +    })
> +
>   -- This is to test the deprecated sqlite3_aggregate_count() API.
>   --
>   --test:do_test(
>   --    "func-23.1",
> ---    function()
> +--    function()S
>   --        sqlite3_create_aggregate("db")
>   --        return test:execsql([[
>   --            SELECT legacy_count() FROM t6;
>