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 430166ECDB; Mon, 14 Mar 2022 19:17:47 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 430166ECDB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1647274667; bh=/D+4BV/m8hRZpWPK4Ogn/lOq1wQcuof591JsPqQ7ifA=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=Z1UJg7TeaBaXoAj4SEfsvAHMJYx/5W4oKXgbsInnPrFeLXP/NU7N/wOGB5Vk86R8/ 37ZPyv6wtyoBLx7jeLdTa+YfKRPQ1d2upzFLpH40bfh8WaWeZ+Vi8gBSn9kFvLwt/o cHnTSNidJNRHk3yHbpPJH6n9ADvocuapD+dI/aT8= Received: from smtpng2.i.mail.ru (smtpng2.i.mail.ru [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 7A1B26ECDB for ; Mon, 14 Mar 2022 19:17:45 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7A1B26ECDB Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1nTnOC-0003AT-ON; Mon, 14 Mar 2022 19:17:45 +0300 Date: Mon, 14 Mar 2022 19:17:43 +0300 To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Message-ID: <20220314161743.GA199674@tarantool.org> References: <0985d5f43f0eb85d5a2fdcff90f0b2a21a2ecfe8.1642066322.git.imeevma@gmail.com> <129b7645-1b81-a3b1-c397-a2d878fb59a1@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <129b7645-1b81-a3b1-c397-a2d878fb59a1@tarantool.org> X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD95983D7D89D92196DC3ADA2256CB2C78C5981BBAFE660DCA7182A05F538085040B57BAE79E75E174B4954CD0A3FF27EEBB3853CA2FCE239CBD1AEAE6F736475CA X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7AED985C8E545F588EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637A164EE347039141E8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8FD46A3E52C2164ECC37C6392E145E848117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC3A703B70628EAD7BA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD182CC0D3CB04F14752D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6598BD4784909AC60089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A5B2A7CB571CDB8181758282426A82EDC112E31A49A93DA8BFD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7575C41713E13FA5B7410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34E229BE567979C9402335460DD774793BFFD993E9345CE00CD7DEB28750DC86FD280B1B5508B4F0FB1D7E09C32AA3244C0FFD9158A2155BEFBF9FE4175776332369B6CAE0477E908D729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojqbhzdNy/MvitwGEpC9j6Uw== X-Mailru-Sender: 689FA8AB762F739339CABD9B3CA9A7D6E3D77ADA9248A872AF51D093018140A083D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E3365FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v1 1/1] sql: fix assert when MP_EXT received via netbox 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: Mergen Imeev via Tarantool-patches Reply-To: Mergen Imeev Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi! Thanks for the review! And sorry for such a late reply. I rewrote this patch and created a PR instead of pasting the changes here: https://github.com/tarantool/tarantool/pull/6922 On Fri, Jan 14, 2022 at 11:36:34PM +0100, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > See 4 comments below. > > On 13.01.2022 10:37, imeevma@tarantool.org wrote: > > This patch fixes an assertion or segmentation fault when getting the > > value of MP_EXT via netbox. > > > > Closes #6766 > > --- > > https://github.com/tarantool/tarantool/issues/6766 > > https://github.com/tarantool/tarantool/tree/imeevma/gh-6766-fix-assert-when-decoding-mp-ext > > > > src/box/bind.c | 30 ++++++++++++++++++- > > test/sql-tap/engine.cfg | 1 + > > .../gh-6766-fix-bind-for-mp-ext.test.lua | 30 +++++++++++++++++++ > > 1. Lets also add a changelog file. > Added. > > 3 files changed, 60 insertions(+), 1 deletion(-) > > create mode 100755 test/sql-tap/gh-6766-fix-bind-for-mp-ext.test.lua > > > > diff --git a/src/box/bind.c b/src/box/bind.c > > index 441c9f46f..6672d1271 100644 > > --- a/src/box/bind.c > > +++ b/src/box/bind.c > > @@ -99,9 +101,35 @@ sql_bind_decode(struct sql_bind *bind, int i, const char **packet) > > case MP_BIN: > > bind->s = mp_decode_bin(packet, &bind->bytes); > > break; > > + case MP_EXT: { > > + int8_t ext_type; > > + const char *svp = *packet; > > + uint32_t size = mp_decode_extl(packet, &ext_type); > > + if (ext_type != MP_UUID && ext_type != MP_DECIMAL) { > > + bind->s = svp; > > + *packet += size; > > + bind->bytes = *packet - svp; > > + break; > > 2. It might be better to move this below into 'else' branch after all > the type checks. > True. However, this part is actually wrong, we should throw an error here. Fixed. > > + } > > + *packet = svp; > > + if (ext_type == MP_UUID) { > > + if (mp_decode_uuid(packet, &bind->uuid) == NULL) { > > 3. You already decoded the ext header. So you can call uuid_unpack(). > Same below with decimal_unpack(). > Thanks, fixed. > 4. It still might be good to handle unknown MP_EXT values in sql_bind_column > or before it. Otherwise you will get a crash on any of them: > > netbox = require('net.box') > msgpack = require('msgpack') > box.cfg{listen = 3313} > box.schema.user.grant('guest', 'super') > > str = '\xc7\x00\x0f' > val = msgpack.object_from_raw(str) > con = netbox.connect(box.cfg.listen) > con:execute([[SELECT typeof(?);]], {val}) > > Assertion failed: (p->ext_type == MP_UUID || p->ext_type == MP_DECIMAL), > function sql_bind_column, file > /Users/gerold/Work/Repositories/tarantool/src/box/bind.c, line 220. > > Here I created a value of extension 15, which we don't support. Thank you the test! This problem was fixed by fixing 2. I added this test.