fix(zset): Properly convert non-c strings to floats.

The old code relied on c strtod function that expected some sort of delimiter
at the end. Coincidently my unit-testing code always passed proper c strings so
strod worked as expected. EVAL passes slices to non-c string and this is why the bug was
discovered via eval call. Fixes #148.

Signed-off-by: Roman Gershman <roman@dragonflydb.io>

src/redis/object.c

@@ -658,27 +658,6 @@ size_t stringObjectLen(robj *o) {
     }
 }
 
-int getDoubleFromObject(const robj *o, double *target) {
-    double value;
-
-    if (o == NULL) {
-        value = 0;
-    } else {
-        serverAssertWithInfo(NULL,o,o->type == OBJ_STRING);
-        if (sdsEncodedObject(o)) {
-            if (!string2d(o->ptr, sdslen(o->ptr), &value))
-                return C_ERR;
-        } else if (o->encoding == OBJ_ENCODING_INT) {
-            value = (long)o->ptr;
-        } else {
-            serverPanic("Unknown string encoding");
-        }
-    }
-    *target = value;
-    return C_OK;
-}
-
-
 // ROMAN: Copied from the DISABLED part below 
 int getLongLongFromObject(robj *o, long long *target) {
     long long value;

src/redis/util.c

@@ -513,27 +513,6 @@ int string2ld(const char *s, size_t slen, long double *dp) {
     return 1;
 }
 
-/* Convert a string into a double. Returns 1 if the string could be parsed
- * into a (non-overflowing) double, 0 otherwise. The value will be set to
- * the parsed value when appropriate.
- *
- * Note that this function demands that the string strictly represents
- * a double: no spaces or other characters before or after the string
- * representing the number are accepted. */
-int string2d(const char *s, size_t slen, double *dp) {
-    errno = 0;
-    char *eptr;
-    *dp = strtod(s, &eptr);
-    if (slen == 0 ||
-        isspace(((const char*)s)[0]) ||
-        (size_t)(eptr-(char*)s) != slen ||
-        (errno == ERANGE &&
-            (*dp == HUGE_VAL || *dp == -HUGE_VAL || *dp == 0)) ||
-        isnan(*dp))
-        return 0;
-    return 1;
-}
-
 /* Convert a double to a string representation. Returns the number of bytes
  * required. The representation should always be parsable by strtod(3).
  * This function does not support human-friendly formatting like ld2string

src/redis/util.h

@@ -60,7 +60,6 @@ int string2ll(const char *s, size_t slen, long long *value);
 int string2ull(const char *s, unsigned long long *value);
 int string2l(const char *s, size_t slen, long *value);
 int string2ld(const char *s, size_t slen, long double *dp);
-int string2d(const char *s, size_t slen, double *dp);
 int d2string(char *buf, size_t len, double value);
 int ld2string(char *buf, size_t len, long double value, ld2string_mode mode);
 

src/server/test_utils.cc

@@ -277,14 +277,21 @@ CmdArgVec BaseFamilyTest::TestConnWrapper::Args(ArgSlice list) {
   CHECK_NE(0u, list.size());
 
   CmdArgVec res;
+  string* str = new string;
+
+  // I compact all the arguments together on purpose.
+  // This way I check that arguments handling works well without c-string endings.
+  for (auto v : list) {
+    str->append(v);
+  }
+  tmp_str_vec_.emplace_back(str);
+  size_t offset = 0;
   for (auto v : list) {
     if (v.empty()) {
       res.push_back(MutableSlice{});
     } else {
-      tmp_str_vec_.emplace_back(new string{v});
+      res.emplace_back(str->data() + offset, v.size());
-      auto& s = *tmp_str_vec_.back();
+      offset += v.size();
-
-      res.emplace_back(s.data(), s.size());
     }
   }
 

src/server/zset_family.cc

@@ -11,7 +11,7 @@ extern "C" {
 #include "redis/zset.h"
 }
 
-#include <double-conversion/double-to-string.h>
+#include <absl/strings/charconv.h>
 
 #include "base/logging.h"
 #include "base/stl_util.h"
@@ -23,7 +23,6 @@ extern "C" {
 
 namespace dfly {
 
-using namespace double_conversion;
 using namespace std;
 using namespace facade;
 using absl::SimpleAtoi;
@@ -96,7 +95,7 @@ OpResult<PrimeIterator> FindZEntry(const ZParams& zparams, const OpArgs& op_args
 
   try {
     add_res = db_slice.AddOrFind(op_args.db_ind, key);
-  } catch(bad_alloc&) {
+  } catch (bad_alloc&) {
     return OpStatus::OUT_OF_MEMORY;
   }
 
@@ -541,7 +540,9 @@ bool ParseScore(string_view src, double* score) {
   } else if (src == "+inf") {
     *score = HUGE_VAL;
   } else {
-    return string2d(src.data(), src.size(), score);
+    absl::from_chars_result result = absl::from_chars(src.data(), src.end(), *score);
+    if (int(result.ec) != 0 || result.ptr != src.end() || isnan(*score))
+      return false;
   }
   return true;
 };

src/server/zset_family_test.cc

@@ -243,4 +243,9 @@ TEST_F(ZSetFamilyTest, ZInterStore) {
   EXPECT_THAT(resp.GetVec(), ElementsAre("b", "4"));
 }
 
+TEST_F(ZSetFamilyTest, ZAddBug148) {
+  auto resp = Run({"zadd", "key", "1", "9fe9f1eb"});
+  EXPECT_THAT(resp, IntArg(1));
+}
+
 }  // namespace dfly