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.

diff --git a/redis.conf b/redis.conf
index d48101db3f8fcedc7f71446ee2be32414d655712..85220b80deb9b4dce1b400946a6d221d57f68aa1 100644 (file)
--- a/redis.conf
+++ b/redis.conf
@@ -401,23 +401,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
-
 ################################## SLOW LOG ###################################
 
 # The Redis Slow Log is a system to log queries that exceeded a specified

diff --git a/src/config.c b/src/config.c
index a45e5108e1c5463e1fcba63aee7558570f36d846..4d35521d742b4f5fd18407aed0caa60c168a4cec 100644 (file)
--- a/src/config.c
+++ b/src/config.c
@@ -308,10 +308,6 @@ void loadServerConfigFromString(char *config) {
             }
         } 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)
         {
@@ -553,16 +549,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;
@@ -757,7 +743,6 @@ void configGetCommand(redisClient *c) {
     config_get_bool_field("rdbcompression", server.rdb_compression);
     config_get_bool_field("rdbchecksum", server.rdb_checksum);
     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 9966f945a86ab22229147bc52d7c38c5e3dde490..86cf0d957e71992f67110f30e6b8d15118cde82b 100644 (file)
--- a/src/redis.c
+++ b/src/redis.c
@@ -1067,7 +1067,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 7d1cc7ddf86b0f6d672f9d268d81220a0ac686cf..9702010e2de99281159983ab0b3123b24e0bf585 100644 (file)
--- a/src/redis.h
+++ b/src/redis.h
@@ -585,7 +585,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;
@@ -961,8 +960,6 @@ int *zunionInterGetKeys(struct redisCommand *cmd,robj **argv, int argc, int *num
 
 /* 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 44ceb1e284ccd03f690b6fa3cc7eeeea94f6696f..ae906c68cc4a3157b6ebb70698635fcdb45e76b0 100644 (file)
--- 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;
 }