common: use C conversions in ToNumber

Throwing exceptions seems to confuse ASan. Exceptions are not allowed by the Google C++ style guide anyway.
Add more test cases and validation while at it.

src/util/common.cc

@@ -27,6 +27,8 @@
 
 #include <algorithm>
 #include <cctype>
+#include <cerrno>
+#include <climits>
 #include <cmath>
 #include <cstdio>
 #include <cstdlib>
@@ -60,42 +62,72 @@ Builder::operator std::string() const { return ss_.str(); }
 
 namespace {
 
-// Templated alias that matches the std::stoi() function family (but not
-// std::stof, which lacks the third parameter).
 template <typename T>
-using NumberConversion = T(std::string const&, std::size_t*, int);
+using NumberConversion = T(const char*, char**);
 
 template <typename T>
-bool ToNumberImpl(std::string str, T* ptr, NumberConversion<T> conv) {
-  Trim(&str);
-  try {
-    std::size_t pos;
-    T res = conv(str, &pos, 10);
-    if (pos != str.length()) {
+using NumberValidation = bool(T);
+
+template <typename T>
+bool ToNumberImpl(std::string const& value, T* ptr, NumberConversion<T> conv,
+                  NumberValidation<T> valid) {
+  const char* str = value.c_str();
+  char* str_end;
+  T res = conv(str, &str_end);
+
+  // reject not fully parsed strings
+  while (*str_end != '\0') {
+    if (!isspace(*str_end)) {
       return false;
     }
-    (*ptr) = res;
-    return true;
-  } catch (...) {
+  }
+
+  // reject out of range values
+  if (!valid(res)) {
     return false;
   }
+
+  (*ptr) = res;
+  return true;
 }
 
 }  // namespace
 
 bool ToNumber(std::string const& str, int* ptr) {
-  return ToNumberImpl(str, ptr, std::stoi);
+  return ToNumberImpl(str, ptr,
+                      +[](const char* str, char** str_end) {
+                        errno = 0;
+                        long res = std::strtol(str, str_end, 10);
+                        if (res < INT_MIN || res > INT_MAX) {
+                          errno = ERANGE;
+                          return INT_MAX;
+                        }
+                        return static_cast<int>(res);
+                      },
+                      +[](int) { return (errno == 0); });
 }
 
 bool ToNumber(std::string const& str, long* ptr) {
-  return ToNumberImpl(str, ptr, std::stol);
+  return ToNumberImpl(str, ptr,
+                      +[](const char* str, char** str_end) {
+                        errno = 0;
+                        return std::strtol(str, str_end, 10);
+                      },
+                      +[](long) { return (errno == 0); });
 }
 
 bool ToNumber(std::string const& str, float* ptr) {
-  return ToNumberImpl(
-      str, ptr, +[](std::string const& str, std::size_t* pos, int /*base*/) {
-        return std::stof(str, pos);
-      });
+  return ToNumberImpl(str, ptr,
+                      +[](const char* str, char** str_end) {
+                        errno = 0;
+                        return std::strtof(str, str_end);
+                      },
+                      +[](float f) {
+                        if (errno != 0) {
+                          return false;
+                        }
+                        return std::isfinite(f);
+                      });
 }
 
 std::string& Trim(std::string* str) {

src/util/common_test.cc

@@ -144,6 +144,8 @@ TEST_CASE("ShellExec") {
 TEST_CASE("ToNumber") {
   SECTION("int") {
     int i;
+    REQUIRE(util::string::ToNumber("0", &i));
+    REQUIRE(i == 0);  // parsed and read correctly
     REQUIRE(util::string::ToNumber("1234", &i));
     REQUIRE(i == 1234);  // parsed and read correctly
     REQUIRE_FALSE(util::string::ToNumber("5678abcd", &i));
@@ -152,6 +154,8 @@ TEST_CASE("ToNumber") {
 
   SECTION("long") {
     long l;
+    REQUIRE(util::string::ToNumber("0", &l));
+    REQUIRE(l == 0);  // parsed and read correctly
     REQUIRE(util::string::ToNumber("1234", &l));
     REQUIRE(l == 1234);  // parsed and read correctly
     REQUIRE_FALSE(util::string::ToNumber("5678abcd", &l));
@@ -160,9 +164,19 @@ TEST_CASE("ToNumber") {
 
   SECTION("float") {
     float f;
+    REQUIRE(util::string::ToNumber("0", &f));
+    REQUIRE(f == 0);  // parsed and read correctly
     REQUIRE(util::string::ToNumber("1.234", &f));
     REQUIRE(f == 1.234f);  // parsed and read correctly
     REQUIRE_FALSE(util::string::ToNumber("5.678abcd", &f));
     REQUIRE(f == 1.234f);  // failed parsing, unchanged
+
+    // Out of range values are rejected and writing to the provided pointer
+    // is not attempted in such a case.
+    float* nptr = nullptr;
+    REQUIRE_FALSE(util::string::ToNumber("inf", nptr));
+    REQUIRE_FALSE(util::string::ToNumber("infinity", nptr));
+    REQUIRE_FALSE(util::string::ToNumber("nan", nptr));
+    REQUIRE_FALSE(util::string::ToNumber("nan(16)", nptr));
   }
 }