From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id DFFA66F3FF; Tue, 31 Aug 2021 22:33:09 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DFFA66F3FF DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1630438390; bh=DzvP8E7TEEKPKuWo8F0l5mGZznRPb3sHai4qXdSf0fs=; h=To:Cc:References:In-Reply-To:Date:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=bfohgMLc8t3ve3sfNw01Dq1vX55VEPSYEUW/4T90Ut5OanFLrl3ngbFdASbkC77jS PC+B3rKvLYPkM1XSFqMaP+KiEcZQUwSJBh3m+lRPNKkbpeg17/Bdc+24xM/T7dVOoC uIUupB+n1vSIMUkZmTAtBj557RYFBfdg9RUx3Rh4= Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [217.69.128.38]) (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 344826F3FE for ; Tue, 31 Aug 2021 22:33:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 344826F3FE Received: by smtp58.i.mail.ru with esmtpa (envelope-from ) id 1mL9VL-0005XN-BI; Tue, 31 Aug 2021 22:33:07 +0300 To: Cc: References: <9ec7b38b0979cb2e9ac6cb6b8f2e405c313a67f9.1630305008.git.imeevma@gmail.com> In-Reply-To: <9ec7b38b0979cb2e9ac6cb6b8f2e405c313a67f9.1630305008.git.imeevma@gmail.com> Date: Tue, 31 Aug 2021 22:32:46 +0300 Message-ID: <017001d79e9e$f9d5f8d0$ed81ea70$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQF8e8DS5nlzI215Jd5PRNk155tKOqxEQ0Ng Content-Language: ru X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9EE8D69D3379D80C4D4372D9D0A048EC97FC5358F90773914182A05F53808504096E18A3AE9C5174B849963891CB37779735059A4AD89684D8145841C165BC1EA X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE78BAADB77C21FF6F2EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637803A71E7516670C28638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8F02629777FE9C97F5B65364C62AA93AD117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EEC65AC60A1F0286FE9735652A29929C6C4AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C3BB339968D8EBD1C6BA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7994FE22CF3C16DE0731C566533BA786AA5CC5B56E945C8DA X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458F0AFF96BAACF4158235E5A14AD4A4A4625E192CAD1D9E79DB8BF922E402C092C8AE5D25F42189EF4 X-C1DE0DAB: 0D63561A33F958A53B05662927F73E21AD1E3824449691EF0D1C9ACDDE8C6194D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA752546FE575EB473F1410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D340B06C327CE4A70E8087DADDEAF3AD1A4E0D4E1E1038EFF0DD6C84D52EDB0D7A59F4EC5B17CF2DF761D7E09C32AA3244C7CE916CB6FDD47F3637290232577995C81560E2432555DBB729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojKvNUqyKacB3Qqyt+6G84yQ== X-Mailru-Sender: B5B6A6EBBD94DAD88AE996DFA43B41793BE9BB2ACD23C7C354D26944611DE0B33355544F2E112B035C2808D6142752370A8ED71B308007E3DC85537438B7E1A423D748DE48713E689437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v1 1/1] sql: fix a segfault in hex() on receiving zeroblob X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Timur Safin via Tarantool-patches Reply-To: Timur Safin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" I may miss something obvious, but prior version of a code with pBlob and n was much shorter, compacter and more readable. I'm curious, why do you prefer to always use argv[0]->n and argv[0]->z instead? Also, it seems to me we better to limit the number of bytes customer may request to allocate from HEX()? What about to check against = SQL_LIMIT_LENGTH? Thanks, Timur > -----Original Message----- > From: imeevma@tarantool.org > Sent: Monday, August 30, 2021 9:31 AM > To: tsafin@tarantool.org > Cc: tarantool-patches@dev.tarantool.org > Subject: [PATCH v1 1/1] sql: fix a segfault in hex() on receiving > zeroblob >=20 > This patch fixes a segmentation fault when zeroblob is received by > the > SQL built-in HEX() function. >=20 > Closes #6113 > --- > https://github.com/tarantool/tarantool/issues/6113 > https://github.com/tarantool/tarantool/tree/imeevma/gh-6113-fix-hex- > segfault-2.10 >=20 > .../gh-6113-fix-segfault-in-hex-func.md | 5 ++ > src/box/sql/func.c | 75 ++++++++++------- > -- > test/sql-tap/engine.cfg | 1 + > ...gh-6113-assert-in-hex-on-zeroblob.test.lua | 13 ++++ > 4 files changed, 58 insertions(+), 36 deletions(-) > create mode 100644 changelogs/unreleased/gh-6113-fix-segfault-in- > hex-func.md > create mode 100755 test/sql-tap/gh-6113-assert-in-hex-on- > zeroblob.test.lua >=20 > diff --git a/changelogs/unreleased/gh-6113-fix-segfault-in-hex- > func.md b/changelogs/unreleased/gh-6113-fix-segfault-in-hex-func.md > new file mode 100644 > index 000000000..c59be4d96 > --- /dev/null > +++ b/changelogs/unreleased/gh-6113-fix-segfault-in-hex-func.md > @@ -0,0 +1,5 @@ > +## bugfix/sql > + > +* The HEX() SQL built-in function now does not throw an assert on > receiving > + varbinary values that consist of zero-bytes (gh-6113). > + > diff --git a/src/box/sql/func.c b/src/box/sql/func.c > index c063552d6..fa2a2c245 100644 > --- a/src/box/sql/func.c > +++ b/src/box/sql/func.c > @@ -53,6 +53,44 @@ > static struct mh_strnptr_t *built_in_functions =3D NULL; > static struct func_sql_builtin **functions; >=20 > +/** Array for converting from half-bytes into ASCII hex digits. */ > +static const char hexdigits[] =3D { > + '0', '1', '2', '3', '4', '5', '6', '7', > + '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' > +}; > + > +/** Implementation of the HEX() SQL built-in function. */ > +static void > +func_hex(struct sql_context *ctx, int argc, struct Mem **argv) > +{ > + assert(argc =3D=3D 1); > + (void)argc; > + if (argv[0]->type =3D=3D MEM_TYPE_NULL) > + return mem_set_null(ctx->pOut); > + > + assert(argv[0]->type =3D=3D MEM_TYPE_BIN && argv[0]->n >=3D 0); > + assert((argv[0]->flags & MEM_Zero) =3D=3D 0 || argv[0]->u.nZero >=3D > 0); > + uint32_t size =3D 2 * argv[0]->n; > + if ((argv[0]->flags & MEM_Zero) !=3D 0) > + size +=3D 2 * argv[0]->u.nZero; > + if (size =3D=3D 0) > + return mem_set_str0_static(ctx->pOut, ""); > + > + char *str =3D sqlDbMallocRawNN(sql_get(), size); > + if (str =3D=3D NULL) { > + ctx->is_aborted =3D true; > + return; > + } > + for (int i =3D 0; i < argv[0]->n; ++i) { > + char c =3D argv[0]->z[i]; > + str[2 * i] =3D hexdigits[(c >> 4) & 0xf]; > + str[2 * i + 1] =3D hexdigits[c & 0xf]; > + } > + if ((argv[0]->flags & MEM_Zero) !=3D 0) > + memset(&str[2 * argv[0]->n], '0', 2 * argv[0]->u.nZero); > + mem_set_str_allocated(ctx->pOut, str, size); > +} > + > static const unsigned char * > mem_as_ustr(struct Mem *mem) > { > @@ -1072,14 +1110,6 @@ sql_func_version(struct sql_context *context, > sql_result_text(context, tarantool_version(), -1, SQL_STATIC); > } >=20 > -/* Array for converting from half-bytes (nybbles) into ASCII hex > - * digits. > - */ > -static const char hexdigits[] =3D { > - '0', '1', '2', '3', '4', '5', '6', '7', > - '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' > -}; > - > /* > * Implementation of the QUOTE() function. This function takes a > single > * argument. If the argument is numeric, the return value is the > same as > @@ -1233,33 +1263,6 @@ charFunc(sql_context * context, int argc, > sql_value ** argv) > sql_result_text64(context, (char *)z, zOut - z, sql_free); > } >=20 > -/* > - * The hex() function. Interpret the argument as a blob. Return > - * a hexadecimal rendering as text. > - */ > -static void > -hexFunc(sql_context * context, int argc, sql_value ** argv) > -{ > - int i, n; > - const unsigned char *pBlob; > - char *zHex, *z; > - assert(argc =3D=3D 1); > - UNUSED_PARAMETER(argc); > - pBlob =3D mem_as_bin(argv[0]); > - n =3D mem_len_unsafe(argv[0]); > - assert(pBlob =3D=3D mem_as_bin(argv[0])); /* No encoding change */ > - z =3D zHex =3D contextMalloc(context, ((i64) n) * 2 + 1); > - if (zHex) { > - for (i =3D 0; i < n; i++, pBlob++) { > - unsigned char c =3D *pBlob; > - *(z++) =3D hexdigits[(c >> 4) & 0xf]; > - *(z++) =3D hexdigits[c & 0xf]; > - } > - *z =3D 0; > - sql_result_text(context, zHex, n * 2, sql_free); > - } > -} > - > /* > * The zeroblob(N) function returns a zero-filled blob of size N > bytes. > */ > @@ -2034,7 +2037,7 @@ static struct sql_func_definition definitions[] > =3D { > {"GROUP_CONCAT", 2, {FIELD_TYPE_VARBINARY, > FIELD_TYPE_VARBINARY}, > FIELD_TYPE_VARBINARY, groupConcatStep, groupConcatFinalize}, >=20 > - {"HEX", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_STRING, hexFunc, > NULL}, > + {"HEX", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_STRING, func_hex, > NULL}, > {"IFNULL", 2, {FIELD_TYPE_ANY, FIELD_TYPE_ANY}, > FIELD_TYPE_SCALAR, > sql_builtin_stub, NULL}, >=20 > diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg > index 587adbed9..5ff0219fc 100644 > --- a/test/sql-tap/engine.cfg > +++ b/test/sql-tap/engine.cfg > @@ -35,6 +35,7 @@ > "built-in-functions.test.lua": { > "memtx": {"engine": "memtx"} > }, > + "gh-6113-assert-in-hex-on-zeroblob.test.lua": {}, > "gh-4077-iproto-execute-no-bind.test.lua": {}, > "gh-6375-assert-on-unsupported-ext.test.lua": {}, > "*": { > diff --git a/test/sql-tap/gh-6113-assert-in-hex-on-zeroblob.test.lua > b/test/sql-tap/gh-6113-assert-in-hex-on-zeroblob.test.lua > new file mode 100755 > index 000000000..91a29a5b4 > --- /dev/null > +++ b/test/sql-tap/gh-6113-assert-in-hex-on-zeroblob.test.lua > @@ -0,0 +1,13 @@ > +#!/usr/bin/env tarantool > +local test =3D require("sqltester") > +test:plan(1) > + > +test:do_execsql_test( > + "gh-6113", > + [[ > + SELECT hex(zeroblob(0)), hex(zeroblob(10)); > + ]], { > + '', '00000000000000000000' > + }) > + > +test:finish_test() > -- > 2.25.1