From ed57e9003e8e360777a1e0b3a2066603ceb9d906 Mon Sep 17 00:00:00 2001 From: Aiee <18348405+Aiee@users.noreply.github.com> Date: Mon, 26 Jul 2021 17:48:34 +0800 Subject: [PATCH 1/2] Fix toInteger/toFloat parsing string --- src/common/datatypes/Value.cpp | 19 +++- src/common/datatypes/test/ValueTest.cpp | 139 +++++++++++++++--------- 2 files changed, 103 insertions(+), 55 deletions(-) diff --git a/src/common/datatypes/Value.cpp b/src/common/datatypes/Value.cpp index f8fb10d37..2885b952c 100644 --- a/src/common/datatypes/Value.cpp +++ b/src/common/datatypes/Value.cpp @@ -1595,8 +1595,13 @@ Value Value::toFloat() const { } case Value::Type::STRING: { const auto& str = getStr(); - char *pEnd; - double val = strtod(str.c_str(), &pEnd); + char* pEnd; + auto it = std::find(str.begin(), str.end(), '.'); + double val = (it == str.end()) + ? static_cast( + strtoll(str.c_str(), &pEnd, 10)) // string representing int + : static_cast( + strtod(str.c_str(), &pEnd)); // string representing double if (*pEnd != '\0') { return Value::kNullValue; } @@ -1629,12 +1634,16 @@ Value Value::toInt() const { } case Value::Type::STRING: { const auto& str = getStr(); - char *pEnd; - double val = strtod(str.c_str(), &pEnd); + char* pEnd; + auto it = std::find(str.begin(), str.end(), '.'); + int64_t val = + (it == str.end()) + ? int64_t(strtoll(str.c_str(), &pEnd, 10)) // string representing int + : int64_t(strtod(str.c_str(), &pEnd)); // string representing double if (*pEnd != '\0') { return Value::kNullValue; } - return static_cast(val); + return val; } default: { return Value::kNullBadType; diff --git a/src/common/datatypes/test/ValueTest.cpp b/src/common/datatypes/test/ValueTest.cpp index 4c5135174..92e15adb0 100644 --- a/src/common/datatypes/test/ValueTest.cpp +++ b/src/common/datatypes/test/ValueTest.cpp @@ -638,76 +638,115 @@ TEST(Value, TypeCast) { EXPECT_EQ(Value::Type::NULLVALUE, vb8.type()); } { - auto vf1 = vInt.toFloat(); - EXPECT_EQ(Value::Type::FLOAT, vf1.type()); - EXPECT_EQ(vf1.getFloat(), 1.0); + auto vf = vInt.toFloat(); + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), 1.0); - auto vf2 = vFloat.toFloat(); - EXPECT_EQ(Value::Type::FLOAT, vf2.type()); - EXPECT_EQ(vf2.getFloat(), 3.14); + vf = vFloat.toFloat(); + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), 3.14); - auto vf3 = vStr1.toFloat(); - EXPECT_EQ(Value::Type::NULLVALUE, vf3.type()); + vf = vStr1.toFloat(); + EXPECT_EQ(Value::Type::NULLVALUE, vf.type()); - auto vf4 = vStr2.toFloat(); - EXPECT_EQ(Value::Type::FLOAT, vf4.type()); - EXPECT_EQ(vf4.getFloat(), 3.14); + vf = vStr2.toFloat(); + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), 3.14); - auto vf5 = vBool1.toFloat(); - EXPECT_EQ(Value::Type::NULLVALUE, vf5.type()); + vf = vBool1.toFloat(); + EXPECT_EQ(Value::Type::NULLVALUE, vf.type()); - auto vf6 = vBool2.toFloat(); - EXPECT_EQ(Value::Type::NULLVALUE, vf6.type()); + vf = vBool2.toFloat(); + EXPECT_EQ(Value::Type::NULLVALUE, vf.type()); - auto vf7 = vDate.toFloat(); - EXPECT_EQ(Value::Type::NULLVALUE, vf7.type()); + vf = vDate.toFloat(); + EXPECT_EQ(Value::Type::NULLVALUE, vf.type()); - auto vf8 = vNull.toFloat(); - EXPECT_EQ(Value::Type::NULLVALUE, vf8.type()); + vf = vNull.toFloat(); + EXPECT_EQ(Value::Type::NULLVALUE, vf.type()); - auto vf9 = vIntMin.toFloat(); - EXPECT_EQ(Value::Type::FLOAT, vf9.type()); - EXPECT_EQ(vf9.getFloat(), std::numeric_limits::min()); + vf = vIntMin.toFloat(); + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), std::numeric_limits::min()); - auto vf10 = vIntMax.toFloat(); - EXPECT_EQ(Value::Type::FLOAT, vf10.type()); - EXPECT_EQ(vf10.getFloat(), std::numeric_limits::max()); + vf = vIntMax.toFloat(); + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), static_cast(std::numeric_limits::max())); + + // string of int + vf = Value("-9223372036854775807").toFloat(); + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), static_cast(std::numeric_limits::min() + 1)); + + vf = Value("-9223372036854775808") + .toFloat(); // will be converted to -9223372036854776000.0 + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), static_cast(std::numeric_limits::min())); + + vf = Value("9223372036854775807").toFloat(); + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), static_cast(std::numeric_limits::max())); + + // string of double + vf = Value(std::to_string(std::numeric_limits::max())).toFloat(); + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), std::numeric_limits::max()); + + vf = Value(std::to_string(std::numeric_limits::max())).toFloat(); + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), std::numeric_limits::max()); } { - auto vi1 = vInt.toInt(); - EXPECT_EQ(Value::Type::INT, vi1.type()); - EXPECT_EQ(vi1.getInt(), 1); + auto vi = vInt.toInt(); + EXPECT_EQ(Value::Type::INT, vi.type()); + EXPECT_EQ(vi.getInt(), 1); + + vi = vFloat.toInt(); + EXPECT_EQ(Value::Type::INT, vi.type()); + EXPECT_EQ(vi.getInt(), 3); + + vi = vStr1.toInt(); + EXPECT_EQ(Value::Type::NULLVALUE, vi.type()); + + vi = vStr2.toInt(); + + EXPECT_EQ(Value::Type::INT, vi.type()); + EXPECT_EQ(vi.getInt(), 3); + + vi = vBool1.toInt(); + EXPECT_EQ(Value::Type::NULLVALUE, vi.type()); - auto vi2 = vFloat.toInt(); - EXPECT_EQ(Value::Type::INT, vi2.type()); - EXPECT_EQ(vi2.getInt(), 3); + vi = vBool2.toInt(); + EXPECT_EQ(Value::Type::NULLVALUE, vi.type()); - auto vi3 = vStr1.toInt(); - EXPECT_EQ(Value::Type::NULLVALUE, vi3.type()); + vi = vDate.toInt(); + EXPECT_EQ(Value::Type::NULLVALUE, vi.type()); - auto vi4 = vStr2.toInt(); - EXPECT_EQ(Value::Type::INT, vi4.type()); - EXPECT_EQ(vi4.getInt(), 3); + vi = vNull.toInt(); + EXPECT_EQ(Value::Type::NULLVALUE, vi.type()); - auto vi5 = vBool1.toInt(); - EXPECT_EQ(Value::Type::NULLVALUE, vi5.type()); + vi = vFloatMin.toInt(); + EXPECT_EQ(Value::Type::INT, vi.type()); + EXPECT_EQ(vi.getInt(), std::numeric_limits::min()); - auto vi6 = vBool2.toInt(); - EXPECT_EQ(Value::Type::NULLVALUE, vi6.type()); + vi = vFloatMax.toInt(); + EXPECT_EQ(Value::Type::INT, vi.type()); + EXPECT_EQ(vi.getInt(), std::numeric_limits::max()); - auto vi7 = vDate.toInt(); - EXPECT_EQ(Value::Type::NULLVALUE, vi7.type()); + // string of int + vi = Value("-9223372036854775807").toInt(); + EXPECT_EQ(Value::Type::INT, vi.type()); + EXPECT_EQ(vi.getInt(), std::numeric_limits::min() + 1); - auto vi8 = vNull.toInt(); - EXPECT_EQ(Value::Type::NULLVALUE, vi8.type()); + vi = Value("-9223372036854775808").toInt(); + EXPECT_EQ(Value::Type::INT, vi.type()); + EXPECT_EQ(vi.getInt(), std::numeric_limits::min()); - auto vi9 = vFloatMin.toInt(); - EXPECT_EQ(Value::Type::INT, vi9.type()); - EXPECT_EQ(vi9.getInt(), std::numeric_limits::min()); + vi = Value("9223372036854775807").toInt(); + EXPECT_EQ(Value::Type::INT, vi.type()); + EXPECT_EQ(vi.getInt(), std::numeric_limits::max()); - auto vi10 = vFloatMax.toInt(); - EXPECT_EQ(Value::Type::INT, vi10.type()); - EXPECT_EQ(vi10.getInt(), std::numeric_limits::max()); + // string of double } } From 7884faf5f4cf60de5edce43b845c4a0a540e98ed Mon Sep 17 00:00:00 2001 From: Aiee <18348405+Aiee@users.noreply.github.com> Date: Mon, 26 Jul 2021 19:36:42 +0800 Subject: [PATCH 2/2] Check toInteger() overflow when parsing string --- src/common/datatypes/Value.cpp | 14 ++++++----- src/common/datatypes/test/ValueTest.cpp | 33 ++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/common/datatypes/Value.cpp b/src/common/datatypes/Value.cpp index 2885b952c..dffc3743a 100644 --- a/src/common/datatypes/Value.cpp +++ b/src/common/datatypes/Value.cpp @@ -1596,12 +1596,7 @@ Value Value::toFloat() const { case Value::Type::STRING: { const auto& str = getStr(); char* pEnd; - auto it = std::find(str.begin(), str.end(), '.'); - double val = (it == str.end()) - ? static_cast( - strtoll(str.c_str(), &pEnd, 10)) // string representing int - : static_cast( - strtod(str.c_str(), &pEnd)); // string representing double + double val = strtod(str.c_str(), &pEnd); if (*pEnd != '\0') { return Value::kNullValue; } @@ -1634,6 +1629,7 @@ Value Value::toInt() const { } case Value::Type::STRING: { const auto& str = getStr(); + errno = 0; char* pEnd; auto it = std::find(str.begin(), str.end(), '.'); int64_t val = @@ -1643,6 +1639,12 @@ Value Value::toInt() const { if (*pEnd != '\0') { return Value::kNullValue; } + // Check overflow + if ((val == std::numeric_limits::max() || + val == std::numeric_limits::min()) && + errno == ERANGE) { + return kNullOverflow; + } return val; } default: { diff --git a/src/common/datatypes/test/ValueTest.cpp b/src/common/datatypes/test/ValueTest.cpp index 92e15adb0..ca2385e88 100644 --- a/src/common/datatypes/test/ValueTest.cpp +++ b/src/common/datatypes/test/ValueTest.cpp @@ -673,7 +673,7 @@ TEST(Value, TypeCast) { EXPECT_EQ(Value::Type::FLOAT, vf.type()); EXPECT_EQ(vf.getFloat(), static_cast(std::numeric_limits::max())); - // string of int + // String of int vf = Value("-9223372036854775807").toFloat(); EXPECT_EQ(Value::Type::FLOAT, vf.type()); EXPECT_EQ(vf.getFloat(), static_cast(std::numeric_limits::min() + 1)); @@ -687,7 +687,11 @@ TEST(Value, TypeCast) { EXPECT_EQ(Value::Type::FLOAT, vf.type()); EXPECT_EQ(vf.getFloat(), static_cast(std::numeric_limits::max())); - // string of double + // String of double + vf = Value("123.").toFloat(); + EXPECT_EQ(Value::Type::FLOAT, vf.type()); + EXPECT_EQ(vf.getFloat(), 123.0); + vf = Value(std::to_string(std::numeric_limits::max())).toFloat(); EXPECT_EQ(Value::Type::FLOAT, vf.type()); EXPECT_EQ(vf.getFloat(), std::numeric_limits::max()); @@ -695,6 +699,13 @@ TEST(Value, TypeCast) { vf = Value(std::to_string(std::numeric_limits::max())).toFloat(); EXPECT_EQ(Value::Type::FLOAT, vf.type()); EXPECT_EQ(vf.getFloat(), std::numeric_limits::max()); + + // Invlaid string + vf = Value("12abc").toFloat(); + EXPECT_EQ(Value::kNullValue, vf); + + vf = Value("1.2abc").toFloat(); + EXPECT_EQ(Value::kNullValue, vf); } { auto vi = vInt.toInt(); @@ -734,6 +745,10 @@ TEST(Value, TypeCast) { EXPECT_EQ(vi.getInt(), std::numeric_limits::max()); // string of int + vi = Value("123.").toInt(); + EXPECT_EQ(Value::Type::INT, vi.type()); + EXPECT_EQ(vi.getInt(), 123); + vi = Value("-9223372036854775807").toInt(); EXPECT_EQ(Value::Type::INT, vi.type()); EXPECT_EQ(vi.getInt(), std::numeric_limits::min() + 1); @@ -746,7 +761,19 @@ TEST(Value, TypeCast) { EXPECT_EQ(Value::Type::INT, vi.type()); EXPECT_EQ(vi.getInt(), std::numeric_limits::max()); - // string of double + // String to int Overflow + vi = Value("9223372036854775808").toInt(); + EXPECT_EQ(Value::kNullOverflow, vi); + + vi = Value("-9223372036854775809").toInt(); + EXPECT_EQ(Value::kNullOverflow, vi); + + // Invlaid string + vi = Value("12abc").toInt(); + EXPECT_EQ(Value::kNullValue, vi); + + vi = Value("1.2abc").toInt(); + EXPECT_EQ(Value::kNullValue, vi); } }