From e9b014cfacb443a0e828002d900a5a94a704d965 Mon Sep 17 00:00:00 2001 From: Mihir Joshi Date: Fri, 21 Nov 2014 22:35:42 -0500 Subject: [PATCH 1/2] stricter options for SET command Issue: #2157 As the SET command is parsed, it remembers which options are already set and if a duplicate option is found, raises an error because it is essentially an invalid syntax. It still allows mutually exclusive options like EX and PX because taking an option over another (precedence) is not essentially a syntactic error. --- src/t_string.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index 74c05793c..0bbefcaad 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -61,6 +61,8 @@ static int checkStringLength(redisClient *c, long long size) { #define REDIS_SET_NO_FLAGS 0 #define REDIS_SET_NX (1<<0) /* Set if key not exists. */ #define REDIS_SET_XX (1<<1) /* Set if key exists. */ +#define REDIS_SET_EX (1<<2) /* Set if time in seconds is given */ +#define REDIS_SET_PX (1<<3) /* Set if time in ms in given */ void setGenericCommand(redisClient *c, int flags, robj *key, robj *val, robj *expire, int unit, robj *ok_reply, robj *abort_reply) { long long milliseconds = 0; /* initialized to avoid any harmness warning */ @@ -101,19 +103,21 @@ void setCommand(redisClient *c) { char *a = c->argv[j]->ptr; robj *next = (j == c->argc-1) ? NULL : c->argv[j+1]; - if ((a[0] == 'n' || a[0] == 'N') && + if (!(flags & REDIS_SET_NX) && (a[0] == 'n' || a[0] == 'N') && (a[1] == 'x' || a[1] == 'X') && a[2] == '\0') { flags |= REDIS_SET_NX; - } else if ((a[0] == 'x' || a[0] == 'X') && + } else if (!(flags & REDIS_SET_XX) && (a[0] == 'x' || a[0] == 'X') && (a[1] == 'x' || a[1] == 'X') && a[2] == '\0') { flags |= REDIS_SET_XX; - } else if ((a[0] == 'e' || a[0] == 'E') && + } else if (!(flags & REDIS_SET_EX) && (a[0] == 'e' || a[0] == 'E') && (a[1] == 'x' || a[1] == 'X') && a[2] == '\0' && next) { + flags |= REDIS_SET_EX; unit = UNIT_SECONDS; expire = next; j++; - } else if ((a[0] == 'p' || a[0] == 'P') && + } else if (!(flags & REDIS_SET_PX) && (a[0] == 'p' || a[0] == 'P') && (a[1] == 'x' || a[1] == 'X') && a[2] == '\0' && next) { + flags |= REDIS_SET_PX; unit = UNIT_MILLISECONDS; expire = next; j++; From 352172a7ef5015c0c487ba6258cdf3b4b31a551c Mon Sep 17 00:00:00 2001 From: Mihir Joshi Date: Sun, 14 Dec 2014 10:12:58 -0500 Subject: [PATCH 2/2] Stricter options for SET command - As per Antirez's suggestion, this commit raises an error when mutually exclusive options are provided. Duplicate options are allowed. --- src/t_string.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index 0bbefcaad..067aa10e3 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -103,20 +103,24 @@ void setCommand(redisClient *c) { char *a = c->argv[j]->ptr; robj *next = (j == c->argc-1) ? NULL : c->argv[j+1]; - if (!(flags & REDIS_SET_NX) && (a[0] == 'n' || a[0] == 'N') && - (a[1] == 'x' || a[1] == 'X') && a[2] == '\0') { + if ((a[0] == 'n' || a[0] == 'N') && + (a[1] == 'x' || a[1] == 'X') && a[2] == '\0' && + !(flags & REDIS_SET_XX)) { flags |= REDIS_SET_NX; - } else if (!(flags & REDIS_SET_XX) && (a[0] == 'x' || a[0] == 'X') && - (a[1] == 'x' || a[1] == 'X') && a[2] == '\0') { + } else if ((a[0] == 'x' || a[0] == 'X') && + (a[1] == 'x' || a[1] == 'X') && a[2] == '\0' && + !(flags & REDIS_SET_NX)) { flags |= REDIS_SET_XX; - } else if (!(flags & REDIS_SET_EX) && (a[0] == 'e' || a[0] == 'E') && - (a[1] == 'x' || a[1] == 'X') && a[2] == '\0' && next) { + } else if ((a[0] == 'e' || a[0] == 'E') && + (a[1] == 'x' || a[1] == 'X') && a[2] == '\0' && + !(flags & REDIS_SET_PX) && next) { flags |= REDIS_SET_EX; unit = UNIT_SECONDS; expire = next; j++; - } else if (!(flags & REDIS_SET_PX) && (a[0] == 'p' || a[0] == 'P') && - (a[1] == 'x' || a[1] == 'X') && a[2] == '\0' && next) { + } else if ((a[0] == 'p' || a[0] == 'P') && + (a[1] == 'x' || a[1] == 'X') && a[2] == '\0' && + !(flags & REDIS_SET_EX) && next) { flags |= REDIS_SET_PX; unit = UNIT_MILLISECONDS; expire = next;