You need to sign in or sign up before continuing.
Commit 2deb5551 authored by Guy Benoish's avatar Guy Benoish
Browse files

ld2string should fail if string contains \0 in the middle

This bug affected RM_StringToLongDouble and HINCRBYFLOAT.
I added tests for both cases.

Main changes:
1. Fixed string2ld to fail if string contains \0 in the middle
2. Use string2ld in getLongDoubleFromObject - No point of
   having duplicated code here

The two changes above broke RM_SaveLongDouble/RM_LoadLongDouble
because the long double string was saved with length+1 (An innocent
mistake, but it's actually a bug - The length passed to
RM_SaveLongDouble should not include the last \0).
parent bf53f928
...@@ -3901,7 +3901,7 @@ void RM_SaveLongDouble(RedisModuleIO *io, long double value) { ...@@ -3901,7 +3901,7 @@ void RM_SaveLongDouble(RedisModuleIO *io, long double value) {
/* Long double has different number of bits in different platforms, so we /* Long double has different number of bits in different platforms, so we
* save it as a string type. */ * save it as a string type. */
size_t len = ld2string(buf,sizeof(buf),value,LD_STR_HEX); size_t len = ld2string(buf,sizeof(buf),value,LD_STR_HEX);
RM_SaveStringBuffer(io,buf,len+1); /* len+1 for '\0' */ RM_SaveStringBuffer(io,buf,len);
} }
/* In the context of the rdb_save method of a module data type, loads back the /* In the context of the rdb_save method of a module data type, loads back the
......
...@@ -640,21 +640,13 @@ int getDoubleFromObjectOrReply(client *c, robj *o, double *target, const char *m ...@@ -640,21 +640,13 @@ int getDoubleFromObjectOrReply(client *c, robj *o, double *target, const char *m
int getLongDoubleFromObject(robj *o, long double *target) { int getLongDoubleFromObject(robj *o, long double *target) {
long double value; long double value;
char *eptr;
if (o == NULL) { if (o == NULL) {
value = 0; value = 0;
} else { } else {
serverAssertWithInfo(NULL,o,o->type == OBJ_STRING); serverAssertWithInfo(NULL,o,o->type == OBJ_STRING);
if (sdsEncodedObject(o)) { if (sdsEncodedObject(o)) {
errno = 0; if (!string2ld(o->ptr, sdslen(o->ptr), &value))
value = strtold(o->ptr, &eptr);
if (sdslen(o->ptr) == 0 ||
isspace(((const char*)o->ptr)[0]) ||
(size_t)(eptr-(char*)o->ptr) != sdslen(o->ptr) ||
(errno == ERANGE &&
(value == HUGE_VAL || value == -HUGE_VAL || value == 0)) ||
isnan(value))
return C_ERR; return C_ERR;
} else if (o->encoding == OBJ_ENCODING_INT) { } else if (o->encoding == OBJ_ENCODING_INT) {
value = (long)o->ptr; value = (long)o->ptr;
......
...@@ -471,13 +471,14 @@ int string2ld(const char *s, size_t slen, long double *dp) { ...@@ -471,13 +471,14 @@ int string2ld(const char *s, size_t slen, long double *dp) {
long double value; long double value;
char *eptr; char *eptr;
if (slen >= sizeof(buf)) return 0; if (slen == 0 || slen >= sizeof(buf)) return 0;
memcpy(buf,s,slen); memcpy(buf,s,slen);
buf[slen] = '\0'; buf[slen] = '\0';
errno = 0; errno = 0;
value = strtold(buf, &eptr); value = strtold(buf, &eptr);
if (isspace(buf[0]) || eptr[0] != '\0' || if (isspace(buf[0]) || eptr[0] != '\0' ||
(size_t)(eptr-buf) != slen ||
(errno == ERANGE && (errno == ERANGE &&
(value == HUGE_VAL || value == -HUGE_VAL || value == 0)) || (value == HUGE_VAL || value == -HUGE_VAL || value == 0)) ||
errno == EINVAL || errno == EINVAL ||
......
...@@ -74,6 +74,15 @@ int test_ld_conv(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { ...@@ -74,6 +74,15 @@ int test_ld_conv(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
RedisModule_ReplyWithError(ctx, err); RedisModule_ReplyWithError(ctx, err);
goto final; goto final;
} }
/* Make sure we can't convert a string that has \0 in it */
char buf[4] = "123";
buf[1] = '\0';
RedisModuleString *s3 = RedisModule_CreateString(ctx, buf, 3);
long double ld3;
if (RedisModule_StringToLongDouble(s3, &ld3) == REDISMODULE_OK) {
RedisModule_ReplyWithError(ctx, "Invalid string successfully converted to long double");
goto final;
}
RedisModule_ReplyWithLongDouble(ctx, ld2); RedisModule_ReplyWithLongDouble(ctx, ld2);
final: final:
RedisModule_FreeString(ctx, s1); RedisModule_FreeString(ctx, s1);
......
...@@ -390,6 +390,13 @@ start_server {tags {"hash"}} { ...@@ -390,6 +390,13 @@ start_server {tags {"hash"}} {
lappend rv [string match "ERR*not*float*" $bigerr] lappend rv [string match "ERR*not*float*" $bigerr]
} {1 1} } {1 1}
test {HINCRBYFLOAT fails against hash value that contains a null-terminator in the middle} {
r hset h f "1\x002"
catch {r hincrbyfloat h f 1} err
set rv {}
lappend rv [string match "ERR*not*float*" $err]
} {1}
test {HSTRLEN against the small hash} { test {HSTRLEN against the small hash} {
set err {} set err {}
foreach k [array names smallhash *] { foreach k [array names smallhash *] {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment