Fix hincrby/hincrbyfloat on wrong value and float precision check

src/redis_hash.cc

@@ -1,5 +1,6 @@
 #include "redis_hash.h"
 #include <limits>
+#include <cmath>
 #include <iostream>
 #include <rocksdb/status.h>
 
@@ -50,14 +51,18 @@ rocksdb::Status Hash::IncrBy(const Slice &user_key, const Slice &field, int64_t
   InternalKey(ns_key, field, metadata.version).Encode(&sub_key);
   if (s.ok()) {
     std::string value_bytes;
+    std::size_t idx = 0;
     s = db_->Get(rocksdb::ReadOptions(), sub_key, &value_bytes);
     if (!s.ok() && !s.IsNotFound()) return s;
     if (s.ok()) {
       try {
-        old_value = std::stoll(value_bytes);
+        old_value = std::stoll(value_bytes, &idx);
       } catch (std::exception &e) {
         return rocksdb::Status::InvalidArgument(e.what());
       }
+      if (isspace(value_bytes[0]) || idx != value_bytes.size()) {
+        return rocksdb::Status::InvalidArgument("value is not an integer");
+      }
       exists = true;
     }
   }
@@ -96,23 +101,27 @@ rocksdb::Status Hash::IncrByFloat(const Slice &user_key, const Slice &field, dou
   InternalKey(ns_key, field, metadata.version).Encode(&sub_key);
   if (s.ok()) {
     std::string value_bytes;
+    std::size_t idx = 0;
     s = db_->Get(rocksdb::ReadOptions(), sub_key, &value_bytes);
     if (!s.ok() && !s.IsNotFound()) return s;
     if (s.ok()) {
       try {
-        old_value = std::stod(value_bytes);
+        old_value = std::stod(value_bytes, &idx);
       } catch (std::exception &e) {
         return rocksdb::Status::InvalidArgument(e.what());
       }
+      if (isspace(value_bytes[0]) || idx != value_bytes.size()) {
+        return rocksdb::Status::InvalidArgument("value is not an float");
+      }
       exists = true;
     }
   }
-  if ((increment < 0 && old_value < 0 && increment < (std::numeric_limits<double>::lowest()-old_value))
-      || (increment > 0 && old_value > 0 && increment > (std::numeric_limits<double>::max()-old_value))) {
-    return rocksdb::Status::InvalidArgument("increment or decrement would overflow");
+  double n = old_value + increment;
+  if (std::isinf(n) || std::isnan(n)) {
+    return rocksdb::Status::InvalidArgument("increment would produce NaN or Infinity");
   }
 
-  *ret = old_value + increment;
+  *ret = n;
   rocksdb::WriteBatch batch;
   WriteBatchLogData log_data(kRedisHash);
   batch.PutLogData(log_data.Encode());

tests/functional/hash_test.py

@@ -38,6 +38,12 @@ def test_hincrbyfloat():
         assert(is_double_eq(ret, i*1.234))
     ret = conn.delete(key)
     assert(ret == 1)
+    ret = conn.hincrbyfloat(key, "fi", 1.11)
+    assert(is_double_eq(ret, 1.11))
+    ret = conn.hincrbyfloat(key, "fi", -1.11)
+    assert(is_double_eq(ret, 0.0))
+    ret = conn.delete(key)
+    assert(ret == 1)
     # TODO(linty): not number of overflow case
     assert_raise(redis.RedisError, conn.hincrbyfloat, key, "f1", "invalid")