Skip to content

Scripting: forbid non deterministic WRITE commands#5208

Closed
soloestoy wants to merge 3 commits intoredis:unstablefrom
soloestoy:fix-lua-expire
Closed

Scripting: forbid non deterministic WRITE commands#5208
soloestoy wants to merge 3 commits intoredis:unstablefrom
soloestoy:fix-lua-expire

Conversation

@soloestoy
Copy link
Contributor

Some non deterministic WRITE commands like SPOP should not be allowed in lua script.

@soloestoy
Copy link
Contributor Author

soloestoy commented Aug 3, 2018

BTW, I think maybe we should take *expire* as random commands, because of issue #5206.

diff --git a/src/server.c b/src/server.c
index 66d42ee..28a9e9d 100644
--- a/src/server.c
+++ b/src/server.c
@@ -229,10 +229,10 @@ struct redisCommand redisCommandTable[] = {
     {"move",moveCommand,3,"wF",0,NULL,1,1,1,0,0},
     {"rename",renameCommand,3,"w",0,NULL,1,2,1,0,0},
     {"renamenx",renamenxCommand,3,"wF",0,NULL,1,2,1,0,0},
-    {"expire",expireCommand,3,"wF",0,NULL,1,1,1,0,0},
-    {"expireat",expireatCommand,3,"wF",0,NULL,1,1,1,0,0},
-    {"pexpire",pexpireCommand,3,"wF",0,NULL,1,1,1,0,0},
-    {"pexpireat",pexpireatCommand,3,"wF",0,NULL,1,1,1,0,0},
+    {"expire",expireCommand,3,"wRF",0,NULL,1,1,1,0,0},
+    {"expireat",expireatCommand,3,"wRF",0,NULL,1,1,1,0,0},
+    {"pexpire",pexpireCommand,3,"wRF",0,NULL,1,1,1,0,0},
+    {"pexpireat",pexpireatCommand,3,"wRF",0,NULL,1,1,1,0,0},
     {"keys",keysCommand,2,"rS",0,NULL,0,0,0,0,0},
     {"scan",scanCommand,-2,"rR",0,NULL,0,0,0,0,0},
     {"dbsize",dbsizeCommand,1,"rF",0,NULL,0,0,0,0,0},

But some user's lua script have to call redis.replicate_commands(), look forward to your suggestion @antirez .

@soloestoy
Copy link
Contributor Author

No need to take expire as random commands, just forbid deleting by expire from lua scripts is OK.

@antirez
Copy link
Contributor

antirez commented Sep 6, 2018

Thanks, I'll check this one today.

Copy link
Member

@oranagra oranagra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soloestoy i'm not sure i see the connection between the title (talking about SPOP) and the implementation (change in expireGenericCommand).
is that related to the change you did in luaRedisGenericCommand? can you please explain it to me?

@oranagra
Copy link
Member

oranagra commented May 4, 2021

For the record, this will implicitly be solved when we implement #8370 / #5292 in redis 7.0.

@oranagra
Copy link
Member

no longer relevant, script propagation was deprecated in #9812

@oranagra oranagra closed this Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants