From bee2c1010c35814d2490a4aa66cdd95ae4ba361a Mon Sep 17 00:00:00 2001 From: YaalLek <140312422+YaalLek@users.noreply.github.com> Date: Tue, 21 Nov 2023 17:19:25 +0200 Subject: [PATCH 1/5] json_value.cpp bug in the edges of uint/int Fixing bug of sending a number that is a bit bigger than max it returns 0: https://stackoverflow.com/questions/77261400/jsoncpp-do-not-protect-from-uint64-overflow-and-have-weird-behavior/77261716#77261716 --- src/lib_json/json_value.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index aa2b744ca..19c187ad4 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -87,7 +87,8 @@ template static inline bool InRange(double d, T min, U max) { // The casts can lose precision, but we are looking only for // an approximate range. Might fail on edge cases though. ~cdunn - return d >= static_cast(min) && d <= static_cast(max); + return d >= static_cast(min) && d <= static_cast(max) && + !(static_cast(d) != min || d == min); } #else // if !defined(JSON_USE_INT64_DOUBLE_CONVERSION) static inline double integerToDouble(Json::UInt64 value) { @@ -101,7 +102,8 @@ template static inline double integerToDouble(T value) { template static inline bool InRange(double d, T min, U max) { - return d >= integerToDouble(min) && d <= integerToDouble(max); + return d >= integerToDouble(min) && d <= integerToDouble(max) && + !(static_cast(d) == min && d != integerToDouble(min)); } #endif // if !defined(JSON_USE_INT64_DOUBLE_CONVERSION) From db53e6d939436bdff3b54ecb1c15330074fa1c09 Mon Sep 17 00:00:00 2001 From: YaalLek <140312422+YaalLek@users.noreply.github.com> Date: Tue, 21 Nov 2023 17:27:58 +0200 Subject: [PATCH 2/5] Update json_value.cpp Fixing bug of sending a number that is a bit bigger than max it returns 0: https://stackoverflow.com/questions/77261400/jsoncpp-do-not-protect-from-uint64-overflow-and-have-weird-behavior/77261716#77261716 --- src/lib_json/json_value.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index 19c187ad4..7b8349717 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -103,7 +103,7 @@ template static inline double integerToDouble(T value) { template static inline bool InRange(double d, T min, U max) { return d >= integerToDouble(min) && d <= integerToDouble(max) && - !(static_cast(d) == min && d != integerToDouble(min)); + !(static_cast(d) == min || d != integerToDouble(min)); } #endif // if !defined(JSON_USE_INT64_DOUBLE_CONVERSION) From e040b981d3fe09f6ac8beb55f614553a17d37b13 Mon Sep 17 00:00:00 2001 From: YaalLek <140312422+YaalLek@users.noreply.github.com> Date: Wed, 22 Nov 2023 17:29:42 +0200 Subject: [PATCH 3/5] Update test cases --- src/test_lib_json/main.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index a6f21c45a..cc2b7f8c7 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -1182,15 +1182,13 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, integers) { JSONTEST_ASSERT_EQUAL(true, val.asBool()); JSONTEST_ASSERT_STRING_EQUAL("-9223372036854775808", val.asString()); - // int64 min (floating point constructor). Note that kint64min *is* exactly - // representable as a double. + // int64 min (floating point constructor). Since double values in proximity of kint64min + // are rounded to kint64min, we don't check for conversion to int64. val = Json::Value(double(kint64min)); JSONTEST_ASSERT_EQUAL(Json::realValue, val.type()); checks = IsCheck(); - checks.isInt64_ = true; - checks.isIntegral_ = true; checks.isDouble_ = true; checks.isNumeric_ = true; JSONTEST_ASSERT_PRED(checkIs(val, checks)); @@ -1199,8 +1197,6 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, integers) { JSONTEST_ASSERT(!val.isConvertibleTo(Json::intValue)); JSONTEST_ASSERT(!val.isConvertibleTo(Json::uintValue)); - JSONTEST_ASSERT_EQUAL(kint64min, val.asInt64()); - JSONTEST_ASSERT_EQUAL(kint64min, val.asLargestInt()); JSONTEST_ASSERT_EQUAL(-9223372036854775808.0, val.asDouble()); JSONTEST_ASSERT_EQUAL(-9223372036854775808.0, val.asFloat()); JSONTEST_ASSERT_EQUAL(true, val.asBool()); From 8093d8c47c25c38dcc2f9dba3e2eb680d8a1d7a5 Mon Sep 17 00:00:00 2001 From: YaalLek <140312422+YaalLek@users.noreply.github.com> Date: Wed, 22 Nov 2023 17:33:24 +0200 Subject: [PATCH 4/5] json_value.cpp bug in the edges of uint/int Fixing bug of sending a number that is a bit bigger than max it returns 0: https://stackoverflow.com/questions/77261400/jsoncpp-do-not-protect-from-uint64-overflow-and-have-weird-behavior/77261716#77261716 --- src/lib_json/json_value.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index 7b8349717..4ad67c8ad 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -88,7 +88,7 @@ static inline bool InRange(double d, T min, U max) { // The casts can lose precision, but we are looking only for // an approximate range. Might fail on edge cases though. ~cdunn return d >= static_cast(min) && d <= static_cast(max) && - !(static_cast(d) != min || d == min); + !(static_cast(d) == min && d != static_cast(min)); } #else // if !defined(JSON_USE_INT64_DOUBLE_CONVERSION) static inline double integerToDouble(Json::UInt64 value) { @@ -103,7 +103,7 @@ template static inline double integerToDouble(T value) { template static inline bool InRange(double d, T min, U max) { return d >= integerToDouble(min) && d <= integerToDouble(max) && - !(static_cast(d) == min || d != integerToDouble(min)); + !(static_cast(d) == min && d != integerToDouble(min)); } #endif // if !defined(JSON_USE_INT64_DOUBLE_CONVERSION) @@ -707,6 +707,10 @@ Value::Int64 Value::asInt64() const { JSON_ASSERT_MESSAGE(isInt64(), "LargestUInt out of Int64 range"); return Int64(value_.uint_); case realValue: + // If the double value is in proximity to minInt64, it will be rounded to minInt64. + // The correct value in this scenario is indeterminable + JSON_ASSERT_MESSAGE(value_.real_ != minInt64, + "Double value is minInt64, precise value cannot be determined"); JSON_ASSERT_MESSAGE(InRange(value_.real_, minInt64, maxInt64), "double out of Int64 range"); return Int64(value_.real_); @@ -1310,7 +1314,10 @@ bool Value::isInt64() const { // Note that maxInt64 (= 2^63 - 1) is not exactly representable as a // double, so double(maxInt64) will be rounded up to 2^63. Therefore we // require the value to be strictly less than the limit. - return value_.real_ >= double(minInt64) && + // minInt64 is -2^63 which can be represented as a double, but since double values in its proximity are also + // rounded to -2^63, we require the value to be strictly greater than the limit to avoid returning + // 'true' for values that are not in the range + return value_.real_ > double(minInt64) && value_.real_ < double(maxInt64) && IsIntegral(value_.real_); default: break; @@ -1349,7 +1356,10 @@ bool Value::isIntegral() const { // Note that maxUInt64 (= 2^64 - 1) is not exactly representable as a // double, so double(maxUInt64) will be rounded up to 2^64. Therefore we // require the value to be strictly less than the limit. - return value_.real_ >= double(minInt64) && + // minInt64 is -2^63 which can be represented as a double, but since double values in its proximity are also + // rounded to -2^63, we require the value to be strictly greater than the limit to avoid returning + // 'true' for values that are not in the range + return value_.real_ > double(minInt64) && value_.real_ < maxUInt64AsDouble && IsIntegral(value_.real_); #else return value_.real_ >= minInt && value_.real_ <= maxUInt && From 298ca1eab4ddf75d53d13d766c6240deebcd39c9 Mon Sep 17 00:00:00 2001 From: YaalLek Date: Tue, 10 Sep 2024 10:12:27 +0300 Subject: [PATCH 5/5] Run clang tidy --- src/lib_json/json_value.cpp | 29 ++++++++++++++++------------- src/test_lib_json/main.cpp | 4 ++-- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index f4735e845..72ba8e59f 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -88,7 +88,7 @@ static inline bool InRange(double d, T min, U max) { // The casts can lose precision, but we are looking only for // an approximate range. Might fail on edge cases though. ~cdunn return d >= static_cast(min) && d <= static_cast(max) && - !(static_cast(d) == min && d != static_cast(min)); + !(static_cast(d) == min && d != static_cast(min)); } #else // if !defined(JSON_USE_INT64_DOUBLE_CONVERSION) static inline double integerToDouble(Json::UInt64 value) { @@ -707,10 +707,11 @@ Value::Int64 Value::asInt64() const { JSON_ASSERT_MESSAGE(isInt64(), "LargestUInt out of Int64 range"); return Int64(value_.uint_); case realValue: - // If the double value is in proximity to minInt64, it will be rounded to minInt64. - // The correct value in this scenario is indeterminable - JSON_ASSERT_MESSAGE(value_.real_ != minInt64, - "Double value is minInt64, precise value cannot be determined"); + // If the double value is in proximity to minInt64, it will be rounded to + // minInt64. The correct value in this scenario is indeterminable + JSON_ASSERT_MESSAGE( + value_.real_ != minInt64, + "Double value is minInt64, precise value cannot be determined"); JSON_ASSERT_MESSAGE(InRange(value_.real_, minInt64, maxInt64), "double out of Int64 range"); return Int64(value_.real_); @@ -1317,11 +1318,12 @@ bool Value::isInt64() const { // Note that maxInt64 (= 2^63 - 1) is not exactly representable as a // double, so double(maxInt64) will be rounded up to 2^63. Therefore we // require the value to be strictly less than the limit. - // minInt64 is -2^63 which can be represented as a double, but since double values in its proximity are also - // rounded to -2^63, we require the value to be strictly greater than the limit to avoid returning - // 'true' for values that are not in the range - return value_.real_ > double(minInt64) && - value_.real_ < double(maxInt64) && IsIntegral(value_.real_); + // minInt64 is -2^63 which can be represented as a double, but since double + // values in its proximity are also rounded to -2^63, we require the value + // to be strictly greater than the limit to avoid returning 'true' for + // values that are not in the range + return value_.real_ > double(minInt64) && value_.real_ < double(maxInt64) && + IsIntegral(value_.real_); default: break; } @@ -1359,9 +1361,10 @@ bool Value::isIntegral() const { // Note that maxUInt64 (= 2^64 - 1) is not exactly representable as a // double, so double(maxUInt64) will be rounded up to 2^64. Therefore we // require the value to be strictly less than the limit. - // minInt64 is -2^63 which can be represented as a double, but since double values in its proximity are also - // rounded to -2^63, we require the value to be strictly greater than the limit to avoid returning - // 'true' for values that are not in the range + // minInt64 is -2^63 which can be represented as a double, but since double + // values in its proximity are also rounded to -2^63, we require the value + // to be strictly greater than the limit to avoid returning 'true' for + // values that are not in the range return value_.real_ > double(minInt64) && value_.real_ < maxUInt64AsDouble && IsIntegral(value_.real_); #else diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index 668c1469a..5a0ce01ce 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -1191,8 +1191,8 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, integers) { JSONTEST_ASSERT_EQUAL(true, val.asBool()); JSONTEST_ASSERT_STRING_EQUAL("-9223372036854775808", val.asString()); - // int64 min (floating point constructor). Since double values in proximity of kint64min - // are rounded to kint64min, we don't check for conversion to int64. + // int64 min (floating point constructor). Since double values in proximity of + // kint64min are rounded to kint64min, we don't check for conversion to int64. val = Json::Value(double(kint64min)); JSONTEST_ASSERT_EQUAL(Json::realValue, val.type());