Skip to content

Commit 54fc4e2

Browse files
YaalLekbaylesj
andauthored
json_value.cpp bug in the edges of uint/int (open-source-parsers#1519)
* json_value.cpp bug in the edges of uint/int Fixing bug of sending a number that is a bit bigger than max<uint64_t> it returns 0: https://stackoverflow.com/questions/77261400/jsoncpp-do-not-protect-from-uint64-overflow-and-have-weird-behavior/77261716#77261716 * Update json_value.cpp Fixing bug of sending a number that is a bit bigger than max<uint64_t> it returns 0: https://stackoverflow.com/questions/77261400/jsoncpp-do-not-protect-from-uint64-overflow-and-have-weird-behavior/77261716#77261716 * Update test cases * json_value.cpp bug in the edges of uint/int Fixing bug of sending a number that is a bit bigger than max<uint64_t> it returns 0: https://stackoverflow.com/questions/77261400/jsoncpp-do-not-protect-from-uint64-overflow-and-have-weird-behavior/77261716#77261716 * Run clang tidy --------- Co-authored-by: Jordan Bayles <[email protected]>
1 parent 76ff1db commit 54fc4e2

File tree

2 files changed

+22
-11
lines changed

2 files changed

+22
-11
lines changed

src/lib_json/json_value.cpp

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ template <typename T, typename U>
8787
static inline bool InRange(double d, T min, U max) {
8888
// The casts can lose precision, but we are looking only for
8989
// an approximate range. Might fail on edge cases though. ~cdunn
90-
return d >= static_cast<double>(min) && d <= static_cast<double>(max);
90+
return d >= static_cast<double>(min) && d <= static_cast<double>(max) &&
91+
!(static_cast<U>(d) == min && d != static_cast<double>(min));
9192
}
9293
#else // if !defined(JSON_USE_INT64_DOUBLE_CONVERSION)
9394
static inline double integerToDouble(Json::UInt64 value) {
@@ -101,7 +102,8 @@ template <typename T> static inline double integerToDouble(T value) {
101102

102103
template <typename T, typename U>
103104
static inline bool InRange(double d, T min, U max) {
104-
return d >= integerToDouble(min) && d <= integerToDouble(max);
105+
return d >= integerToDouble(min) && d <= integerToDouble(max) &&
106+
!(static_cast<U>(d) == min && d != integerToDouble(min));
105107
}
106108
#endif // if !defined(JSON_USE_INT64_DOUBLE_CONVERSION)
107109

@@ -705,6 +707,11 @@ Value::Int64 Value::asInt64() const {
705707
JSON_ASSERT_MESSAGE(isInt64(), "LargestUInt out of Int64 range");
706708
return Int64(value_.uint_);
707709
case realValue:
710+
// If the double value is in proximity to minInt64, it will be rounded to
711+
// minInt64. The correct value in this scenario is indeterminable
712+
JSON_ASSERT_MESSAGE(
713+
value_.real_ != minInt64,
714+
"Double value is minInt64, precise value cannot be determined");
708715
JSON_ASSERT_MESSAGE(InRange(value_.real_, minInt64, maxInt64),
709716
"double out of Int64 range");
710717
return Int64(value_.real_);
@@ -1311,8 +1318,12 @@ bool Value::isInt64() const {
13111318
// Note that maxInt64 (= 2^63 - 1) is not exactly representable as a
13121319
// double, so double(maxInt64) will be rounded up to 2^63. Therefore we
13131320
// require the value to be strictly less than the limit.
1314-
return value_.real_ >= double(minInt64) &&
1315-
value_.real_ < double(maxInt64) && IsIntegral(value_.real_);
1321+
// minInt64 is -2^63 which can be represented as a double, but since double
1322+
// values in its proximity are also rounded to -2^63, we require the value
1323+
// to be strictly greater than the limit to avoid returning 'true' for
1324+
// values that are not in the range
1325+
return value_.real_ > double(minInt64) && value_.real_ < double(maxInt64) &&
1326+
IsIntegral(value_.real_);
13161327
default:
13171328
break;
13181329
}
@@ -1350,7 +1361,11 @@ bool Value::isIntegral() const {
13501361
// Note that maxUInt64 (= 2^64 - 1) is not exactly representable as a
13511362
// double, so double(maxUInt64) will be rounded up to 2^64. Therefore we
13521363
// require the value to be strictly less than the limit.
1353-
return value_.real_ >= double(minInt64) &&
1364+
// minInt64 is -2^63 which can be represented as a double, but since double
1365+
// values in its proximity are also rounded to -2^63, we require the value
1366+
// to be strictly greater than the limit to avoid returning 'true' for
1367+
// values that are not in the range
1368+
return value_.real_ > double(minInt64) &&
13541369
value_.real_ < maxUInt64AsDouble && IsIntegral(value_.real_);
13551370
#else
13561371
return value_.real_ >= minInt && value_.real_ <= maxUInt &&

src/test_lib_json/main.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,15 +1191,13 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, integers) {
11911191
JSONTEST_ASSERT_EQUAL(true, val.asBool());
11921192
JSONTEST_ASSERT_STRING_EQUAL("-9223372036854775808", val.asString());
11931193

1194-
// int64 min (floating point constructor). Note that kint64min *is* exactly
1195-
// representable as a double.
1194+
// int64 min (floating point constructor). Since double values in proximity of
1195+
// kint64min are rounded to kint64min, we don't check for conversion to int64.
11961196
val = Json::Value(double(kint64min));
11971197

11981198
JSONTEST_ASSERT_EQUAL(Json::realValue, val.type());
11991199

12001200
checks = IsCheck();
1201-
checks.isInt64_ = true;
1202-
checks.isIntegral_ = true;
12031201
checks.isDouble_ = true;
12041202
checks.isNumeric_ = true;
12051203
JSONTEST_ASSERT_PRED(checkIs(val, checks));
@@ -1208,8 +1206,6 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, integers) {
12081206
JSONTEST_ASSERT(!val.isConvertibleTo(Json::intValue));
12091207
JSONTEST_ASSERT(!val.isConvertibleTo(Json::uintValue));
12101208

1211-
JSONTEST_ASSERT_EQUAL(kint64min, val.asInt64());
1212-
JSONTEST_ASSERT_EQUAL(kint64min, val.asLargestInt());
12131209
JSONTEST_ASSERT_EQUAL(-9223372036854775808.0, val.asDouble());
12141210
JSONTEST_ASSERT_EQUAL(-9223372036854775808.0, val.asFloat());
12151211
JSONTEST_ASSERT_EQUAL(true, val.asBool());

0 commit comments

Comments
 (0)