tryTo<> generics for string to integer conversion (#4676)

`tryTo<>` generics for string to integer conversion The first approach to substitute all `safeStrto*` conversions to `tryTo<>` generics. Thare are some advantages in using templates here: - Destination value type explicitly takes a part in call syntax. - You could use it other template code Also I have removed `safeStrtoi` from the code as an example of usage.
2024-11-07 09:58:54 +00:00 · 2018-07-12 18:03:57 +01:00 · 2018-07-12 18:03:57 +01:00 · 585e73e1e8
commit 585e73e1e8
parent 8b864f1935
4 changed files with 370 additions and 20 deletions
--- a/osquery/core/conversions.h
+++ b/osquery/core/conversions.h
@ -176,22 +176,6 @@ inline Status safeStrtoll(const std::string& rep, size_t base, long long& out) {
  return Status(0);
 }

-/// Safely convert a string representation of an integer base.
-inline Status safeStrtoi(const std::string& rep, int base, int& out) {
-  try {
-    out = std::stoi(rep, nullptr, base);
-  } catch (const std::invalid_argument& ia) {
-    return Status(
-        1, std::string("If no conversion could be performed. ") + ia.what());
-  } catch (const std::out_of_range& oor) {
-    return Status(1,
-                  std::string("Value read is out of the range of representable "
-                              "values by an int. ") +
-                      oor.what());
-  }
-  return Status(0);
-}
-
 /// Safely convert a string representation of an integer base.
 inline Status safeStrtoull(const std::string& rep,
                           size_t base,
@ -311,6 +295,7 @@ std::string stringFromCFData(const CFDataRef& cf_data);
 enum class ConversionError {
  InvalidArgument,
  OutOfRange,
+  Unknown,
 };

 template <typename ToType, typename FromType>
@ -325,10 +310,114 @@ tryTo(FromType&& from) {

 namespace impl {

+template <typename Type>
+struct IsStlString {
+  static constexpr bool value = std::is_same<Type, std::string>::value ||
+                                std::is_same<Type, std::wstring>::value;
+};
+
+template <typename Type>
+struct IsInteger {
+  static constexpr bool value =
+      std::is_integral<Type>::value && !std::is_same<Type, bool>::value;
+};
+
+template <typename FromType,
+          typename ToType,
+          typename IntType,
+          typename =
+              typename std::enable_if<std::is_same<ToType, IntType>::value &&
+                                          IsStlString<FromType>::value,
+                                      IntType>::type>
+struct IsConversionFromStringToIntEnabledFor {
+  using type = IntType;
+};
+
+template <typename ToType, typename FromType>
+inline
+    typename IsConversionFromStringToIntEnabledFor<FromType, ToType, int>::type
+    throwingStringToInt(const FromType& from, const int base) {
+  auto pos = std::size_t{};
+  return std::stoi(from, &pos, base);
+}
+
+template <typename ToType, typename FromType>
+inline typename IsConversionFromStringToIntEnabledFor<FromType,
+                                                      ToType,
+                                                      long int>::type
+throwingStringToInt(const FromType& from, const int base) {
+  auto pos = std::size_t{};
+  return std::stol(from, &pos, base);
+}
+
+template <typename ToType, typename FromType>
+inline typename IsConversionFromStringToIntEnabledFor<FromType,
+                                                      ToType,
+                                                      long long int>::type
+throwingStringToInt(const FromType& from, const int base) {
+  auto pos = std::size_t{};
+  return std::stoll(from, &pos, base);
+}
+
+template <typename ToType, typename FromType>
+inline typename IsConversionFromStringToIntEnabledFor<FromType,
+                                                      ToType,
+                                                      unsigned int>::type
+throwingStringToInt(const FromType& from, const int base) {
+  auto pos = std::size_t{};
+  return std::stoul(from, &pos, base);
+}
+
+template <typename ToType, typename FromType>
+inline typename IsConversionFromStringToIntEnabledFor<FromType,
+                                                      ToType,
+                                                      unsigned long int>::type
+throwingStringToInt(const FromType& from, const int base) {
+  auto pos = std::size_t{};
+  return std::stoul(from, &pos, base);
+}
+
+template <typename ToType, typename FromType>
+inline
+    typename IsConversionFromStringToIntEnabledFor<FromType,
+                                                   ToType,
+                                                   unsigned long long int>::type
+    throwingStringToInt(const FromType& from, const int base) {
+  auto pos = std::size_t{};
+  return std::stoull(from, &pos, base);
+}
+
 Expected<bool, ConversionError> stringToBool(std::string from);

 } // namespace impl

+/**
+ * Template tryTo for [w]string to integer conversion
+ */
+template <typename ToType, typename FromType>
+inline typename std::enable_if<impl::IsInteger<ToType>::value &&
+                                   impl::IsStlString<FromType>::value,
+                               Expected<ToType, ConversionError>>::type
+tryTo(const FromType& from, const int base = 10) noexcept {
+  try {
+    return impl::throwingStringToInt<ToType>(from, base);
+  } catch (const std::invalid_argument& ia) {
+    return createError(ConversionError::InvalidArgument,
+                       "If no conversion could be performed. ")
+           << ia.what();
+  } catch (const std::out_of_range& oor) {
+    return createError(ConversionError::OutOfRange,
+                       "Value read is out of the range of representable values "
+                       "by an int. ")
+           << oor.what();
+  } catch (...) {
+    return createError(ConversionError::Unknown,
+                       "Unknown error during conversion ")
+           << boost::core::demangle(typeid(FromType).name()) << " to "
+           << boost::core::demangle(typeid(ToType).name()) << " base " << base;
+  }
+}
+
 /**
 * Parsing general representation of boolean value in string.
 *     "1" : true
--- a/osquery/core/tests/conversions_tests.cpp
+++ b/osquery/core/tests/conversions_tests.cpp
@ -8,6 +8,9 @@
 *  You may select, at your option, one of the above-listed licenses.
 */

+#include <cstdint>
+#include <string>
+
 #include <boost/make_shared.hpp>
 #include <boost/shared_ptr.hpp>

@ -325,6 +328,258 @@ TEST_F(ConversionsTests, tryTo_same_type) {
  ASSERT_FALSE(ret2.isError());
 }

+template <typename ValueType, typename StrType>
+void testTryToForRvalue(ValueType value, const StrType& str) {
+  auto ret = tryTo<ValueType>(StrType{str});
+  ASSERT_FALSE(ret.isError());
+  ASSERT_EQ(ret.get(), value);
+}
+
+template <typename ValueType, typename StrType>
+void testTryToForLValue(ValueType value, StrType str) {
+  auto ret = tryTo<ValueType>(str);
+  ASSERT_FALSE(ret.isError());
+  ASSERT_EQ(ret.get(), value);
+}
+
+template <typename ValueType, typename StrType>
+void testTryToForConstLValue(ValueType value, const StrType str) {
+  auto ret = tryTo<ValueType>(str);
+  ASSERT_FALSE(ret.isError());
+  ASSERT_EQ(ret.get(), value);
+}
+
+template <typename ValueType, typename StrType>
+void testTryToForString(ValueType value, const StrType str) {
+  testTryToForRvalue(value, str);
+  testTryToForLValue(value, str);
+  testTryToForConstLValue(value, str);
+}
+
+template <typename ValueType>
+void testTryToForValue(ValueType value) {
+  testTryToForString(value, std::to_string(value));
+  testTryToForString(value, std::to_wstring(value));
+}
+
+template <typename IntType>
+void testTryToForUnsignedInt() {
+  testTryToForValue<IntType>(119);
+  testTryToForValue<IntType>(std::numeric_limits<IntType>::max());
+  testTryToForValue<IntType>(std::numeric_limits<IntType>::min());
+  testTryToForValue<IntType>(std::numeric_limits<IntType>::lowest());
+  {
+    auto ret = tryTo<IntType>(std::string{"0xfb"}, 16);
+    ASSERT_FALSE(ret.isError());
+    ASSERT_EQ(ret.get(), 251);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"FB"}, 16);
+    ASSERT_FALSE(ret.isError());
+    ASSERT_EQ(ret.get(), 251);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"0xFb"}, 16);
+    ASSERT_FALSE(ret.isError());
+    ASSERT_EQ(ret.get(), 251);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"E1bC2"}, 16);
+    ASSERT_FALSE(ret.isError());
+    ASSERT_EQ(ret.get(), 924610);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"10101"}, 2);
+    ASSERT_FALSE(ret.isError());
+    ASSERT_EQ(ret.get(), 21);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"035"}, 8);
+    ASSERT_FALSE(ret.isError());
+    ASSERT_EQ(ret.get(), 29);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"47"}, 8);
+    ASSERT_FALSE(ret.isError());
+    ASSERT_EQ(ret.get(), 39);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"+15"});
+    ASSERT_FALSE(ret.isError());
+    ASSERT_EQ(ret.get(), 15);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"+1A"}, 16);
+    ASSERT_FALSE(ret.isError());
+    ASSERT_EQ(ret.get(), 26);
+  }
+  // failure tests
+  {
+    auto ret = tryTo<IntType>(std::string{""});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"x"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"xor"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{".1"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"(10)"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"O"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"lO0"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"IV"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"s1"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"u1"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"#12"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"%99"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"*483"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"/488"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"\\493"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"+ 19"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string(2, '\0'));
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+}
+
+template <typename IntType>
+void testTryToForSignedInt() {
+  testTryToForUnsignedInt<IntType>();
+  testTryToForValue<int>(-126);
+  {
+    auto ret = tryTo<IntType>(std::string{"-7A"}, 16);
+    ASSERT_FALSE(ret.isError());
+    ASSERT_EQ(ret.get(), -122);
+  }
+  // failure tests
+  {
+    auto ret = tryTo<IntType>(std::string{"--14779"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"+-1813"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+  {
+    auto ret = tryTo<IntType>(std::string{"- 3"});
+    ASSERT_TRUE(ret.isError());
+    ASSERT_EQ(ret.getErrorCode(), ConversionError::InvalidArgument);
+  }
+}
+
+TEST_F(ConversionsTests, try_i_to_string_and_back) {
+  testTryToForSignedInt<int>();
+}
+
+TEST_F(ConversionsTests, try_l_to_string_and_back) {
+  testTryToForSignedInt<long>();
+}
+
+TEST_F(ConversionsTests, try_ll_to_string_and_back) {
+  testTryToForSignedInt<long long>();
+}
+
+TEST_F(ConversionsTests, try_i32_to_string_and_back) {
+  testTryToForSignedInt<std::int32_t>();
+}
+
+TEST_F(ConversionsTests, try_i64_to_string_and_back) {
+  testTryToForSignedInt<std::int64_t>();
+}
+
+TEST_F(ConversionsTests, try_imax_to_string_and_back) {
+  testTryToForSignedInt<std::intmax_t>();
+}
+
+TEST_F(ConversionsTests, try_u_to_string_and_back) {
+  testTryToForUnsignedInt<unsigned>();
+}
+
+TEST_F(ConversionsTests, try_ul_to_string_and_back) {
+  testTryToForUnsignedInt<unsigned long>();
+}
+
+TEST_F(ConversionsTests, try_ull_to_string_and_back) {
+  testTryToForUnsignedInt<unsigned long long>();
+}
+
+TEST_F(ConversionsTests, try_u32_to_string_and_back) {
+  testTryToForUnsignedInt<std::uint32_t>();
+}
+
+TEST_F(ConversionsTests, try_u64_to_string_and_back) {
+  testTryToForUnsignedInt<std::uint64_t>();
+}
+
+TEST_F(ConversionsTests, try_umax_to_string_and_back) {
+  testTryToForUnsignedInt<std::uintmax_t>();
+}
+
+TEST_F(ConversionsTests, try_size_t_to_string_and_back) {
+  testTryToForUnsignedInt<std::size_t>();
+}
+
 TEST_F(ConversionsTests, tryTo_string_to_boolean_valid_args) {
  const auto test_table = std::unordered_map<std::string, bool>{
      {"1", true},        {"0", false},       {"y", true},
--- a/osquery/database/plugins/rocksdb.cpp
+++ b/osquery/database/plugins/rocksdb.cpp
@ -257,8 +257,11 @@ Status RocksDBDatabasePlugin::get(const std::string& domain,
  std::string result;
  auto s = this->get(domain, key, result);
  if (s.ok()) {
-    if (safeStrtoi(result, 10, value)) {
-      return Status(1, "Could not deserialize str to int");
+    auto expectedValue = tryTo<int>(result);
+    if (expectedValue.isError()) {
+      return Status::failure("Could not deserialize str to int");
+    } else {
+      value = expectedValue.take();
    }
  }
  return s;
--- a/osquery/database/plugins/sqlite.cpp
+++ b/osquery/database/plugins/sqlite.cpp
@ -148,8 +148,11 @@ Status SQLiteDatabasePlugin::get(const std::string& domain,
  std::string result;
  auto s = this->get(domain, key, result);
  if (s.ok()) {
-    if (safeStrtoi(result, 10, value)) {
-      return Status(1, "Could not deserialize str to int");
+    auto expectedValue = tryTo<int>(result);
+    if (expectedValue.isError()) {
+      return Status::failure("Could not deserialize str to int");
+    } else {
+      value = expectedValue.take();
    }
  }
  return s;