Add manual check for negative sign when parsing ull

CesiumGltf/include/CesiumGltf/MetadataConversions.h

@@ -7,11 +7,12 @@
 #include <glm/common.hpp>
 #include <glm/gtx/string_cast.hpp>
 
+#include <cerrno>
 #include <cstdint>
+#include <iostream>
 #include <optional>
 #include <string>
 #include <string_view>
-#include <system_error>
 
 namespace CesiumGltf {
 /**
@@ -198,30 +199,37 @@ struct MetadataConversions<
     }
 
     errno = 0;
+
     char* pLastUsed;
     int64_t parsedValue = std::strtoll(from.c_str(), &pLastUsed, 10);
-    if (errno != ERANGE && pLastUsed == from.c_str() + from.size()) {
-      // Successfully parsed the entire string as an integer of this type.
-      return CesiumUtility::losslessNarrow<TTo, int64_t>(parsedValue);
+    if (errno != EINVAL && errno != ERANGE) {
+      // Check if the entire string was parsed.
+      if (pLastUsed == from.c_str() + from.size()) {
+        // Successfully parsed the entire string as an integer of this type.
+        return CesiumUtility::losslessNarrow<TTo, int64_t>(parsedValue);
+      }
     }
 
     errno = 0;
+
     // Failed to parse as an integer. Maybe we can parse as a double and
     // truncate it?
     double parsedDouble = std::strtod(from.c_str(), &pLastUsed);
-    if (errno != ERANGE && pLastUsed == from.c_str() + from.size()) {
-      // Successfully parsed the entire string as a double.
-      // Convert it to an integer if we can.
-      double truncated = glm::trunc(parsedDouble);
-
-      int64_t asInteger = static_cast<int64_t>(truncated);
-      double roundTrip = static_cast<double>(asInteger);
-      if (roundTrip == truncated) {
-        return CesiumUtility::losslessNarrow<TTo, int64_t>(asInteger);
+    if (errno != EINVAL && errno != ERANGE) {
+      // Check if the entire string was parsed.
+      if (pLastUsed == from.c_str() + from.size()) {
+        // Successfully parsed the entire string as a double.
+        // Convert it to an integer if we can.
+        double truncated = glm::trunc(parsedDouble);
+
+        int64_t asInteger = static_cast<int64_t>(truncated);
+        double roundTrip = static_cast<double>(asInteger);
+        if (roundTrip == truncated) {
+          return CesiumUtility::losslessNarrow<TTo, int64_t>(asInteger);
+        }
       }
     }
 
-    errno = 0;
     return std::nullopt;
   }
 };
@@ -252,34 +260,45 @@ struct MetadataConversions<
       return std::nullopt;
     }
 
+    if (from.find('-') != std::string::npos) {
+      // The string must be manually checked for a negative sign because for
+      // std::strtoull accepts negative numbers and bitcasts them, which is not
+      // desired!
+      return std::nullopt;
+    }
+
     errno = 0;
+
     char* pLastUsed;
     uint64_t parsedValue = std::strtoull(from.c_str(), &pLastUsed, 10);
-    if (errno == ERANGE) {
-      return std::nullopt;
-    }
-    if (errno != ERANGE && pLastUsed == from.c_str() + from.size()) {
-      // Successfully parsed the entire string as an integer of this type.
-      return CesiumUtility::losslessNarrow<TTo, uint64_t>(parsedValue);
+    if (errno != EINVAL && errno != ERANGE) {
+      // Check if the entire string was parsed.
+      if (pLastUsed == from.c_str() + from.size()) {
+        // Successfully parsed the entire string as an integer of this type.
+        return CesiumUtility::losslessNarrow<TTo, uint64_t>(parsedValue);
+      }
     }
 
-    errno = 0;
     // Failed to parse as an integer. Maybe we can parse as a double and
     // truncate it?
-    double parsedDouble = std::strtod(from.c_str(), &pLastUsed);
-    if (errno != ERANGE && pLastUsed == from.c_str() + from.size()) {
-      // Successfully parsed the entire string as a double.
-      // Convert it to an integer if we can.
-      double truncated = glm::trunc(parsedDouble);
+    errno = 0;
 
-      uint64_t asInteger = static_cast<uint64_t>(truncated);
-      double roundTrip = static_cast<double>(asInteger);
-      if (roundTrip == truncated) {
-        return CesiumUtility::losslessNarrow<TTo, uint64_t>(asInteger);
+    double parsedDouble = std::strtod(from.c_str(), &pLastUsed);
+    if (errno != EINVAL && errno != ERANGE) {
+      // Check if the entire string was parsed.
+      if (pLastUsed == from.c_str() + from.size()) {
+        // Successfully parsed the entire string as a double.
+        // Convert it to an integer if we can.
+        double truncated = glm::trunc(parsedDouble);
+
+        uint64_t asInteger = static_cast<uint64_t>(truncated);
+        double roundTrip = static_cast<double>(asInteger);
+        if (roundTrip == truncated) {
+          return CesiumUtility::losslessNarrow<TTo, uint64_t>(asInteger);
+        }
       }
     }
 
-    errno = 0;
     return std::nullopt;
   }
 };

CesiumGltf/test/TestMetadataConversions.cpp

@@ -144,9 +144,31 @@ TEST_CASE("Test MetadataConversions for integer") {
         std::numeric_limits<double>::max()));
   }
 
+  SECTION("returns std::nullopt for invalid strings") {
+    // out-of-range number
+    REQUIRE(!MetadataConversions<uint8_t, std::string>::convert("-1"));
+    // mixed number and non-number input
+    REQUIRE(!MetadataConversions<int8_t, std::string>::convert("10 hello"));
+    // non-number input
+    REQUIRE(
+        !MetadataConversions<uint8_t, std::string>::convert("not a number"));
+    // empty input
+    REQUIRE(!MetadataConversions<int8_t, std::string>::convert(""));
+
+    // extra tests for proper string parsing
+    REQUIRE(!MetadataConversions<uint64_t, std::string>::convert("-1"));
+    REQUIRE(!MetadataConversions<uint64_t, std::string>::convert(
+        "184467440737095515000"));
+
+    REQUIRE(!MetadataConversions<int64_t, std::string>::convert(
+        "-111111111111111111111111111111111"));
+    REQUIRE(!MetadataConversions<int64_t, std::string>::convert(
+        "111111111111111111111111111111111"));
+  }
+
   SECTION("returns std::nullopt for invalid string views") {
     // out-of-range number
-    REQUIRE(!MetadataConversions<uint64_t, std::string_view>::convert(
+    REQUIRE(!MetadataConversions<uint8_t, std::string_view>::convert(
         std::string_view("-1")));
     // mixed number and non-number input
     REQUIRE(!MetadataConversions<int8_t, std::string_view>::convert(
@@ -159,18 +181,6 @@ TEST_CASE("Test MetadataConversions for integer") {
         std::string_view()));
   }
 
-  SECTION("returns std::nullopt for invalid strings") {
-    // out-of-range number
-    REQUIRE(!MetadataConversions<uint64_t, std::string>::convert("-1"));
-    // mixed number and non-number input
-    REQUIRE(!MetadataConversions<int8_t, std::string>::convert("10 hello"));
-    // non-number input
-    REQUIRE(
-        !MetadataConversions<uint8_t, std::string>::convert("not a number"));
-    // empty input
-    REQUIRE(!MetadataConversions<int8_t, std::string>::convert(""));
-  }
-
   SECTION("returns std::nullopt for incompatible types") {
     // vecN
     REQUIRE(!MetadataConversions<int32_t, glm::ivec3>::convert(