From 0cdecca1418b0329ede2c43151912d35ea790a51 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 29 Mar 2012 12:02:28 +0200
Subject: [PATCH 01/10] Protect globals access in Lua scripting.
---
src/scripting.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/src/scripting.c b/src/scripting.c
index 0f876961..03938edd 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -412,6 +412,45 @@ void luaLoadLibraries(lua_State *lua) {
#endif
}
+void scriptingProtectGlobals(lua_State *lua) {
+ char *s[26];
+ sds code = sdsempty();
+ int j;
+
+ /* strict.lua from: http://metalua.luaforge.net/src/lib/strict.lua.html */
+ s[0]="local mt = getmetatable(_G)\n";
+ s[1]="if mt == nil then\n";
+ s[2]=" mt = {}\n";
+ s[3]=" setmetatable(_G, mt)\n";
+ s[4]="end\n";
+ s[5]="__STRICT = true\n";
+ s[6]="mt.__declared = {}\n";
+ s[7]="mt.__newindex = function (t, n, v)\n";
+ s[8]=" if __STRICT and not mt.__declared[n] and debug.getinfo(2) then\n";
+ s[9]=" local w = debug.getinfo(2, \"S\").what\n";
+ s[10]=" if w ~= \"main\" and w ~= \"C\" then\n";
+ s[11]=" error(\"assign to undeclared global var '\"..n..\"'\", 2)\n";
+ s[12]=" end\n";
+ s[13]=" mt.__declared[n] = true\n";
+ s[14]=" end\n";
+ s[15]=" rawset(t, n, v)\n";
+ s[16]="end\n";
+ s[17]="mt.__index = function (t, n)\n";
+ s[18]=" if debug.getinfo(2) and not mt.__declared[n] and debug.getinfo(2, \"S\").what ~= \"C\" then\n";
+ s[19]=" error(\"global var '\"..n..\"' is not declared\", 2)\n";
+ s[20]=" end\n";
+ s[21]=" return rawget(t, n)\n";
+ s[22]="end\n";
+ s[23]="function global(...)\n";
+ s[24]=" for _, v in ipairs{...} do mt.__declared[v] = true end\n";
+ s[25]="end\n";
+
+ for (j = 0; j < 26; j++) code = sdscatlen(code,s[j],strlen(s[j]));
+ luaL_loadbuffer(lua,code,sdslen(code),"strict_lua");
+ lua_pcall(lua,0,0,0);
+ sdsfree(code);
+}
+
/* Initialize the scripting environment.
* It is possible to call this function to reset the scripting environment
* assuming that we call scriptingRelease() before.
@@ -501,6 +540,11 @@ void scriptingInit(void) {
server.lua_client->flags |= REDIS_LUA_CLIENT;
}
+ /* 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. */
+ scriptingProtectGlobals(lua);
+
server.lua = lua;
}
From 37b29ef2fa2ce3e6bafcf1d8979504532ed8cc31 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 13 Apr 2012 11:16:50 +0200
Subject: [PATCH 02/10] Scripting: globals protection can now be switched
on/off.
---
redis.conf | 17 +++++++++++
src/config.c | 15 ++++++++++
src/redis.c | 1 +
src/redis.h | 3 ++
src/scripting.c | 79 ++++++++++++++++++++++++++++---------------------
5 files changed, 82 insertions(+), 33 deletions(-)
diff --git a/redis.conf b/redis.conf
index 1b79e09e..d2eae1af 100644
--- a/redis.conf
+++ b/redis.conf
@@ -392,6 +392,23 @@ 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 8dffe288..5aa807f2 100644
--- a/src/config.c
+++ b/src/config.c
@@ -311,6 +311,10 @@ 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)
{
@@ -552,6 +556,16 @@ 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;
@@ -735,6 +749,7 @@ 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 7ddeb886..089251b6 100644
--- a/src/redis.c
+++ b/src/redis.c
@@ -1068,6 +1068,7 @@ 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 5a3d1a9c..bd4ef5a1 100644
--- a/src/redis.h
+++ b/src/redis.h
@@ -717,6 +717,7 @@ 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;
@@ -1102,6 +1103,8 @@ 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 03938edd..138437cf 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -412,45 +412,57 @@ void luaLoadLibraries(lua_State *lua) {
#endif
}
-void scriptingProtectGlobals(lua_State *lua) {
- char *s[26];
+/* This function installs metamethods in the global table _G that prevent
+ * 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. */
+void scriptingEnableGlobalsProtection(lua_State *lua) {
+ char *s[32];
sds code = sdsempty();
- int j;
+ int j = 0;
- /* strict.lua from: http://metalua.luaforge.net/src/lib/strict.lua.html */
- s[0]="local mt = getmetatable(_G)\n";
- s[1]="if mt == nil then\n";
- s[2]=" mt = {}\n";
- s[3]=" setmetatable(_G, mt)\n";
- s[4]="end\n";
- s[5]="__STRICT = true\n";
- s[6]="mt.__declared = {}\n";
- s[7]="mt.__newindex = function (t, n, v)\n";
- s[8]=" if __STRICT and not mt.__declared[n] and debug.getinfo(2) then\n";
- s[9]=" local w = debug.getinfo(2, \"S\").what\n";
- s[10]=" if w ~= \"main\" and w ~= \"C\" then\n";
- s[11]=" error(\"assign to undeclared global var '\"..n..\"'\", 2)\n";
- s[12]=" end\n";
- s[13]=" mt.__declared[n] = true\n";
- s[14]=" end\n";
- s[15]=" rawset(t, n, v)\n";
- s[16]="end\n";
- s[17]="mt.__index = function (t, n)\n";
- s[18]=" if debug.getinfo(2) and not mt.__declared[n] and debug.getinfo(2, \"S\").what ~= \"C\" then\n";
- s[19]=" error(\"global var '\"..n..\"' is not declared\", 2)\n";
- s[20]=" end\n";
- s[21]=" return rawget(t, n)\n";
- s[22]="end\n";
- s[23]="function global(...)\n";
- s[24]=" for _, v in ipairs{...} do mt.__declared[v] = true end\n";
- s[25]="end\n";
+ /* strict.lua from: http://metalua.luaforge.net/src/lib/strict.lua.html.
+ * Modified to be adapted to Redis. */
+ s[j++]="mt = {}\n";
+ s[j++]="setmetatable(_G, mt)\n";
+ s[j++]="mt.declared = {}\n";
+ s[j++]="mt.__newindex = function (t, n, v)\n";
+ 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++]=" end\n";
+ s[j++]=" mt.declared[n] = true\n";
+ s[j++]=" end\n";
+ s[j++]=" rawset(t, n, v)\n";
+ 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++]=" end\n";
+ s[j++]=" return rawget(t, n)\n";
+ s[j++]="end\n";
+ s[j++]="function global(...)\n";
+ s[j++]=" for _, v in ipairs{...} do mt.declared[v] = true end\n";
+ s[j++]="end\n";
+ s[j++]=NULL;
- for (j = 0; j < 26; j++) code = sdscatlen(code,s[j],strlen(s[j]));
- luaL_loadbuffer(lua,code,sdslen(code),"strict_lua");
+ for (j = 0; s[j] != NULL; j++) code = sdscatlen(code,s[j],strlen(s[j]));
+ luaL_loadbuffer(lua,code,sdslen(code),"enable_strict_lua");
lua_pcall(lua,0,0,0);
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.
@@ -543,7 +555,8 @@ 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. */
- scriptingProtectGlobals(lua);
+ if (server.lua_protect_globals)
+ scriptingEnableGlobalsProtection(lua);
server.lua = lua;
}
From 2fd7c9efdedd81cfa2909ebbcdb30eb9b58d7094 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 13 Apr 2012 11:48:45 +0200
Subject: [PATCH 03/10] Tests for lua globals protection.
---
tests/unit/scripting.tcl | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index b9307cc1..091b4c43 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -219,6 +219,43 @@ start_server {tags {"scripting"}} {
list [r eval {return redis.sha1hex('')} 0] \
[r eval {return redis.sha1hex('Pizza & Mandolino')} 0]
} {da39a3ee5e6b4b0d3255bfef95601890afd80709 74822d82031af7493c20eefa13bd07ec4fada82f}
+
+ test {Globals protection reading an undeclared global variable} {
+ catch {r eval {return a} 0} e
+ set e
+ } {*ERR*global variable*not declared*}
+
+ test {Globals protection setting an undeclared global variable} {
+ catch {r eval {a=10} 0} e
+ set e
+ } {*ERR*assignment to undeclared*}
+
+ test {Globals protection bypassed using 'global' function} {
+ catch {r eval {global("a"); a=10; return a} 0} e
+ set e
+ } {10}
+
+ test {Globals protection can be disabled} {
+ r config set lua-protect-globals no
+ catch {r eval {b=20; return b} 0} e
+ set e
+ } {20}
+
+ test {Globals protection can be re-enabled} {
+ r config set lua-protect-globals yes
+ catch {r eval {c=30; return c} 0} e
+ set e
+ } {*ERR*assignment to undeclared*}
+
+ test {Globals protection 'global' function works with mutliple args} {
+ catch {r eval {
+ global("var1","var2")
+ var1=10
+ var2=20
+ return {var1,var2}
+ } 0 } e
+ set e
+ } {10 20}
}
start_server {tags {"scripting repl"}} {
From c9edd1b28ae429e6c34462917fcb5c9d616e0ef8 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 13 Apr 2012 12:13:02 +0200
Subject: [PATCH 04/10] Globals protection global() function modified for speed
and correctness.
---
src/scripting.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/scripting.c b/src/scripting.c
index 138437cf..44ceb1e2 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -427,7 +427,7 @@ void scriptingEnableGlobalsProtection(lua_State *lua) {
/* strict.lua from: http://metalua.luaforge.net/src/lib/strict.lua.html.
* Modified to be adapted to Redis. */
- s[j++]="mt = {}\n";
+ s[j++]="local mt = {}\n";
s[j++]="setmetatable(_G, mt)\n";
s[j++]="mt.declared = {}\n";
s[j++]="mt.__newindex = function (t, n, v)\n";
@@ -447,7 +447,11 @@ void scriptingEnableGlobalsProtection(lua_State *lua) {
s[j++]=" return rawget(t, n)\n";
s[j++]="end\n";
s[j++]="function global(...)\n";
- s[j++]=" for _, v in ipairs{...} do mt.declared[v] = true end\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;
From 6663653f515285aebe772663a24c381929c3e512 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 13 Apr 2012 13:26:59 +0200
Subject: [PATCH 05/10] 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 d2eae1af..1b79e09e 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 5aa807f2..8dffe288 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 089251b6..7ddeb886 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 bd4ef5a1..5a3d1a9c 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 44ceb1e2..ae906c68 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;
}
From d86c4a7bf0989f2466c3742a09a9c72633b5f0ba Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 13 Apr 2012 13:36:08 +0200
Subject: [PATCH 06/10] mt.declared is no longer needed.
Lua global protection can now be simpified becuase we no longer have the
global() function. It's useless to occupy memory with this table, it is
also not faster because the metamethods we use are only called when a
global object does not exist or we are trying to create it from a
script.
---
src/scripting.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/scripting.c b/src/scripting.c
index ae906c68..76b25ad2 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -426,19 +426,17 @@ void scriptingEnableGlobalsProtection(lua_State *lua) {
* Modified to be adapted to Redis. */
s[j++]="local mt = {}\n";
s[j++]="setmetatable(_G, mt)\n";
- s[j++]="mt.declared = {}\n";
s[j++]="mt.__newindex = function (t, n, v)\n";
- s[j++]=" if not mt.declared[n] and debug.getinfo(2) then\n";
+ s[j++]=" if 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(\"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";
s[j++]=" rawset(t, n, v)\n";
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++]=" if debug.getinfo(2) and debug.getinfo(2, \"S\").what ~= \"C\" then\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";
From 3cd4ad267c9aa2a94b9f901acd1e29b6653d981d Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 13 Apr 2012 13:40:57 +0200
Subject: [PATCH 07/10] Tests modified to match the new global protection
implementation.
---
tests/unit/scripting.tcl | 33 +++------------------------------
1 file changed, 3 insertions(+), 30 deletions(-)
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index 091b4c43..a9aae7b8 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -223,39 +223,12 @@ start_server {tags {"scripting"}} {
test {Globals protection reading an undeclared global variable} {
catch {r eval {return a} 0} e
set e
- } {*ERR*global variable*not declared*}
+ } {*ERR*attempted to access unexisting global*}
- test {Globals protection setting an undeclared global variable} {
+ test {Globals protection setting an undeclared global*} {
catch {r eval {a=10} 0} e
set e
- } {*ERR*assignment to undeclared*}
-
- test {Globals protection bypassed using 'global' function} {
- catch {r eval {global("a"); a=10; return a} 0} e
- set e
- } {10}
-
- test {Globals protection can be disabled} {
- r config set lua-protect-globals no
- catch {r eval {b=20; return b} 0} e
- set e
- } {20}
-
- test {Globals protection can be re-enabled} {
- r config set lua-protect-globals yes
- catch {r eval {c=30; return c} 0} e
- set e
- } {*ERR*assignment to undeclared*}
-
- test {Globals protection 'global' function works with mutliple args} {
- catch {r eval {
- global("var1","var2")
- var1=10
- var2=20
- return {var1,var2}
- } 0 } e
- set e
- } {10 20}
+ } {*ERR*attempted to create global*}
}
start_server {tags {"scripting repl"}} {
From 3a0214041542433895004f2fe97b08a3c4e9cb61 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 13 Apr 2012 14:54:49 +0200
Subject: [PATCH 08/10] Use Lua tostring() before concatenation.
---
src/scripting.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/scripting.c b/src/scripting.c
index 76b25ad2..c449714f 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -437,7 +437,7 @@ void scriptingEnableGlobalsProtection(lua_State *lua) {
s[j++]="end\n";
s[j++]="mt.__index = function (t, n)\n";
s[j++]=" if debug.getinfo(2) and debug.getinfo(2, \"S\").what ~= \"C\" then\n";
- s[j++]=" error(\"Script attempted to access unexisting global variable '\"..n..\"'\", 2)\n";
+ s[j++]=" error(\"Script attempted to access unexisting global variable '\"..tostring(n)..\"'\", 2)\n";
s[j++]=" end\n";
s[j++]=" return rawget(t, n)\n";
s[j++]="end\n";
From 6f659f34cf8aa22eb9977a2d38f7a6bd396c4b42 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 13 Apr 2012 15:12:16 +0200
Subject: [PATCH 09/10] EVAL errors are more clear now.
---
src/scripting.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/scripting.c b/src/scripting.c
index c449714f..4c7de33b 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -444,7 +444,7 @@ void scriptingEnableGlobalsProtection(lua_State *lua) {
s[j++]=NULL;
for (j = 0; s[j] != NULL; j++) code = sdscatlen(code,s[j],strlen(s[j]));
- luaL_loadbuffer(lua,code,sdslen(code),"enable_strict_lua");
+ luaL_loadbuffer(lua,code,sdslen(code),"@enable_strict_lua");
lua_pcall(lua,0,0,0);
sdsfree(code);
}
@@ -525,7 +525,7 @@ void scriptingInit(void) {
" if b == false then b = '' end\n"
" return a<b\n"
"end\n";
- luaL_loadbuffer(lua,compare_func,strlen(compare_func),"cmp_func_def");
+ luaL_loadbuffer(lua,compare_func,strlen(compare_func),"@cmp_func_def");
lua_pcall(lua,0,0,0);
}
@@ -676,7 +676,7 @@ int luaCreateFunction(redisClient *c, lua_State *lua, char *funcname, robj *body
funcdef = sdscatlen(funcdef,body->ptr,sdslen(body->ptr));
funcdef = sdscatlen(funcdef," end",4);
- if (luaL_loadbuffer(lua,funcdef,sdslen(funcdef),"func definition")) {
+ if (luaL_loadbuffer(lua,funcdef,sdslen(funcdef),"@user_script")) {
addReplyErrorFormat(c,"Error compiling script (new function): %s\n",
lua_tostring(lua,-1));
lua_pop(lua,1);
From 13a21caae348060f93a04df3603b69d4ba125056 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 13 Apr 2012 15:23:32 +0200
Subject: [PATCH 10/10] New test for scripting engine: DECR_IF_GT.
---
tests/unit/scripting.tcl | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index a9aae7b8..009c1347 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -229,6 +229,28 @@ start_server {tags {"scripting"}} {
catch {r eval {a=10} 0} e
set e
} {*ERR*attempted to create global*}
+
+ test {Test an example script DECR_IF_GT} {
+ set decr_if_gt {
+ local current
+
+ current = redis.call('get',KEYS[1])
+ if not current then return nil end
+ if current > ARGV[1] then
+ return redis.call('decr',KEYS[1])
+ else
+ return redis.call('get',KEYS[1])
+ end
+ }
+ r set foo 5
+ set res {}
+ lappend res [r eval $decr_if_gt 1 foo 2]
+ lappend res [r eval $decr_if_gt 1 foo 2]
+ lappend res [r eval $decr_if_gt 1 foo 2]
+ lappend res [r eval $decr_if_gt 1 foo 2]
+ lappend res [r eval $decr_if_gt 1 foo 2]
+ set res
+ } {4 3 2 2 2}
}
start_server {tags {"scripting repl"}} {