This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 6baed12d fix(json): NumOp crash on large numbers (#2345)
6baed12d is described below
commit 6baed12dc7776c6a4a199993bd93d317f876fb2a
Author: Twice <[email protected]>
AuthorDate: Sat Jun 1 15:15:52 2024 +0900
fix(json): NumOp crash on large numbers (#2345)
---
src/types/json.h | 38 +++++++++++++++++---------------
src/types/redis_json.cc | 7 +++---
tests/cppunit/types/json_test.cc | 4 ++--
tests/gocase/unit/type/json/json_test.go | 3 +++
4 files changed, 28 insertions(+), 24 deletions(-)
diff --git a/src/types/json.h b/src/types/json.h
index fe651ccc..c9b681c4 100644
--- a/src/types/json.h
+++ b/src/types/json.h
@@ -21,6 +21,7 @@
#pragma once
#include <algorithm>
+#include <cmath>
#include <cstddef>
#include <jsoncons/json.hpp>
#include <jsoncons/json_error.hpp>
@@ -595,28 +596,29 @@ struct JsonValue {
if (!status.IsOK()) {
return;
}
- if (!origin.is_number()) {
+ // is_number() will return true
+ // if it's actually a string but can convert to a number
+ // so here we should exclude such case
+ if (!origin.is_number() || origin.is_string()) {
result->value.push_back(jsoncons::json::null());
return;
}
- if (number.value.is_double() || origin.is_double()) {
- double v = 0;
- if (op == NumOpEnum::Incr) {
- v = origin.as_double() + number.value.as_double();
- } else if (op == NumOpEnum::Mul) {
- v = origin.as_double() * number.value.as_double();
- }
- if (std::isinf(v)) {
- status = {Status::RedisExecErr, "result is an infinite number"};
- return;
- }
- origin = v;
+ double v = 0;
+ if (op == NumOpEnum::Incr) {
+ v = origin.as_double() + number.value.as_double();
+ } else if (op == NumOpEnum::Mul) {
+ v = origin.as_double() * number.value.as_double();
+ }
+ if (std::isinf(v)) {
+ status = {Status::RedisExecErr, "the result is an infinite number"};
+ return;
+ }
+ double v_int = 0;
+ if (std::modf(v, &v_int) == 0 &&
double(std::numeric_limits<int64_t>::min()) < v &&
+ v < double(std::numeric_limits<int64_t>::max())) {
+ origin = int64_t(v);
} else {
- if (op == NumOpEnum::Incr) {
- origin = origin.as_integer<int64_t>() +
number.value.as_integer<int64_t>();
- } else if (op == NumOpEnum::Mul) {
- origin = origin.as_integer<int64_t>() *
number.value.as_integer<int64_t>();
- }
+ origin = v;
}
result->value.push_back(origin);
});
diff --git a/src/types/redis_json.cc b/src/types/redis_json.cc
index 9929b22c..5f14b024 100644
--- a/src/types/redis_json.cc
+++ b/src/types/redis_json.cc
@@ -447,12 +447,11 @@ rocksdb::Status Json::NumMultBy(const std::string
&user_key, const std::string &
rocksdb::Status Json::numop(JsonValue::NumOpEnum op, const std::string
&user_key, const std::string &path,
const std::string &value, JsonValue *result) {
- JsonValue number;
auto number_res = JsonValue::FromString(value);
- if (!number_res || !number_res.GetValue().value.is_number()) {
- return rocksdb::Status::InvalidArgument("should be a number");
+ if (!number_res || !number_res.GetValue().value.is_number() ||
number_res.GetValue().value.is_string()) {
+ return rocksdb::Status::InvalidArgument("the input value should be a
number");
}
- number = std::move(number_res.GetValue());
+ JsonValue number = std::move(number_res.GetValue());
auto ns_key = AppendNamespacePrefix(user_key);
JsonMetadata metadata;
diff --git a/tests/cppunit/types/json_test.cc b/tests/cppunit/types/json_test.cc
index 40b8f2a1..30d6f60e 100644
--- a/tests/cppunit/types/json_test.cc
+++ b/tests/cppunit/types/json_test.cc
@@ -620,7 +620,7 @@ TEST_F(RedisJsonTest, NumMultBy) {
ASSERT_EQ(res.Print(0, true).GetValue(), "[2]");
res.value.clear();
ASSERT_TRUE(json_->NumMultBy(key_, "$.foo", "0.5", &res).ok());
- ASSERT_EQ(res.Print(0, true).GetValue(), "[1.0]");
+ ASSERT_EQ(res.Print(0, true).GetValue(), "[1]");
res.value.clear();
ASSERT_TRUE(json_->NumMultBy(key_, "$.bar", "1", &res).ok());
@@ -634,7 +634,7 @@ TEST_F(RedisJsonTest, NumMultBy) {
// num object
ASSERT_TRUE(json_->Set(key_, "$", "1.0").ok());
ASSERT_TRUE(json_->NumMultBy(key_, "$", "1", &res).ok());
- ASSERT_EQ(res.Print(0, true).GetValue(), "[1.0]");
+ ASSERT_EQ(res.Print(0, true).GetValue(), "[1]");
res.value.clear();
ASSERT_TRUE(json_->NumMultBy(key_, "$", "1.5", &res).ok());
ASSERT_EQ(res.Print(0, true).GetValue(), "[1.5]");
diff --git a/tests/gocase/unit/type/json/json_test.go
b/tests/gocase/unit/type/json/json_test.go
index e609d14f..8182db02 100644
--- a/tests/gocase/unit/type/json/json_test.go
+++ b/tests/gocase/unit/type/json/json_test.go
@@ -513,6 +513,9 @@ func TestJson(t *testing.T) {
EqualJSON(t, `[3]`, rdb.Do(ctx, "JSON.NUMINCRBY", "a", "$.foo",
2).Val())
EqualJSON(t, `[3.5]`, rdb.Do(ctx, "JSON.NUMINCRBY", "a",
"$.foo", 0.5).Val())
+ require.Error(t, rdb.Do(ctx, "JSON.NUMINCRBY", "a", "$.foo",
"9e99999").Err())
+ require.Error(t, rdb.Do(ctx, "JSON.NUMINCRBY", "a", "$.foo",
"999999999999999999999999999999").Err())
+
// wrong type
require.Equal(t, `[null]`, rdb.Do(ctx, "JSON.NUMINCRBY", "a",
"$.bar", 1).Val())
