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 A37356E459; Thu, 18 Nov 2021 17:09:16 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A37356E459 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1637244556; bh=+Cg28rnDuU6f1gqjsx5XpuH9uyAmCWVtBNY54vkebdQ=; h=To:Cc:Date:In-Reply-To:References:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=x4Og14+oNwLRT8oF3iL+29dEOyi/Ae+K1X8kbID9OiStrII/UsZ1Jh2DwgYguEyLt ccjYpJ3GpUYF7VFo6v7kSywBtAm9BbgAfUXCkgK5fkRdMF22ylKPX3JXtI2nEgN3gC WxH1exdueoc9qeC3zF3WvexETQYhz2TBlhAIjVwo= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (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 C9C556E459 for ; Thu, 18 Nov 2021 17:08:44 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C9C556E459 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1mni5k-00037k-7Z; Thu, 18 Nov 2021 17:08:44 +0300 To: v.shpilevoy@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Thu, 18 Nov 2021 17:08:43 +0300 Message-Id: X-Mailer: git-send-email 2.25.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD927BBD05C2915ABC3F5F793E7CCBBEEA633125E1C7D4BB9B8182A05F5380850404C228DA9ACA6FE272170A6BEA6524D7D8F80AFF4982B97991D289732E18041123BB456FB5FDB3C05 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7E50C24D7D7C3118AC2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE74ABD1465354149F7EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BBCA57AF85F7723F2080FC2680C0853BF5F92532F903E53C9CC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC81D471462564A2E19F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BDAE3FA6833AEA0C275ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975C83792D63EF07D36BFD976D7B46275FD3EBBA18DF95F0C60C9C2B6934AE262D3EE7EAB7254005DCED114C52B35DBB74F4E7EAB7254005DCEDA5DF9383870C0FED1E0A4E2319210D9B64D260DF9561598F01A9E91200F654B0CE242E6D48BF61858E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3453741B480A6503C3D50966185092E161034C8A748F6CBA6C1C20A445231B1D20C35A906004088C351D7E09C32AA3244C6AC98808A353AE41FF05C94601AFE89C63871F383B54D9B3729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojb91S64R+cbsgRIRlY5FMAQ== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D465FA9B474D1ECA86E149CA2E8239AC683D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH v1 1/2] sql: properly check bind variable names 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: imeevma@tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" After this patch, variable names will have to follow the rules defined for identifiers in SQL. This essentially means that the number will no longer be used as the first character of the bind variable name. Part of #4763 --- src/box/sql/expr.c | 46 +++++++++++++++++++++++++++++++++++++ src/box/sql/parse.y | 32 ++++++++++++-------------- src/box/sql/sqlInt.h | 10 ++++++++ src/box/sql/tokenize.c | 26 ++++++--------------- test/sql-tap/bind.test.lua | 15 ++++++++++++ test/sql-tap/misc1.test.lua | 4 ++-- test/sql/iproto.result | 2 +- 7 files changed, 96 insertions(+), 39 deletions(-) create mode 100755 test/sql-tap/bind.test.lua diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index eb169aeb8..74a98c550 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -1314,6 +1314,52 @@ sqlExprAssignVarNumber(Parse * pParse, Expr * pExpr, u32 n) } } +struct Expr * +expr_variable(struct Parse *parse, struct Token *spec, struct Token *id) +{ + assert(spec != 0 && id != NULL && spec->n == 1); + if (id->z - spec->z != 1) { + diag_set(ClientError, ER_SQL_UNKNOWN_TOKEN, parse->line_count, + spec->z - parse->zTail + 1, spec->n, spec->z); + parse->is_aborted = true; + return NULL; + } + if (parse->parse_only) { + diag_set(ClientError, ER_SQL_PARSER_GENERIC_WITH_POS, + parse->line_count, parse->line_pos, + "bindings are not allowed in DDL"); + parse->is_aborted = true; + return NULL; + } + if (spec->z[0] == '#' && (id != NULL && sqlIsdigit(id->z[0]))) { + diag_set(ClientError, ER_SQL_SYNTAX_NEAR_TOKEN, + parse->line_count, spec->n, spec->z); + parse->is_aborted = true; + return NULL; + } + uint32_t len = spec->n + id->n; + struct Expr *expr = sqlDbMallocRawNN(parse->db, + sizeof(*expr) + len + 1); + if (expr == NULL) { + parse->is_aborted = true; + return NULL; + } + memset(expr, 0, sizeof(*expr)); + expr->type = FIELD_TYPE_BOOLEAN; + expr->op = TK_VARIABLE; + expr->flags = EP_Leaf; + expr->iAgg = -1; + expr->u.zToken = (char *)(expr + 1); + expr->u.zToken[0] = spec->z[0]; + memcpy(expr->u.zToken + 1, id->z, id->n); + expr->u.zToken[len] = '\0'; + assert(SQL_MAX_EXPR_DEPTH > 0); + expr->nHeight = 1; + + sqlExprAssignVarNumber(parse, expr, len); + return expr; +} + /* * Recursively delete an expression tree. */ diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 92fb37dd4..06e6244e3 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -1087,31 +1087,29 @@ term(A) ::= INTEGER(X). { A.zEnd = X.z + X.n; if( A.pExpr ) A.pExpr->flags |= EP_Leaf; } -expr(A) ::= VARIABLE(X). { - Token t = X; +expr(A) ::= VARNUM(X). { if (pParse->parse_only) { - spanSet(&A, &t, &t); diag_set(ClientError, ER_SQL_PARSER_GENERIC_WITH_POS, pParse->line_count, pParse->line_pos, "bindings are not allowed in DDL"); pParse->is_aborted = true; A.pExpr = NULL; - } else if (!(X.z[0]=='#' && sqlIsdigit(X.z[1]))) { - u32 n = X.n; + A.zEnd = &X.z[X.n]; + } else { spanExpr(&A, pParse, TK_VARIABLE, X); - if (A.pExpr->u.zToken[0] == '?' && n > 1) { - diag_set(ClientError, ER_SQL_SYNTAX_NEAR_TOKEN, pParse->line_count, t.n, t.z); - pParse->is_aborted = true; - } else { - sqlExprAssignVarNumber(pParse, A.pExpr, n); - } - }else{ - assert( t.n>=2 ); - spanSet(&A, &t, &t); - diag_set(ClientError, ER_SQL_SYNTAX_NEAR_TOKEN, pParse->line_count, t.n, t.z); - pParse->is_aborted = true; - A.pExpr = NULL; + assert(A.pExpr->u.zToken[0] == '?' && X.n == 1); + sqlExprAssignVarNumber(pParse, A.pExpr, X.n); } } +expr(A) ::= VARIABLE(X) id(Y). { + A.pExpr = expr_variable(pParse, &X, &Y); + A.zStart = X.z; + A.zEnd = &Y.z[Y.n]; +} +expr(A) ::= VARIABLE(X) INTEGER(Y). { + A.pExpr = expr_variable(pParse, &X, &Y); + A.zStart = X.z; + A.zEnd = &Y.z[Y.n]; +} expr(A) ::= expr(A) COLLATE id(C). { A.pExpr = sqlExprAddCollateToken(pParse, A.pExpr, &C, 1); A.zEnd = &C.z[C.n]; diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index dcd71e5bd..ab5b098ee 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -2617,6 +2617,16 @@ Expr *sqlExprFunction(Parse *, ExprList *, Token *); void sqlExprAssignVarNumber(Parse *, Expr *, u32); ExprList *sqlExprListAppendVector(Parse *, ExprList *, IdList *, Expr *); +/** + * Parse tokens as a name or a position of bound variable. + * + * @param parse Parse context. + * @param spec Special symbol for bound variable. + * @param id Name or position number of bound variable. + */ +struct Expr * +expr_variable(struct Parse *parse, struct Token *spec, struct Token *id); + /** * Set the sort order for the last element on the given ExprList. * diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c index f2d5a2df5..8bc519b9d 100644 --- a/src/box/sql/tokenize.c +++ b/src/box/sql/tokenize.c @@ -58,8 +58,8 @@ #define CC_KYWD 1 /* Alphabetics or '_'. Usable in a keyword */ #define CC_ID 2 /* unicode characters usable in IDs */ #define CC_DIGIT 3 /* Digits */ -#define CC_DOLLAR 4 /* '$' */ -#define CC_VARALPHA 5 /* '@', '#', ':'. Alphabetic SQL variables */ +/** SQL variables: '@', '#', ':', and '$'. */ +#define CC_VARALPHA 5 #define CC_VARNUM 6 /* '?'. Numeric SQL variables */ #define CC_SPACE 7 /* Space characters */ #define CC_QUOTE 8 /* '\''. String literals */ @@ -90,7 +90,7 @@ static const char sql_ascii_class[] = { /* x0 x1 x2 x3 x4 x5 x6 x7 x8 x9 xa xb xc xd xe xf */ /* 0x */ 27, 27, 27, 27, 27, 27, 27, 27, 27, 7, 28, 7, 7, 7, 27, 27, /* 1x */ 27, 27, 27, 27, 27, 27, 27, 27, 27, 27, 27, 27, 27, 27, 27, 27, -/* 2x */ 7, 15, 9, 5, 4, 22, 24, 8, 17, 18, 21, 20, 23, 11, 26, 16, +/* 2x */ 7, 15, 9, 5, 5, 22, 24, 8, 17, 18, 21, 20, 23, 11, 26, 16, /* 3x */ 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 5, 19, 12, 14, 13, 6, /* 4x */ 5, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, /* 5x */ 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 29, 27, 30, 27, 1, @@ -184,7 +184,7 @@ int sql_token(const char *z, int *type, bool *is_reserved) { *is_reserved = false; - int i, n; + int i; char c, delim; /* Switch on the character-class of the first byte * of the token. See the comment on the CC_ defines @@ -369,23 +369,11 @@ sql_token(const char *z, int *type, bool *is_reserved) } return i; case CC_VARNUM: - *type = TK_VARIABLE; - for (i = 1; sqlIsdigit(z[i]); i++) { - } - return i; - case CC_DOLLAR: + *type = TK_VARNUM; + return 1; case CC_VARALPHA: - n = 0; *type = TK_VARIABLE; - for (i = 1; (c = z[i]) != 0; i++) { - if (IdChar(c)) - n++; - else - break; - } - if (n == 0) - *type = TK_ILLEGAL; - return i; + return 1; case CC_KYWD: for (i = 1; sql_ascii_class[*(unsigned char*)(z+i)] <= CC_KYWD; i++) { diff --git a/test/sql-tap/bind.test.lua b/test/sql-tap/bind.test.lua new file mode 100755 index 000000000..aaf14a44d --- /dev/null +++ b/test/sql-tap/bind.test.lua @@ -0,0 +1,15 @@ +#!/usr/bin/env tarantool +local test = require("sqltester") +test:plan(1) + +-- Make sure that bind variable names cannot start with a number. +test:do_test( + "bind-1", + function() + local res = {pcall(box.execute, [[SELECT @1asd;]], {{['@1asd'] = 123}})} + return {tostring(res[3])} + end, { + "At line 1 at or near position 9: unrecognized token '1asd'" + }) + +test:finish_test() diff --git a/test/sql-tap/misc1.test.lua b/test/sql-tap/misc1.test.lua index f207d3e92..204e070d2 100755 --- a/test/sql-tap/misc1.test.lua +++ b/test/sql-tap/misc1.test.lua @@ -1052,7 +1052,7 @@ test:do_catchsql_test( select''like''like''like#0; ]], { -- - 1, [[Syntax error at line 1 near '#0']] + 1, [[Syntax error at line 1 near '#']] -- }) @@ -1062,7 +1062,7 @@ test:do_catchsql_test( VALUES(0,0x0MATCH#0; ]], { -- - 1, [[Syntax error at line 1 near '#0']] + 1, [[Syntax error at line 1 near '#']] -- }) diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 6212aa0c0..5a424596e 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -351,7 +351,7 @@ cn:execute('drop table if exists test3') -- cn:execute('select ?1, ?2, ?3', {1, 2, 3}) --- -- error: Syntax error at line 1 near '?1' +- error: Syntax error at line 1 near '1' ... cn:execute('select $name, $name2', {1, 2}) --- -- 2.25.1