From 6663653f515285aebe772663a24c381929c3e512 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 13 Apr 2012 13:26:59 +0200 Subject: [PATCH] Stop access to global vars. Not configurable. After considering the interaction between ability to delcare globals in scripts using the 'global' function, and the complexities related to hanlding replication and AOF in a sane way with globals AND ability to turn protection On and Off, we reconsidered the design. The new design makes clear that there is only one good way to write Redis scripts, that is not using globals. In the rare cases state must be retained across calls a Redis key can be used. --- redis.conf | 17 ----------------- src/config.c | 15 --------------- src/redis.c | 1 - src/redis.h | 3 --- src/scripting.c | 25 ++++--------------------- 5 files changed, 4 insertions(+), 57 deletions(-) diff --git a/redis.conf b/redis.conf index d2eae1af2..1b79e09ef 100644 --- a/redis.conf +++ b/redis.conf @@ -392,23 +392,6 @@ auto-aof-rewrite-min-size 64mb # Set it to 0 or a negative value for unlimited execution without warnings. lua-time-limit 5000 -# By default variables in a Lua script are global, this means that a correct -# script must declare all the local variables explicitly using the 'local' -# keyword. Lua beginners are known to violate this rule, polluting the global -# namespace, or creating scripts that may fail under certain conditions, for -# this reason by default Redis installs a protection that will raise an error -# every time a script attempts to access a global variable that was not -# explicitly declared via global(). -# -# It's worth to note that normal Redis scripts should never use globals, but -# we don't entirely disable the possibility because from time to time crazy -# things in the right hands can be pretty powerful. -# -# Globals protection may result into a minor performance hint, so it is -# possible to disable the feature in production environments using the -# following configuration directive, or at runtime using CONFIG SET. -lua-protect-globals yes - ################################ REDIS CLUSTER ############################### # # Normal Redis instances can't be part of a Redis Cluster, only nodes that are diff --git a/src/config.c b/src/config.c index 5aa807f2d..8dffe2887 100644 --- a/src/config.c +++ b/src/config.c @@ -311,10 +311,6 @@ void loadServerConfigFromString(char *config) { server.cluster.configfile = zstrdup(argv[1]); } else if (!strcasecmp(argv[0],"lua-time-limit") && argc == 2) { server.lua_time_limit = strtoll(argv[1],NULL,10); - } else if (!strcasecmp(argv[0],"lua-protect-globals") && argc == 2) { - if ((server.lua_protect_globals = yesnotoi(argv[1])) == -1) { - err = "argument must be 'yes' or 'no'"; goto loaderr; - } } else if (!strcasecmp(argv[0],"slowlog-log-slower-than") && argc == 2) { @@ -556,16 +552,6 @@ void configSetCommand(redisClient *c) { } else if (!strcasecmp(c->argv[2]->ptr,"lua-time-limit")) { if (getLongLongFromObject(o,&ll) == REDIS_ERR || ll < 0) goto badfmt; server.lua_time_limit = ll; - } else if (!strcasecmp(c->argv[2]->ptr,"lua-protect-globals")) { - int enable = yesnotoi(o->ptr); - - if (enable == -1) goto badfmt; - if (enable == 0 && server.lua_protect_globals == 1) { - scriptingDisableGlobalsProtection(server.lua); - } else if (enable && server.lua_protect_globals == 0) { - scriptingEnableGlobalsProtection(server.lua); - } - server.lua_protect_globals = enable; } else if (!strcasecmp(c->argv[2]->ptr,"slowlog-log-slower-than")) { if (getLongLongFromObject(o,&ll) == REDIS_ERR) goto badfmt; server.slowlog_log_slower_than = ll; @@ -749,7 +735,6 @@ void configGetCommand(redisClient *c) { config_get_bool_field("daemonize", server.daemonize); config_get_bool_field("rdbcompression", server.rdb_compression); config_get_bool_field("activerehashing", server.activerehashing); - config_get_bool_field("lua-protect-globals", server.lua_protect_globals); /* Everything we can't handle with macros follows. */ diff --git a/src/redis.c b/src/redis.c index 089251b68..7ddeb886c 100644 --- a/src/redis.c +++ b/src/redis.c @@ -1068,7 +1068,6 @@ void initServerConfig() { server.lua_time_limit = REDIS_LUA_TIME_LIMIT; server.lua_client = NULL; server.lua_timedout = 0; - server.lua_protect_globals = 1; updateLRUClock(); resetServerSaveParams(); diff --git a/src/redis.h b/src/redis.h index bd4ef5a11..5a3d1a9c5 100644 --- a/src/redis.h +++ b/src/redis.h @@ -717,7 +717,6 @@ struct redisServer { int lua_timedout; /* True if we reached the time limit for script execution. */ int lua_kill; /* Kill the script if true. */ - int lua_protect_globals; /* If true globals must be declared */ /* Assert & bug reportign */ char *assert_failed; char *assert_file; @@ -1103,8 +1102,6 @@ void clusterPropagatePublish(robj *channel, robj *message); /* Scripting */ void scriptingInit(void); -void scriptingEnableGlobalsProtection(lua_State *lua); -void scriptingDisableGlobalsProtection(lua_State *lua); /* Git SHA1 */ char *redisGitSHA1(void); diff --git a/src/scripting.c b/src/scripting.c index 44ceb1e28..ae906c68c 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -416,10 +416,7 @@ void luaLoadLibraries(lua_State *lua) { * the creation of globals accidentally. * * It should be the last to be called in the scripting engine initialization - * sequence, because it may interact with creation of globals. - * Note that the function is designed to be called multiple times if needed - * without issues, because it is possible to enabled/disable globals protection - * at runtime with CONFIG SET. */ + * sequence, because it may interact with creation of globals. */ void scriptingEnableGlobalsProtection(lua_State *lua) { char *s[32]; sds code = sdsempty(); @@ -434,7 +431,7 @@ void scriptingEnableGlobalsProtection(lua_State *lua) { s[j++]=" if not mt.declared[n] and debug.getinfo(2) then\n"; s[j++]=" local w = debug.getinfo(2, \"S\").what\n"; s[j++]=" if w ~= \"main\" and w ~= \"C\" then\n"; - s[j++]=" error(\"assignment to undeclared global variable '\"..n..\"'\", 2)\n"; + s[j++]=" error(\"Script attempted to create global variable '\"..tostring(n)..\"'\", 2)\n"; s[j++]=" end\n"; s[j++]=" mt.declared[n] = true\n"; s[j++]=" end\n"; @@ -442,17 +439,10 @@ void scriptingEnableGlobalsProtection(lua_State *lua) { s[j++]="end\n"; s[j++]="mt.__index = function (t, n)\n"; s[j++]=" if debug.getinfo(2) and not mt.declared[n] and debug.getinfo(2, \"S\").what ~= \"C\" then\n"; - s[j++]=" error(\"global variable '\"..n..\"' is not declared\", 2)\n"; + s[j++]=" error(\"Script attempted to access unexisting global variable '\"..n..\"'\", 2)\n"; s[j++]=" end\n"; s[j++]=" return rawget(t, n)\n"; s[j++]="end\n"; - s[j++]="function global(...)\n"; - s[j++]=" local nargs = select(\"#\",...)\n"; - s[j++]=" for i = 1, nargs do\n"; - s[j++]=" local v = select(i,...)\n"; - s[j++]=" mt.declared[v] = true\n"; - s[j++]=" end\n"; - s[j++]="end\n"; s[j++]=NULL; for (j = 0; s[j] != NULL; j++) code = sdscatlen(code,s[j],strlen(s[j])); @@ -461,12 +451,6 @@ void scriptingEnableGlobalsProtection(lua_State *lua) { sdsfree(code); } -void scriptingDisableGlobalsProtection(lua_State *lua) { - char *s = "setmetatable(_G, nil)\n"; - luaL_loadbuffer(lua,s,strlen(s),"disable_strict_lua"); - lua_pcall(lua,0,0,0); -} - /* Initialize the scripting environment. * It is possible to call this function to reset the scripting environment * assuming that we call scriptingRelease() before. @@ -559,8 +543,7 @@ void scriptingInit(void) { /* Lua beginners ofter don't use "local", this is likely to introduce * subtle bugs in their code. To prevent problems we protect accesses * to global variables. */ - if (server.lua_protect_globals) - scriptingEnableGlobalsProtection(lua); + scriptingEnableGlobalsProtection(lua); server.lua = lua; }