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

@@ -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

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. */
 

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();

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);

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;
 }