From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 4BE312E2FE for ; Wed, 15 May 2019 13:34:34 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id L3D7drvT4T77 for ; Wed, 15 May 2019 13:34:34 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id DD2B62A0EE for ; Wed, 15 May 2019 13:34:33 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v1 2/3] sql: ban sql functions coinciding with builtins Date: Wed, 15 May 2019 20:34:29 +0300 Message-Id: <068d058c4b8354f332e67e95761f722ab72cbae0.1557941573.git.kshcherbatov@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org, korablev@tarantool.org Cc: Kirill Shcherbatov Previously, it was possible to create a function with name that is already reserved for some predefined builtin. It is a bug. This patch bans sql_create_function for such cases. Also removed FUNC_PERFECT_MATCH user session flag. Closes #4219 Needed for #3691 --- src/box/lua/lua_sql.c | 8 +- src/box/sql/callback.c | 174 +++++++++++++++------------------- src/box/sql/expr.c | 18 ++-- src/box/sql/func.c | 33 +++---- src/box/sql/main.c | 33 ++++--- src/box/sql/resolve.c | 7 +- src/box/sql/sqlInt.h | 25 ++++- src/box/sql/vdbemem.c | 2 +- test/sql-tap/lua_sql.test.lua | 11 ++- 9 files changed, 157 insertions(+), 154 deletions(-) diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c index 36b75ff08..561353520 100644 --- a/src/box/lua/lua_sql.c +++ b/src/box/lua/lua_sql.c @@ -195,7 +195,11 @@ lbox_sql_create_function(struct lua_State *L) is_deterministic ? SQL_DETERMINISTIC : 0, func_info, lua_sql_call, NULL, NULL, lua_sql_destroy); - if (rc != 0) - return luaL_error(L, sqlErrStr(rc)); + if (rc != 0) { + const char *err = rc == SQL_TARANTOOL_ERROR ? + box_error_message(box_error_last()) : + sqlErrStr(rc); + return luaL_error(L, err); + } return 0; } diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c index 49197532e..b4247324d 100644 --- a/src/box/sql/callback.c +++ b/src/box/sql/callback.c @@ -131,6 +131,19 @@ functionSearch(int h, /* Hash of the name */ return 0; } +/** + * Cache function is used to organise sqlBuiltinFunctions table. + * @param func_name The name of builtin function. + * @param func_name_len The length of the @a name. + * @retval Hash value is calculated for given name. + */ +static int +sql_builtin_func_name_hash(const char *func_name, uint32_t func_name_len) +{ + return (sqlUpperToLower[(u8) func_name[0]] + + func_name_len) % SQL_FUNC_HASH_SZ; +} + /* * Insert a new FuncDef into a FuncDefHash hash table. */ @@ -144,9 +157,7 @@ sqlInsertBuiltinFuncs(FuncDef * aDef, /* List of global functions to be inserted FuncDef *pOther; const char *zName = aDef[i].zName; int nName = sqlStrlen30(zName); - int h = - (sqlUpperToLower[(u8) zName[0]] + - nName) % SQL_FUNC_HASH_SZ; + int h = sql_builtin_func_name_hash(zName, nName); pOther = functionSearch(h, zName); if (pOther) { assert(pOther != &aDef[i] && pOther->pNext != &aDef[i]); @@ -160,110 +171,79 @@ sqlInsertBuiltinFuncs(FuncDef * aDef, /* List of global functions to be inserted } } -/* - * Locate a user function given a name, a number of arguments and a flag - * indicating whether the function prefers UTF-16 over UTF-8. Return a - * pointer to the FuncDef structure that defines that function, or return - * NULL if the function does not exist. - * - * If the createFlag argument is true, then a new (blank) FuncDef - * structure is created and liked into the "db" structure if a - * no matching function previously existed. - * - * If nArg is -2, then the first valid function found is returned. A - * function is valid if xSFunc is non-zero. The nArg==(-2) - * case is used to see if zName is a valid function name for some number - * of arguments. If nArg is -2, then createFlag must be 0. - * - * If createFlag is false, then a function with the required name and - * number of arguments may be returned even if the eTextRep flag does not - * match that requested. - */ -FuncDef * -sqlFindFunction(sql * db, /* An open database */ - const char *zName, /* Name of the function. zero-terminated */ - int nArg, /* Number of arguments. -1 means any number */ - u8 createFlag /* Create new entry if true and does not otherwise exist */ - ) +struct FuncDef * +sql_find_function(struct sql *db, const char *func_name, int arg_count, + bool is_builtin, bool is_create) { - FuncDef *p; /* Iterator variable */ - FuncDef *pBest = 0; /* Best match found so far */ - int bestScore = 0; /* Score of best match */ - int h; /* Hash value */ - int nName; /* Length of the name */ - struct session *user_session = current_session(); - - assert(nArg >= (-2)); - assert(nArg >= (-1) || createFlag == 0); - nName = sqlStrlen30(zName); - - /* First search for a match amongst the application-defined functions. + assert(arg_count >= -2); + assert(arg_count >= -1 || !is_create); + assert(!is_create || !is_builtin); + uint32_t func_name_len = strlen(func_name); + /* + * The 'score' of the best match is calculated + * with matchQuality estimator. + */ + int func_score = 0; + /* + * The pointer of a function having the highest 'score' + * for given name and arg_count parameters. */ - p = (FuncDef *) sqlHashFind(&db->aFunc, zName); - while (p) { - int score = matchQuality(p, nArg); - if (score > bestScore) { - pBest = p; - bestScore = score; + struct FuncDef *func = NULL; + if (is_builtin) + goto lookup_for_builtin; + /* Search amongst the user-defined functions. */ + for (struct FuncDef *p = sqlHashFind(&db->aFunc, func_name); + p != NULL; p = p->pNext) { + int score = matchQuality(p, arg_count); + if (score > func_score) { + func = p; + func_score = score; } - p = p->pNext; } - - /* If no match is found, search the built-in functions. - * - * If the SQL_PreferBuiltin flag is set, then search the built-in - * functions even if a prior app-defined function was found. And give - * priority to built-in functions. - * - * Except, if createFlag is true, that means that we are trying to - * install a new function. Whatever FuncDef structure is returned it will - * have fields overwritten with new information appropriate for the - * new function. But the FuncDefs for built-in functions are read-only. - * So we must not search for built-ins when creating a new function. - */ - if (!createFlag && - (pBest == 0 - || (user_session->sql_flags & SQL_PreferBuiltin) != 0)) { - bestScore = 0; - h = (sqlUpperToLower[(u8) zName[0]] + - nName) % SQL_FUNC_HASH_SZ; - p = functionSearch(h, zName); - while (p) { - int score = matchQuality(p, nArg); - if (score > bestScore) { - pBest = p; - bestScore = score; + if (!is_create && func == NULL) { +lookup_for_builtin: + func_score = 0; + int h = sql_builtin_func_name_hash(func_name, func_name_len); + for (struct FuncDef *p = functionSearch(h, func_name); + p != NULL; p = p->pNext) { + int score = matchQuality(p, arg_count); + if (score > func_score) { + func = p; + func_score = score; } - p = p->pNext; } } - - /* If the createFlag parameter is true and the search did not reveal an - * exact match for the name, number of arguments and encoding, then add a - * new entry to the hash table and return it. + /* + * If the is_create parameter is true and the search did + * not reveal an exact match for the name, number of + * arguments, then add a new entry to the hash table and + * return it. */ - if (createFlag && bestScore < FUNC_PERFECT_MATCH && - (pBest = - sqlDbMallocZero(db, sizeof(*pBest) + nName + 1)) != 0) { - FuncDef *pOther; - pBest->zName = (const char *)&pBest[1]; - pBest->nArg = (u16) nArg; - pBest->funcFlags = 0; - memcpy((char *)&pBest[1], zName, nName + 1); - pOther = - (FuncDef *) sqlHashInsert(&db->aFunc, pBest->zName, - pBest); - if (pOther == pBest) { - sqlDbFree(db, pBest); + if (is_create && func_score < FUNC_PERFECT_MATCH) { + uint32_t func_sz = sizeof(func[0]) + func_name_len + 1; + func = sqlDbMallocZero(db, func_sz); + if (func == NULL) { + diag_set(OutOfMemory, func_sz, "sqlDbMallocZero", + "func"); + return NULL; + } + func->zName = (const char *) &func[1]; + func->nArg = (u16) arg_count; + func->funcFlags = 0; + memcpy((char *)&func[1], func_name, func_name_len + 1); + struct FuncDef *old_func = + (struct FuncDef *) sqlHashInsert(&db->aFunc, func->zName, + func); + if (old_func == func) { + sqlDbFree(db, func); sqlOomFault(db); - return 0; + diag_set(OutOfMemory, func_sz, "sqlHashInsert", + "func"); + return NULL; } else { - pBest->pNext = pOther; + func->pNext = old_func; } } - - if (pBest && (pBest->xSFunc || createFlag)) { - return pBest; - } - return 0; + return func != NULL && + (func->xSFunc != NULL || is_create || is_builtin) ? func : NULL; } diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index ba7eea59d..f7507938d 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -285,9 +285,9 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id, if (op == TK_FUNCTION) { uint32_t arg_count = p->x.pList == NULL ? 0 : p->x.pList->nExpr; - struct FuncDef *func = sqlFindFunction(parse->db, - p->u.zToken, - arg_count, 0); + struct FuncDef *func = + sql_find_function(parse->db, p->u.zToken, + arg_count, false, false); if (func == NULL) break; if (func->is_coll_derived) { @@ -3991,14 +3991,14 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) nFarg = pFarg ? pFarg->nExpr : 0; assert(!ExprHasProperty(pExpr, EP_IntValue)); zId = pExpr->u.zToken; - pDef = sqlFindFunction(db, zId, nFarg, 0); + pDef = sql_find_function(db, zId, nFarg, false, false); #ifdef SQL_ENABLE_UNKNOWN_SQL_FUNCTION if (pDef == 0 && pParse->explain) { - pDef = - sqlFindFunction(db, "unknown", nFarg, 0); + pDef = sql_find_function(db, "unknown", nFarg, + false, false); } #endif - if (pDef == 0 || pDef->xFinalize != 0) { + if (pDef == NULL || pDef->xFinalize != NULL) { diag_set(ClientError, ER_NO_SUCH_FUNCTION, zId); pParse->is_aborted = true; @@ -5461,12 +5461,12 @@ analyzeAggregate(Walker * pWalker, Expr * pExpr) pItem->iMem = ++pParse->nMem; assert(!ExprHasProperty (pExpr, EP_IntValue)); - pItem->pFunc = sqlFindFunction( + pItem->pFunc = sql_find_function( pParse->db, pExpr->u.zToken, pExpr->x.pList ? pExpr->x.pList->nExpr : 0, - 0); + false, false); if (pExpr->flags & EP_Distinct) { pItem->iDistinct = pParse->nTab++; diff --git a/src/box/sql/func.c b/src/box/sql/func.c index bb7405e68..cbcf26125 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -1811,7 +1811,7 @@ sql_overload_function(sql * db, const char *zName, { int rc = SQL_OK; - if (sqlFindFunction(db, zName, nArg, 0) == 0) { + if (sql_find_function(db, zName, nArg, false, false) == NULL) { rc = sqlCreateFunc(db, zName, type, nArg, 0, 0, sqlInvalidFunction, 0, 0, 0); } @@ -1834,19 +1834,6 @@ sqlRegisterPerConnectionBuiltinFunctions(sql * db) } } -/* - * Set the LIKEOPT flag on the 2-argument function with the given name. - */ -static void -setLikeOptFlag(sql * db, const char *zName, u8 flagVal) -{ - FuncDef *pDef; - pDef = sqlFindFunction(db, zName, 2, 0); - if (ALWAYS(pDef)) { - pDef->funcFlags |= flagVal; - } -} - /** * Register the built-in LIKE function. */ @@ -1859,13 +1846,14 @@ sqlRegisterLikeFunctions(sql *db, int is_case_insensitive) * supplied pattern and FALSE otherwise. */ int *is_like_ci = SQL_INT_TO_PTR(is_case_insensitive); - sqlCreateFunc(db, "LIKE", FIELD_TYPE_BOOLEAN, 2, 0, - is_like_ci, likeFunc, 0, 0, 0); - sqlCreateFunc(db, "LIKE", FIELD_TYPE_BOOLEAN, 3, 0, - is_like_ci, likeFunc, 0, 0, 0); - setLikeOptFlag(db, "LIKE", - !(is_case_insensitive) ? (SQL_FUNC_LIKE | - SQL_FUNC_CASE) : SQL_FUNC_LIKE); + struct FuncDef *like2 = sql_find_function(db, "LIKE", 2, true, false); + like2->funcFlags = is_case_insensitive ? SQL_FUNC_LIKE : + SQL_FUNC_LIKE | SQL_FUNC_CASE; + like2->pUserData = is_like_ci; + assert(like2 != NULL); + struct FuncDef *like3 = sql_find_function(db, "LIKE", 2, true, false); + assert(like3 != NULL); + like3->pUserData = is_like_ci; } int @@ -1875,7 +1863,8 @@ sql_is_like_func(struct sql *db, struct Expr *expr, int *is_like_ci) expr->x.pList->nExpr != 2) return 0; assert(!ExprHasProperty(expr, EP_xIsSelect)); - struct FuncDef *func = sqlFindFunction(db, expr->u.zToken, 2, 0); + struct FuncDef *func = + sql_find_function(db, expr->u.zToken, 2, false, false); assert(func != NULL); if ((func->funcFlags & SQL_FUNC_LIKE) == 0) return 0; diff --git a/src/box/sql/main.c b/src/box/sql/main.c index fe1135a71..d52ec1c49 100644 --- a/src/box/sql/main.c +++ b/src/box/sql/main.c @@ -428,29 +428,28 @@ sqlCreateFunc(sql * db, assert(SQL_FUNC_CONSTANT == SQL_DETERMINISTIC); extraFlags = flags & SQL_DETERMINISTIC; - + /* Ensure there is no builtin function with this name. */ + p = sql_find_function(db, zFunctionName, -2, true, false); + if (p != NULL) { + diag_set(ClientError, ER_SQL, "cannot create a function with " + "a signature that coincides builtin function"); + return SQL_TARANTOOL_ERROR; + } /* Check if an existing function is being overridden or deleted. If so, * and there are active VMs, then return SQL_BUSY. If a function * is being overridden/deleted but there are no active VMs, allow the * operation to continue but invalidate all precompiled statements. */ - p = sqlFindFunction(db, zFunctionName, nArg, 0); - if (p && p->nArg == nArg) { - if (db->nVdbeActive) { - sqlErrorWithMsg(db, SQL_BUSY, - "unable to delete/modify user-function due to active statements"); - assert(!db->mallocFailed); - return SQL_BUSY; - } else { - sqlExpirePreparedStatements(db); - } - } - - p = sqlFindFunction(db, zFunctionName, nArg, 1); - assert(p || db->mallocFailed); - if (!p) { - return SQL_NOMEM; + p = sql_find_function(db, zFunctionName, nArg, false, false); + if (p != NULL && p->nArg == nArg && db->nVdbeActive > 0) { + sqlErrorWithMsg(db, SQL_BUSY, "unable to delete/modify " + "user-function due to active statements"); + return SQL_BUSY; } + /* Create a new function with given signature. */ + p = sql_find_function(db, zFunctionName, nArg, false, true); + if (p == NULL) + return SQL_TARANTOOL_ERROR; /* If an older version of the function with a configured destructor is * being replaced invoke the destructor function here. diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 504096e6d..7cf79692c 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -602,10 +602,11 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) assert(!ExprHasProperty(pExpr, EP_xIsSelect)); zId = pExpr->u.zToken; nId = sqlStrlen30(zId); - pDef = sqlFindFunction(pParse->db, zId, n, 0); + pDef = sql_find_function(pParse->db, zId, n, + false, false); if (pDef == 0) { - pDef = - sqlFindFunction(pParse->db, zId, -2,0); + pDef = sql_find_function(pParse->db, zId, -2, + false, false); if (pDef == 0) { no_such_func = 1; } else { diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index d2be6701f..9384ec9e8 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -1506,7 +1506,6 @@ struct sql { #define SQL_ReverseOrder 0x00020000 /* Reverse unordered SELECTs */ #define SQL_RecTriggers 0x00040000 /* Enable recursive triggers */ #define SQL_AutoIndex 0x00100000 /* Enable automatic indexes */ -#define SQL_PreferBuiltin 0x00200000 /* Preference to built-in funcs */ #define SQL_EnableTrigger 0x01000000 /* True to enable triggers */ #define SQL_DeferFKs 0x02000000 /* Defer all FK constraints */ #define SQL_VdbeEQP 0x08000000 /* Debug EXPLAIN QUERY PLAN */ @@ -3927,7 +3926,29 @@ void sqlSelectSetName(Select *, const char *); #define sqlSelectSetName(A,B) #endif void sqlInsertBuiltinFuncs(FuncDef *, int); -FuncDef *sqlFindFunction(sql *, const char *, int, u8); + +/** + * Find a user function by given name and number of arguments. + * @param db The database handle. + * @param func_name The function name 0-terminated string. + * @param arg_count The count of function's arguments. + * May be set -1 for any number. + * @param is_builtin If this flag is set true, perform lookup + * for builtin function. + * @param is_create If this flag is set true, then a new (blank) + * FuncDef structure is created and liked into + * the "db" structure if a no matching function + * previously existed. + * @retval Returns FuncDef pointer when object was found and NULL + * otherwise (if is_create is not set true). + * When is_create is set true returns NULL only in case of + * memory allocation error. + * Sets diag message in case of error. + */ +struct FuncDef * +sql_find_function(struct sql *db, const char *func_name, int arg_count, + bool is_builtin, bool is_create); + void sqlRegisterBuiltinFunctions(void); void sqlRegisterDateTimeFunctions(void); void sqlRegisterPerConnectionBuiltinFunctions(sql *); diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c index f73ea0a71..537a72f10 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -1245,7 +1245,7 @@ valueFromFunction(sql * db, /* The database connection */ pList = p->x.pList; if (pList) nVal = pList->nExpr; - pFunc = sqlFindFunction(db, p->u.zToken, nVal, 0); + pFunc = sql_find_function(db, p->u.zToken, nVal, false, false); assert(pFunc); if ((pFunc->funcFlags & (SQL_FUNC_CONSTANT | SQL_FUNC_SLOCHNG)) == 0 || (pFunc->funcFlags & SQL_FUNC_NEEDCOLL) diff --git a/test/sql-tap/lua_sql.test.lua b/test/sql-tap/lua_sql.test.lua index b0913e7f4..fd7cd732a 100755 --- a/test/sql-tap/lua_sql.test.lua +++ b/test/sql-tap/lua_sql.test.lua @@ -1,7 +1,7 @@ #!/usr/bin/env tarantool test = require("sqltester") NULL = require('msgpack').NULL -test:plan(24) +test:plan(25) local function func1(a) return a @@ -18,6 +18,15 @@ test:do_test( end, {2}) +test:do_test( + "lua_sql-1.1", + function () + local ret, errmsg = pcall(box.internal.sql_create_function, + "upper", "TEXT", func1) + return {ret, errmsg} + end, + {0, "SQL error: cannot create a function with a signature that coincides builtin function"}) + -- new function should replace prewious one test:do_test( "lua_sql-1.1", -- 2.21.0