From: Kirill Shcherbatov <kshcherbatov@tarantool.org> To: tarantool-patches@freelists.org, Konstantin Osipov <kostja@tarantool.org> Cc: vdavydov.dev@gmail.com Subject: Re: [tarantool-patches] Re: [PATCH v1 2/3] schema: extend _func space to persist lua functions Date: Fri, 17 May 2019 15:33:09 +0300 [thread overview] Message-ID: <92b6b7a0-7d8d-c239-6a40-b72f03f17e06@tarantool.org> (raw) In-Reply-To: <20190514182103.GA3801@atlas> Hi! Thank you for your feedback. > The column name should be 'code'. There is a separate column > 'language' that defines function language. > >> box.schema.func.create('sum', {lua_code = lua_code}) > > bikeshed, but I would call both the column and the option 'body', > or 'text', not 'code'. This field is called 'body' now. > > While we are at it, I would also add 'type' - function or > procedure, and other related to SQL procedures fields. > > See for example here: > > https://mariadb.com/kb/en/library/mysqlproc-table/ > (not all of these fields are relevant, and perhaps we need an > 'options' map, to make the whole thing easily extensible in the > future, but it's better to put as many fields as possible into > columns, not options). Thank you for this reference. I think we'll need 'is_deterministic' flag soon enough (in scope of functional indexes), so I've introduced it. To make the process of introducing such options extensible, @locker and I decided to put 'is_deterministic' and other new flags in the 'opts' map field. I've reused existent opts_decode mechanism in scope of this patch. > This belongs to an own function in util.c, luaT_prepare_sandbox(). It could > perhaps be done entirely in Lua and in a more efficient way. > Perhaps you could have a prepared sandbox which you could just > clone. Please investigate. I've introduced luaT_get_sandbox public method that clones an existent 'defaullt' sandbox and pushes it on the top of Lua stack in utils.h. They use private references luaL_default_sandbox_ref and luaL_deepcopy_func_ref are initialized on first demand. I've discussed this problem with @Totktonada an he approved my concept. > >> +/** >> + * Take over the modules that belonged to the old function. >> + * Reset module and function pointers in onl function. >> +*/ > > I don't understand this comment, please rephrase, and please do > not forget to describe the usage and the purpose, not just what > the function does. This allows backward compatibility with reusing loaded modules when updating the function definition, as is done in Tarantool now. /** * Take over the modules that belonged to the old function. * Reset module and function pointers in old function. * This helper allows you to inherit already loaded function * objects, that is useful on function object update. */ void func_capture_module(struct func *new_func, struct func *old_func); >> +/** >> + * Execute the persistent Lua function. >> + * Assemble Lua object when required. >> + */ >> +static int >> +execute_lua_ref_call(lua_State *L) > > What's a ref-call? Why the name of the function does not match the > comment? The new name is static int execute_persistent_function(lua_State *L) ================================================= This patch extends _func system space with a new string fields body and opts to persist Lua functions. Refactored func hash machinery to pass a new function object and to return previous one. This allows to implement fault-tolerant func_new call that constructs a lua function object. Each persistent function may use limited amount of Lua functions and modules: -assert -error -pairs -ipairs -next -pcall -xpcall -type -print -select -string -tonumber -tostring -unpack -math -utf8 Global variables are forbidden in persistent Lua functions. This patch makes persistent Lua functions are available via net.box.connect() :call method. Example: lua_code = [[function(a, b) return a + b end]] box.schema.func.create('sum', {body = lua_code, opts = {is_deterministic = true}}) conn:call("sum", {1, 3}) Part of #4182 Needed for #1260 --- src/box/alter.cc | 94 +++++++++++----- src/box/bootstrap.snap | Bin 4374 -> 4403 bytes src/box/call.c | 2 +- src/box/func.c | 69 +++++++++++- src/box/func.h | 17 ++- src/box/func_def.c | 9 ++ src/box/func_def.h | 36 +++++- src/box/lua/call.c | 36 +++++- src/box/lua/call.h | 4 +- src/box/lua/schema.lua | 9 +- src/box/lua/upgrade.lua | 21 +++- src/box/schema.cc | 46 ++++---- src/box/schema.h | 15 ++- src/box/schema_def.h | 2 + src/lua/utils.c | 67 +++++++++++ src/lua/utils.h | 8 ++ test/box-py/bootstrap.result | 6 +- test/box/access_misc.result | 4 +- test/box/persistent_func.result | 179 ++++++++++++++++++++++++++++++ test/box/persistent_func.test.lua | 83 ++++++++++++++ 20 files changed, 623 insertions(+), 84 deletions(-) create mode 100644 test/box/persistent_func.result create mode 100644 test/box/persistent_func.test.lua diff --git a/src/box/alter.cc b/src/box/alter.cc index 9279426d2..e3fd2e7ba 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -2402,21 +2402,42 @@ func_def_get_ids_from_tuple(struct tuple *tuple, uint32_t *fid, uint32_t *uid) static struct func_def * func_def_new_from_tuple(struct tuple *tuple) { - uint32_t len; - const char *name = tuple_field_str_xc(tuple, BOX_FUNC_FIELD_NAME, - &len); - if (len > BOX_NAME_MAX) + uint32_t name_len, body_len; + const char *name, *body; + name = tuple_field_str_xc(tuple, BOX_FUNC_FIELD_NAME, &name_len); + if (name_len > BOX_NAME_MAX) { tnt_raise(ClientError, ER_CREATE_FUNCTION, tt_cstr(name, BOX_INVALID_NAME_MAX), "function name is too long"); - identifier_check_xc(name, len); - struct func_def *def = (struct func_def *) malloc(func_def_sizeof(len)); + } + identifier_check_xc(name, name_len); + if (tuple_field_count(tuple) > BOX_FUNC_FIELD_BODY) { + body = tuple_field_str_xc(tuple, BOX_FUNC_FIELD_BODY, + &body_len); + } else { + body = NULL; + body_len = 0; + } + + uint32_t def_sz = func_def_sizeof(name_len, body_len); + struct func_def *def = (struct func_def *) malloc(def_sz); if (def == NULL) - tnt_raise(OutOfMemory, func_def_sizeof(len), "malloc", "def"); + tnt_raise(OutOfMemory, def_sz, "malloc", "def"); auto def_guard = make_scoped_guard([=] { free(def); }); + func_opts_create(&def->opts); func_def_get_ids_from_tuple(tuple, &def->fid, &def->uid); - memcpy(def->name, name, len); - def->name[len] = 0; + + def->name = (char *)def + sizeof(struct func_def); + memcpy(def->name, name, name_len); + def->name[name_len] = 0; + if (body_len > 0) { + def->body = def->name + name_len + 1; + memcpy(def->body, body, body_len); + def->body[body_len] = 0; + } else { + def->body = NULL; + } + if (tuple_field_count(tuple) > BOX_FUNC_FIELD_SETUID) def->setuid = tuple_field_u32_xc(tuple, BOX_FUNC_FIELD_SETUID); else @@ -2433,30 +2454,40 @@ func_def_new_from_tuple(struct tuple *tuple) /* Lua is the default. */ def->language = FUNC_LANGUAGE_LUA; } + if (def->language != FUNC_LANGUAGE_LUA && body_len > 0) { + tnt_raise(ClientError, ER_CREATE_FUNCTION, name, + "function body may be specified only for " + "Lua language"); + } + if (tuple_field_count(tuple) > BOX_FUNC_FIELD_OPTS) { + const char *opts = tuple_field(tuple, BOX_FUNC_FIELD_OPTS); + if (opts_decode(&def->opts, func_opts_reg, &opts, + ER_WRONG_SPACE_OPTIONS, BOX_FUNC_FIELD_OPTS, + NULL) != 0) + diag_raise(); + } def_guard.is_active = false; return def; } /** Remove a function from function cache */ static void -func_cache_remove_func(struct trigger * /* trigger */, void *event) +func_cache_remove_func(struct trigger *trigger, void * /* event */) { - struct txn_stmt *stmt = txn_last_stmt((struct txn *) event); - uint32_t fid = tuple_field_u32_xc(stmt->old_tuple ? - stmt->old_tuple : stmt->new_tuple, - BOX_FUNC_FIELD_ID); - func_cache_delete(fid); + struct func *old_func = (struct func *) trigger->data; + func_cache_delete(old_func->def->fid); + func_delete(old_func); } /** Replace a function in the function cache */ static void -func_cache_replace_func(struct trigger * /* trigger */, void *event) +func_cache_replace_func(struct trigger *trigger, void * /* event */) { - struct txn_stmt *stmt = txn_last_stmt((struct txn*) event); - struct func_def *def = func_def_new_from_tuple(stmt->new_tuple); - auto def_guard = make_scoped_guard([=] { free(def); }); - func_cache_replace(def); - def_guard.is_active = false; + struct func *new_func = (struct func *) trigger->data; + struct func *old_func; + func_cache_replace(new_func, &old_func); + assert(old_func != NULL); + func_delete(old_func); } /** @@ -2477,13 +2508,20 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) struct func *old_func = func_by_id(fid); if (new_tuple != NULL && old_func == NULL) { /* INSERT */ struct func_def *def = func_def_new_from_tuple(new_tuple); + auto def_guard = make_scoped_guard([=] { free(def); }); access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION, PRIV_C); - auto def_guard = make_scoped_guard([=] { free(def); }); - func_cache_replace(def); + struct func *func = func_new(def); + if (func == NULL) + diag_raise(); + auto func_guard = make_scoped_guard([=] { func_delete(func); }); def_guard.is_active = false; + struct func *old_func = NULL; + func_cache_replace(func, &old_func); + assert(old_func == NULL); + func_guard.is_active = false; struct trigger *on_rollback = - txn_alter_trigger_new(func_cache_remove_func, NULL); + txn_alter_trigger_new(func_cache_remove_func, func); txn_on_rollback(txn, on_rollback); } else if (new_tuple == NULL) { /* DELETE */ uint32_t uid; @@ -2501,15 +2539,19 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event) "function has grants"); } struct trigger *on_commit = - txn_alter_trigger_new(func_cache_remove_func, NULL); + txn_alter_trigger_new(func_cache_remove_func, old_func); txn_on_commit(txn, on_commit); } else { /* UPDATE, REPLACE */ struct func_def *def = func_def_new_from_tuple(new_tuple); auto def_guard = make_scoped_guard([=] { free(def); }); access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION, PRIV_A); + struct func *func = func_new(def); + if (func == NULL) + diag_raise(); + def_guard.is_active = false; struct trigger *on_commit = - txn_alter_trigger_new(func_cache_replace_func, NULL); + txn_alter_trigger_new(func_cache_replace_func, func); txn_on_commit(txn, on_commit); } } diff --git a/src/box/call.c b/src/box/call.c index 56da53fb3..773b914b1 100644 --- a/src/box/call.c +++ b/src/box/call.c @@ -202,7 +202,7 @@ box_process_call(struct call_request *request, struct port *port) if (func && func->def->language == FUNC_LANGUAGE_C) { rc = box_c_call(func, request, port); } else { - rc = box_lua_call(request, port); + rc = box_lua_call(func, request, port); } /* Restore the original user */ if (orig_credentials) diff --git a/src/box/func.c b/src/box/func.c index a817851fd..d650b0144 100644 --- a/src/box/func.c +++ b/src/box/func.c @@ -34,6 +34,7 @@ #include "lua/utils.h" #include "error.h" #include "diag.h" +#include "fiber.h" #include <dlfcn.h> /** @@ -355,6 +356,49 @@ restore: return -1; } +/** + * Assemble a Lua function object on Lua stack and return + * the reference. + * Returns func object reference on success, LUA_REFNIL otherwise. + */ +static int +func_lua_code_load(struct func_def *def) +{ + int rc = LUA_REFNIL; + struct region *region = &fiber()->gc; + size_t region_svp = region_used(region); + struct lua_State *L = lua_newthread(tarantool_L); + int coro_ref = luaL_ref(tarantool_L, LUA_REGISTRYINDEX); + + /* + * Assemble a Lua function object with luaL_loadstring of + * special 'return FUNCTION_BODY' expression and call it. + * Set default sandbox to configure it to use only a + * limited number of functions and modules. + */ + const char *load_pref = "return "; + uint32_t load_str_sz = strlen(load_pref) + strlen(def->body) + 1; + char *load_str = region_alloc(region, load_str_sz); + if (load_str == NULL) { + diag_set(OutOfMemory, load_str_sz, "region", "load_str"); + goto end; + } + sprintf(load_str, "%s%s", load_pref, def->body); + if (luaL_loadstring(L, load_str) != 0 || + lua_pcall(L, 0, 1, 0) != 0 || !lua_isfunction(L, -1) || + luaT_get_sandbox(L) != 0) { + diag_set(ClientError, ER_LOAD_FUNCTION, def->name, + def->body); + goto end; + } + lua_setfenv(L, -2); + rc = luaL_ref(L, LUA_REGISTRYINDEX); +end: + region_truncate(region, region_svp); + luaL_unref(L, LUA_REGISTRYINDEX, coro_ref); + return rc; +} + struct func * func_new(struct func_def *def) { @@ -380,6 +424,15 @@ func_new(struct func_def *def) func->owner_credentials.auth_token = BOX_USER_MAX; /* invalid value */ func->func = NULL; func->module = NULL; + if (func->def->body != NULL) { + func->lua_func_ref = func_lua_code_load(def); + if (func->lua_func_ref == LUA_REFNIL) { + free(func); + return NULL; + } + } else { + func->lua_func_ref = LUA_REFNIL; + } return func; } @@ -395,8 +448,11 @@ func_unload(struct func *func) } module_gc(func->module); } + if (func->lua_func_ref != -1) + luaL_unref(tarantool_L, LUA_REGISTRYINDEX, func->lua_func_ref); func->module = NULL; func->func = NULL; + func->lua_func_ref = -1; } /** @@ -452,17 +508,18 @@ func_call(struct func *func, box_function_ctx_t *ctx, const char *args, } void -func_update(struct func *func, struct func_def *def) +func_delete(struct func *func) { func_unload(func); free(func->def); - func->def = def; + free(func); } void -func_delete(struct func *func) +func_capture_module(struct func *new_func, struct func *old_func) { - func_unload(func); - free(func->def); - free(func); + new_func->module = old_func->module; + new_func->func = old_func->func; + old_func->module = NULL; + old_func->func = NULL; } diff --git a/src/box/func.h b/src/box/func.h index 8dcd61d7b..eaf5a3902 100644 --- a/src/box/func.h +++ b/src/box/func.h @@ -65,6 +65,11 @@ struct func { * Anchor for module membership. */ struct rlist item; + /** + * The reference index of Lua function object. + * Is equal to LUA_REFNIL when undefined. + */ + int lua_func_ref; /** * For C functions, the body of the function. */ @@ -100,9 +105,6 @@ module_free(void); struct func * func_new(struct func_def *def); -void -func_update(struct func *func, struct func_def *def); - void func_delete(struct func *func); @@ -113,6 +115,15 @@ int func_call(struct func *func, box_function_ctx_t *ctx, const char *args, const char *args_end); +/** + * Take over the modules that belonged to the old function. + * Reset module and function pointers in old function. + * This helper allows you to inherit already loaded function + * objects, that can is useful on update. +*/ +void +func_capture_module(struct func *new_func, struct func *old_func); + /** * Reload dynamically loadable module. * diff --git a/src/box/func_def.c b/src/box/func_def.c index 76ed77b24..6c143f075 100644 --- a/src/box/func_def.c +++ b/src/box/func_def.c @@ -1,3 +1,12 @@ #include "func_def.h" +#include "opt_def.h" const char *func_language_strs[] = {"LUA", "C"}; + +const struct func_opts func_opts_default = { + /* .is_deterministic = */ false, +}; + +const struct opt_def func_opts_reg[] = { + OPT_DEF("is_deterministic", OPT_BOOL, struct func_opts, is_deterministic), +}; diff --git a/src/box/func_def.h b/src/box/func_def.h index 5b52ab498..6c35a684b 100644 --- a/src/box/func_def.h +++ b/src/box/func_def.h @@ -32,6 +32,7 @@ */ #include "trivia/util.h" +#include "opt_def.h" #include <stdbool.h> /** @@ -45,6 +46,19 @@ enum func_language { extern const char *func_language_strs[]; +/** Function options. */ +struct func_opts { + /** + * Whether the routine is deterministic (can produce + * only one result for a given list of parameters) + * or not. + */ + bool is_deterministic; +}; + +extern const struct func_opts func_opts_default; +extern const struct opt_def func_opts_reg[]; + /** * Definition of a function. Function body is not stored * or replicated (yet). @@ -54,6 +68,10 @@ struct func_def { uint32_t fid; /** Owner of the function. */ uint32_t uid; + /** Function name. */ + char *name; + /** Definition of the routine. */ + char *body; /** * True if the function requires change of user id before * invocation. @@ -63,8 +81,8 @@ struct func_def { * The language of the stored function. */ enum func_language language; - /** Function name. */ - char name[0]; + /** The function options. */ + struct func_opts opts; }; /** @@ -73,10 +91,20 @@ struct func_def { * for a function of length @a a name_len. */ static inline size_t -func_def_sizeof(uint32_t name_len) +func_def_sizeof(uint32_t name_len, uint32_t body_len) { /* +1 for '\0' name terminating. */ - return sizeof(struct func_def) + name_len + 1; + size_t sz = sizeof(struct func_def) + name_len + 1; + if (body_len > 0) + sz += body_len + 1; + return sz; +} + +/** Create index options using default values. */ +static inline void +func_opts_create(struct func_opts *opts) +{ + *opts = func_opts_default; } /** diff --git a/src/box/lua/call.c b/src/box/lua/call.c index c729778c4..dcd505cc3 100644 --- a/src/box/lua/call.c +++ b/src/box/lua/call.c @@ -32,6 +32,8 @@ #include "box/call.h" #include "box/error.h" #include "fiber.h" +#include "box/func.h" +#include "box/schema.h" #include "lua/utils.h" #include "lua/msgpack.h" @@ -253,6 +255,33 @@ execute_lua_call(lua_State *L) return lua_gettop(L); } +static int +execute_persistent_function(lua_State *L) +{ + struct call_request *request = (struct call_request *) + lua_topointer(L, 1); + lua_settop(L, 0); + + const char *name = request->name; + uint32_t name_len = mp_decode_strl(&name); + + struct func *func = func_by_name(name, name_len); + assert(func != NULL); + /* Construct Lua function if required. */ + assert(func->lua_func_ref != LUA_REFNIL); + lua_rawgeti(L, LUA_REGISTRYINDEX, func->lua_func_ref); + + /* Push the rest of args (a tuple). */ + const char *args = request->args; + uint32_t arg_count = mp_decode_array(&args); + luaL_checkstack(L, arg_count, "lua_func: out of stack"); + for (uint32_t i = 0; i < arg_count; i++) + luamp_decode(L, luaL_msgpack_default, &args); + + lua_call(L, arg_count, LUA_MULTRET); + return lua_gettop(L); +} + static int execute_lua_eval(lua_State *L) { @@ -396,9 +425,12 @@ box_process_lua(struct call_request *request, struct port *base, } int -box_lua_call(struct call_request *request, struct port *port) +box_lua_call(struct func *func, struct call_request *request, struct port *port) { - return box_process_lua(request, port, execute_lua_call); + if (func != NULL && func->def->body != NULL) + return box_process_lua(request, port, execute_persistent_function); + else + return box_process_lua(request, port, execute_lua_call); } int diff --git a/src/box/lua/call.h b/src/box/lua/call.h index 0542123da..d8ef65900 100644 --- a/src/box/lua/call.h +++ b/src/box/lua/call.h @@ -42,13 +42,15 @@ box_lua_call_init(struct lua_State *L); struct port; struct call_request; +struct func; /** * Invoke a Lua stored procedure from the binary protocol * (implementation of 'CALL' command code). */ int -box_lua_call(struct call_request *request, struct port *port); +box_lua_call(struct func *func, struct call_request *request, + struct port *port); int box_lua_eval(struct call_request *request, struct port *port); diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index f31cf7f2c..8cf3867f4 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1981,7 +1981,8 @@ box.schema.func.create = function(name, opts) opts = opts or {} check_param_table(opts, { setuid = 'boolean', if_not_exists = 'boolean', - language = 'string'}) + language = 'string', body = 'string', + opts = 'table'}) local _func = box.space[box.schema.FUNC_ID] local _vfunc = box.space[box.schema.VFUNC_ID] local func = _vfunc.index.name:get{name} @@ -1991,10 +1992,12 @@ box.schema.func.create = function(name, opts) end return end - opts = update_param_table(opts, { setuid = false, language = 'lua'}) + opts = update_param_table(opts, { setuid = false, language = 'lua', + body = '', opts = setmap{}}) opts.language = string.upper(opts.language) opts.setuid = opts.setuid and 1 or 0 - _func:auto_increment{session.euid(), name, opts.setuid, opts.language} + _func:auto_increment{session.euid(), name, opts.setuid, opts.language, + opts.body, opts.opts} end box.schema.func.drop = function(name, opts) diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua index 89d6e3d52..ef3d23c6b 100644 --- a/src/box/lua/upgrade.lua +++ b/src/box/lua/upgrade.lua @@ -324,7 +324,7 @@ local function initial_1_7_5() -- create "box.schema.user.info" function log.info('create function "box.schema.user.info" with setuid') - _func:replace{1, ADMIN, 'box.schema.user.info', 1, 'LUA'} + _func:replace{1, ADMIN, 'box.schema.user.info', 1, 'LUA', '', MAP} -- grant 'public' role access to 'box.schema.user.info' function log.info('grant execute on function "box.schema.user.info" to public') @@ -737,6 +737,24 @@ local function upgrade_to_2_1_3() end end +local function upgrade_to_2_2_0() + log.info("Update _func format") + local _func = box.space[box.schema.FUNC_ID] + local format = {} + format[1] = {name='id', type='unsigned'} + format[2] = {name='owner', type='unsigned'} + format[3] = {name='name', type='string'} + format[4] = {name='setuid', type='unsigned'} + format[5] = {name='language', type='string'} + format[6] = {name='body', type='string'} + format[7] = {name='opts', type='map'} + for _, v in box.space._func:pairs() do + _ = box.space._func:replace({v.id, v.owner, v.name, v.setuid, + v[5] or 'LUA', '', setmap({})}) + end + _func:format(format) +end + local function get_version() local version = box.space._schema:get{'version'} if version == nil then @@ -768,6 +786,7 @@ local function upgrade(options) {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true}, {version = mkversion(2, 1, 2), func = upgrade_to_2_1_2, auto = true}, {version = mkversion(2, 1, 3), func = upgrade_to_2_1_3, auto = true}, + {version = mkversion(2, 2, 0), func = upgrade_to_2_2_0, auto = true}, } for _, handler in ipairs(handlers) do diff --git a/src/box/schema.cc b/src/box/schema.cc index 9a55c2f14..b70992297 100644 --- a/src/box/schema.cc +++ b/src/box/schema.cc @@ -533,40 +533,35 @@ schema_free(void) } void -func_cache_replace(struct func_def *def) +func_cache_replace(struct func *new_func, struct func **old_func) { - struct func *old = func_by_id(def->fid); - if (old) { - func_update(old, def); - return; - } if (mh_size(funcs) >= BOX_FUNCTION_MAX) tnt_raise(ClientError, ER_FUNCTION_MAX, BOX_FUNCTION_MAX); - struct func *func = func_new(def); - if (func == NULL) { -error: - panic_syserror("Out of memory for the data " - "dictionary cache (stored function)."); - } - const struct mh_i32ptr_node_t node = { def->fid, func }; - mh_int_t k1 = mh_i32ptr_put(funcs, &node, NULL, NULL); - if (k1 == mh_end(funcs)) { - func->def = NULL; - func_delete(func); - goto error; - } - size_t def_name_len = strlen(func->def->name); - uint32_t name_hash = mh_strn_hash(func->def->name, def_name_len); + + mh_int_t k1, k2; + struct mh_i32ptr_node_t old_node, *old_node_ptr = &old_node; + const struct mh_i32ptr_node_t node = { new_func->def->fid, new_func }; + size_t def_name_len = strlen(new_func->def->name); + uint32_t name_hash = mh_strn_hash(new_func->def->name, def_name_len); const struct mh_strnptr_node_t strnode = { - func->def->name, def_name_len, name_hash, func }; + new_func->def->name, def_name_len, name_hash, new_func }; - mh_int_t k2 = mh_strnptr_put(funcs_by_name, &strnode, NULL, NULL); + k1 = mh_i32ptr_put(funcs, &node, &old_node_ptr, NULL); + if (k1 == mh_end(funcs)) + goto error; + if (old_node_ptr != NULL) + *old_func = (struct func *)old_node_ptr->val; + k2 = mh_strnptr_put(funcs_by_name, &strnode, NULL, NULL); if (k2 == mh_end(funcs_by_name)) { mh_i32ptr_del(funcs, k1, NULL); - func->def = NULL; - func_delete(func); goto error; } + if (*old_func != NULL) + func_capture_module(new_func, *old_func); + return; +error: + panic_syserror("Out of memory for the data dictionary cache " + "(stored function)."); } void @@ -582,7 +577,6 @@ func_cache_delete(uint32_t fid) strlen(func->def->name)); if (k != mh_end(funcs)) mh_strnptr_del(funcs_by_name, k, NULL); - func_delete(func); } struct func * diff --git a/src/box/schema.h b/src/box/schema.h index 6f9a96117..ffc41f401 100644 --- a/src/box/schema.h +++ b/src/box/schema.h @@ -163,14 +163,17 @@ schema_free(); struct space *schema_space(uint32_t id); /** - * Insert a new function or update the old one. - * - * @param def Function definition. In a case of success the ownership - * of @a def is transfered to the data dictionary, thus the caller - * must not delete it. + * Replace an existent (if any) function or insert a new one + * in function cache. + * @param func_new Function object to insert. In case of success, + * when old function object is exists, all loaded + * modules data are inherent with + * func_capture_module() when possible. + * @param func_old[out] The replaced function object if any. + * @retval Returns 0 on success, -1 otherwise. */ void -func_cache_replace(struct func_def *def); +func_cache_replace(struct func *new_func, struct func **old_func); void func_cache_delete(uint32_t fid); diff --git a/src/box/schema_def.h b/src/box/schema_def.h index eeeeb950b..8cabc7e24 100644 --- a/src/box/schema_def.h +++ b/src/box/schema_def.h @@ -163,6 +163,8 @@ enum { BOX_FUNC_FIELD_NAME = 2, BOX_FUNC_FIELD_SETUID = 3, BOX_FUNC_FIELD_LANGUAGE = 4, + BOX_FUNC_FIELD_BODY = 5, + BOX_FUNC_FIELD_OPTS = 6, }; /** _collation fields. */ diff --git a/src/lua/utils.c b/src/lua/utils.c index 27ff6b396..973d8b15c 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -46,6 +46,14 @@ static uint32_t CTID_STRUCT_IBUF_PTR; uint32_t CTID_CHAR_PTR; uint32_t CTID_CONST_CHAR_PTR; +static const char *default_sandbox_exports[] = + {"assert", "error", "ipairs", "math", "next", "pairs", "pcall", + "print", "select", "string", "table", "tonumber", "tostring", + "type", "unpack", "xpcall", "utf8"}; + +static int luaL_deepcopy_func_ref = LUA_REFNIL; +static int luaL_default_sandbox_ref = LUA_REFNIL; + void * luaL_pushcdata(struct lua_State *L, uint32_t ctypeid) { @@ -1248,6 +1256,65 @@ luaT_func_find(struct lua_State *L, const char *name, const char *name_end, return 0; } +/** + * Assemble a new sandbox with given exports table on top of the + * Lua stack. All modules in exports list are copying deeply + * to ensure the immutablility of this system object. + */ +static int +luaT_prepare_sandbox(struct lua_State *L, const char *exports[], + uint32_t exports_count) +{ + assert(luaL_deepcopy_func_ref != LUA_REFNIL); + lua_createtable(L, exports_count, 0); + for (unsigned i = 0; i < exports_count; i++) { + int count; + uint32_t name_len = strlen(exports[i]); + if (luaT_func_find(L, exports[i], exports[i] + name_len, + &count) != 0) + return -1; + switch (lua_type(L, -1)) { + case LUA_TTABLE: + lua_rawgeti(L, LUA_REGISTRYINDEX, + luaL_deepcopy_func_ref); + lua_insert(L, -2); + lua_call(L, 1, LUA_MULTRET); + FALLTHROUGH; + case LUA_TFUNCTION: + break; + default: + unreachable(); + } + lua_setfield(L, -2, exports[i]); + } + return 0; +} + +int +luaT_get_sandbox(struct lua_State *L) +{ + if (unlikely(luaL_deepcopy_func_ref == LUA_REFNIL)) { + int count; + const char *deepcopy = "table.deepcopy"; + if (luaT_func_find(L, deepcopy, deepcopy + strlen(deepcopy), + &count) != 0) + return -1; + luaL_deepcopy_func_ref = luaL_ref(L, LUA_REGISTRYINDEX); + assert(luaL_deepcopy_func_ref != LUA_REFNIL); + } + if (unlikely(luaL_default_sandbox_ref == LUA_REFNIL)) { + if (luaT_prepare_sandbox(L, default_sandbox_exports, + nelem(default_sandbox_exports)) != 0) + return -1; + luaL_default_sandbox_ref = luaL_ref(L, LUA_REGISTRYINDEX); + assert(luaL_default_sandbox_ref != LUA_REFNIL); + } + lua_rawgeti(L, LUA_REGISTRYINDEX, luaL_deepcopy_func_ref); + lua_rawgeti(L, LUA_REGISTRYINDEX, luaL_default_sandbox_ref); + lua_call(L, 1, LUA_MULTRET); + return 0; +} + int tarantool_lua_utils_init(struct lua_State *L) { diff --git a/src/lua/utils.h b/src/lua/utils.h index 81e936bee..36bb2e53b 100644 --- a/src/lua/utils.h +++ b/src/lua/utils.h @@ -603,6 +603,14 @@ int luaT_func_find(struct lua_State *L, const char *name, const char *name_end, int *count); +/** + * Prepare a new 'default' sandbox table on the top of the Lua + * stack. The 'default' sandbox consists of a minimum set of + * functions that are sufficient to serve persistent functions. + */ +int +luaT_get_sandbox(struct lua_State *L); + int tarantool_lua_utils_init(struct lua_State *L); diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result index 379f6c51f..ea8ae0984 100644 --- a/test/box-py/bootstrap.result +++ b/test/box-py/bootstrap.result @@ -4,7 +4,7 @@ box.internal.bootstrap() box.space._schema:select{} --- - - ['max_id', 511] - - ['version', 2, 1, 3] + - ['version', 2, 2, 0] ... box.space._cluster:select{} --- @@ -49,7 +49,7 @@ box.space._space:select{} 'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]] - [296, 1, '_func', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid', - 'type': 'unsigned'}]] + 'type': 'unsigned'}, {'name': 'lua_code', 'type': 'string'}]] - [297, 1, '_vfunc', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid', 'type': 'unsigned'}]] @@ -141,7 +141,7 @@ box.space._user:select{} ... box.space._func:select{} --- -- - [1, 1, 'box.schema.user.info', 1, 'LUA'] +- - [1, 1, 'box.schema.user.info', 1, 'LUA', ''] ... box.space._priv:select{} --- diff --git a/test/box/access_misc.result b/test/box/access_misc.result index 36ebfae09..514cfaab0 100644 --- a/test/box/access_misc.result +++ b/test/box/access_misc.result @@ -789,7 +789,7 @@ box.space._space:select() 'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]] - [296, 1, '_func', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid', - 'type': 'unsigned'}]] + 'type': 'unsigned'}, {'name': 'body', 'type': 'string'}, {'name': 'opts', 'type': 'map'}]] - [297, 1, '_vfunc', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'setuid', 'type': 'unsigned'}]] @@ -821,7 +821,7 @@ box.space._space:select() ... box.space._func:select() --- -- - [1, 1, 'box.schema.user.info', 1, 'LUA'] +- - [1, 1, 'box.schema.user.info', 1, 'LUA', ''] ... session = nil --- diff --git a/test/box/persistent_func.result b/test/box/persistent_func.result new file mode 100644 index 000000000..0aae0a4ee --- /dev/null +++ b/test/box/persistent_func.result @@ -0,0 +1,179 @@ +env = require('test_run') +--- +... +test_run = env.new() +--- +... +-- +-- gh-4182: Add persistent LUA functions. +-- +box.schema.user.grant('guest', 'execute', 'universe') +--- +... +net = require('net.box') +--- +... +conn = net.connect(box.cfg.listen) +--- +... +-- Test valid function. +test_run:cmd("setopt delimiter ';'") +--- +- true +... +body = [[function(tuple) + if type(tuple.address) ~= 'string' then return nil, 'Invalid field type' end + local t = tuple.address:lower():split() + for k,v in pairs(t) do t[k] = {v} end + return t +end +]] +test_run:cmd("setopt delimiter ''"); +--- +... +box.schema.func.create('test', {body = body, language = "C"}) +--- +- error: 'Failed to create function ''test'': function body may be specified only + for Lua language' +... +box.schema.func.create('test', {body = body}) +--- +... +box.schema.func.exists('test') +--- +- true +... +conn:call("test", {{address = "Moscow Dolgoprudny"}}) +--- +- [['moscow'], ['dolgoprudny']] +... +box.schema.func.create('test2', {body = body, opts = {is_deterministic = true}}) +--- +... +box.schema.func.create('test3', {body = body, opts = {is_deterministic = true, extra = true}}) +--- +- error: 'Wrong space options (field 6): unexpected option ''extra''' +... +-- Test that monkey-patch attack is not possible. +test_run:cmd("setopt delimiter ';'") +--- +- true +... +body_monkey = [[function(tuple) + math.abs = math.log + return tuple +end +]] +test_run:cmd("setopt delimiter ''"); +--- +... +box.schema.func.create('body_monkey', {body = body_monkey}) +--- +... +conn:call("body_monkey", {{address = "Moscow Dolgoprudny"}}) +--- +- {'address': 'Moscow Dolgoprudny'} +... +math.abs(-666.666) +--- +- 666.666 +... +-- Test taht 'require' is forbidden. +test_run:cmd("setopt delimiter ';'") +--- +- true +... +body_bad1 = [[function(tuple) + local json = require('json') + return json.encode(tuple) +end +]] +test_run:cmd("setopt delimiter ''"); +--- +... +box.schema.func.create('json_serializer', {body = body_bad1}) +--- +... +conn:call("json_serializer", {{address = "Moscow Dolgoprudny"}}) +--- +- error: '[string "return function(tuple) ..."]:1: attempt to call global ''require'' + (a nil value)' +... +-- Test function with spell error - case 1. +test_run:cmd("setopt delimiter ';'") +--- +- true +... +body_bad2 = [[function(tuple) + ret tuple +end +]] +test_run:cmd("setopt delimiter ''"); +--- +... +box.schema.func.create('body_bad2', {body = body_bad2}) +--- +- error: "Failed to dynamically load function 'body_bad2': function(tuple) \tret tuple + end " +... +-- Test function with spell error - case 2. +test_run:cmd("setopt delimiter ';'") +--- +- true +... +body_bad3 = [[func(tuple) + return tuple +end +]] +test_run:cmd("setopt delimiter ''"); +--- +... +box.schema.func.create('body_bad3', {body = body_bad3}) +--- +- error: "Failed to dynamically load function 'body_bad3': func(tuple) \treturn tuple + end " +... +conn:call("body_bad3", {{address = "Moscow Dolgoprudny"}}) +--- +- error: Procedure 'body_bad3' is not defined +... +conn:close() +--- +... +-- Restart server. +test_run:cmd("restart server default") +net = require('net.box') +--- +... +test_run = require('test_run').new() +--- +... +conn = net.connect(box.cfg.listen) +--- +... +conn:call("test", {{address = "Moscow Dolgoprudny"}}) +--- +- [['moscow'], ['dolgoprudny']] +... +conn:close() +--- +... +box.schema.func.drop('test') +--- +... +box.schema.func.exists('test') +--- +- false +... +box.schema.func.drop('body_monkey') +--- +... +box.schema.func.drop('json_serializer') +--- +... +box.schema.func.drop('test2') +--- +... +box.schema.user.revoke('guest', 'execute', 'universe') +--- +... diff --git a/test/box/persistent_func.test.lua b/test/box/persistent_func.test.lua new file mode 100644 index 000000000..7e321fb5c --- /dev/null +++ b/test/box/persistent_func.test.lua @@ -0,0 +1,83 @@ +env = require('test_run') +test_run = env.new() + +-- +-- gh-4182: Add persistent LUA functions. +-- +box.schema.user.grant('guest', 'execute', 'universe') +net = require('net.box') +conn = net.connect(box.cfg.listen) + +-- Test valid function. +test_run:cmd("setopt delimiter ';'") +body = [[function(tuple) + if type(tuple.address) ~= 'string' then return nil, 'Invalid field type' end + local t = tuple.address:lower():split() + for k,v in pairs(t) do t[k] = {v} end + return t +end +]] +test_run:cmd("setopt delimiter ''"); +box.schema.func.create('test', {body = body, language = "C"}) +box.schema.func.create('test', {body = body}) +box.schema.func.exists('test') +conn:call("test", {{address = "Moscow Dolgoprudny"}}) +box.schema.func.create('test2', {body = body, opts = {is_deterministic = true}}) +box.schema.func.create('test3', {body = body, opts = {is_deterministic = true, extra = true}}) + +-- Test that monkey-patch attack is not possible. +test_run:cmd("setopt delimiter ';'") +body_monkey = [[function(tuple) + math.abs = math.log + return tuple +end +]] +test_run:cmd("setopt delimiter ''"); +box.schema.func.create('body_monkey', {body = body_monkey}) +conn:call("body_monkey", {{address = "Moscow Dolgoprudny"}}) +math.abs(-666.666) + +-- Test taht 'require' is forbidden. +test_run:cmd("setopt delimiter ';'") +body_bad1 = [[function(tuple) + local json = require('json') + return json.encode(tuple) +end +]] +test_run:cmd("setopt delimiter ''"); +box.schema.func.create('json_serializer', {body = body_bad1}) +conn:call("json_serializer", {{address = "Moscow Dolgoprudny"}}) + +-- Test function with spell error - case 1. +test_run:cmd("setopt delimiter ';'") +body_bad2 = [[function(tuple) + ret tuple +end +]] +test_run:cmd("setopt delimiter ''"); +box.schema.func.create('body_bad2', {body = body_bad2}) + +-- Test function with spell error - case 2. +test_run:cmd("setopt delimiter ';'") +body_bad3 = [[func(tuple) + return tuple +end +]] +test_run:cmd("setopt delimiter ''"); +box.schema.func.create('body_bad3', {body = body_bad3}) +conn:call("body_bad3", {{address = "Moscow Dolgoprudny"}}) + +conn:close() +-- Restart server. +test_run:cmd("restart server default") +net = require('net.box') +test_run = require('test_run').new() +conn = net.connect(box.cfg.listen) +conn:call("test", {{address = "Moscow Dolgoprudny"}}) +conn:close() +box.schema.func.drop('test') +box.schema.func.exists('test') +box.schema.func.drop('body_monkey') +box.schema.func.drop('json_serializer') +box.schema.func.drop('test2') +box.schema.user.revoke('guest', 'execute', 'universe') -- 2.21.0
next prev parent reply other threads:[~2019-05-17 12:33 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-14 13:29 [PATCH v1 0/3] box: persistent Lua functions Kirill Shcherbatov 2019-05-14 13:29 ` [PATCH v1 1/3] box: refactor box_lua_find helper Kirill Shcherbatov 2019-05-17 12:33 ` [tarantool-patches] " Kirill Shcherbatov 2019-05-14 13:29 ` [PATCH v1 2/3] schema: extend _func space to persist lua functions Kirill Shcherbatov 2019-05-14 18:21 ` [tarantool-patches] " Konstantin Osipov 2019-05-17 12:33 ` Kirill Shcherbatov [this message] 2019-05-17 15:29 ` [tarantool-patches] " Konstantin Osipov 2019-05-14 13:29 ` [PATCH v1 3/3] box: extend box.schema.func with persistent folder Kirill Shcherbatov 2019-05-14 18:24 ` [tarantool-patches] " Konstantin Osipov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=92b6b7a0-7d8d-c239-6a40-b72f03f17e06@tarantool.org \ --to=kshcherbatov@tarantool.org \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --subject='Re: [tarantool-patches] Re: [PATCH v1 2/3] schema: extend _func space to persist lua functions' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox