Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

json_value.cpp bug in the edges of uint/int #1519

Merged
merged 8 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions src/lib_json/json_value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ template <typename T, typename U>
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<double>(min) && d <= static_cast<double>(max);
return d >= static_cast<double>(min) && d <= static_cast<double>(max) &&
!(static_cast<U>(d) == min && d != static_cast<double>(min));
}
#else // if !defined(JSON_USE_INT64_DOUBLE_CONVERSION)
static inline double integerToDouble(Json::UInt64 value) {
Expand All @@ -101,7 +102,8 @@ template <typename T> static inline double integerToDouble(T value) {

template <typename T, typename U>
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<U>(d) == min && d != integerToDouble(min));
}
#endif // if !defined(JSON_USE_INT64_DOUBLE_CONVERSION)

Expand Down Expand Up @@ -705,6 +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");
JSON_ASSERT_MESSAGE(InRange(value_.real_, minInt64, maxInt64),
"double out of Int64 range");
return Int64(value_.real_);
Expand Down Expand Up @@ -1311,8 +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.
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;
}
Expand Down Expand Up @@ -1350,7 +1361,11 @@ 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 &&
Expand Down
8 changes: 2 additions & 6 deletions src/test_lib_json/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1191,15 +1191,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));
Expand All @@ -1208,8 +1206,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());
Expand Down
Loading