gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Fix RM_ScanKey module api not to return int encoded strings

Browse Source 
The scan key module API provides the scan callback with the current
field name and value (if it exists). Those arguments are RedisModuleString*
which means it supposes to point to robj which is encoded as a string.
Using createStringObjectFromLongLong function might return robj that
points to an integer and so break a module that tries for example to
use RedisModule_StringPtrLen on the given field/value.

The PR introduces a fix that uses the createObject function and sdsfromlonglong function.
Using those function promise that the field and value pass to the to the
scan callback will be Strings.

The PR also changes the Scan test module to use RedisModule_StringPtrLen
to catch the issue. without this, the issue is hidden because
RedisModule_ReplyWithString knows to handle integer encoding of the
given robj (RedisModuleString).

The PR also introduces a new test to verify the issue is solved.
This commit is contained in:
meir@redislabs.com 2020-06-14 10:06:00 +03:00
parent 1055398849
commit a89bf734a9
3 changed files with 24 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/module.c
View File

@ -6708,7 +6708,7 @@ int RM_ScanKey(RedisModuleKey *key, RedisModuleScanCursor *cursor, RedisModuleSc
int pos = 0;
int64_t ll;
while(intsetGet(o->ptr,pos++,&ll)) {
robj *field = createStringObjectFromLongLong(ll);
robj *field = createObject(OBJ_STRING,sdsfromlonglong(ll));
fn(key, field, NULL, privdata);
decrRefCount(field);
}
@ -6724,12 +6724,12 @@ int RM_ScanKey(RedisModuleKey *key, RedisModuleScanCursor *cursor, RedisModuleSc
ziplistGet(p,&vstr,&vlen,&vll);
robj *field = (vstr != NULL) ?
createStringObject((char*)vstr,vlen) :
createStringObjectFromLongLong(vll);
createObject(OBJ_STRING,sdsfromlonglong(vll));
p = ziplistNext(o->ptr,p);
ziplistGet(p,&vstr,&vlen,&vll);
robj *value = (vstr != NULL) ?
createStringObject((char*)vstr,vlen) :
createStringObjectFromLongLong(vll);
createObject(OBJ_STRING,sdsfromlonglong(vll));
fn(key, field, value, privdata);
p = ziplistNext(o->ptr,p);
decrRefCount(field);

20
tests/modules/scan.c
View File

@ -55,11 +55,23 @@ void scan_key_callback(RedisModuleKey *key, RedisModuleString* field, RedisModul
REDISMODULE_NOT_USED(key);
scan_key_pd* pd = privdata;
RedisModule_ReplyWithArray(pd->ctx, 2);
RedisModule_ReplyWithString(pd->ctx, field);
if (value)
RedisModule_ReplyWithString(pd->ctx, value);
else
size_t fieldCStrLen;
// The implementation of RedisModuleString is robj with lots of encodings.
// We want to make sure the robj that passes to this callback in
// String encoded, this is why we use RedisModule_StringPtrLen and
// RedisModule_ReplyWithStringBuffer instead of directly use
// RedisModule_ReplyWithString.
const char* fieldCStr = RedisModule_StringPtrLen(field, &fieldCStrLen);
RedisModule_ReplyWithStringBuffer(pd->ctx, fieldCStr, fieldCStrLen);
if(value){
size_t valueCStrLen;
const char* valueCStr = RedisModule_StringPtrLen(value, &valueCStrLen);
RedisModule_ReplyWithStringBuffer(pd->ctx, valueCStr, valueCStrLen);
} else {
RedisModule_ReplyWithNull(pd->ctx);
}
pd->nreplies++;
}

5
tests/unit/moduleapi/scan.tcl
View File

@ -16,6 +16,11 @@ start_server {tags {"modules"}} {
r hmset hh f1 v1 f2 v2
lsort [r scan.scan_key hh]
} {{f1 v1} {f2 v2}}
test {Module scan hash dict with int value} {
r hmset hh1 f1 1
lsort [r scan.scan_key hh1]
} {{f1 1}}
test {Module scan hash dict} {
r config set hash-max-ziplist-entries 2