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 d2eae1af27eb2b35334f938d59721d3a2253f3dc..1b79e09efe1fde338145306304c96b922f0c4526 100644 (file)
--- 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 5aa807f2d40ddd54fe57c48fd50f3e2d6b39d3b0..8dffe28873d509ab50fc6d5ca1f98301502e270a 100644 (file)
--- 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 089251b685d55ca647735db5f9d0217574010755..7ddeb886c216e7235db83b3629b4da34b58644ac 100644 (file)
--- 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 bd4ef5a119b2947a844ed16bf4075af458db04bb..5a3d1a9c5f42f987f374b5f8bd74fec0ffa0f17f 100644 (file)
--- 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 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;
 }