Use throw rather than abort() for guid parse failures (#992)

* Use throw rather than abort() for guid parse failures Make `winrt::guid("...")` more useful at runtime by throwing on failure instead of aborting the program. * Remove noexcept, add tests * Revert operator== changes, revise tests Co-authored-by: Kenny Kerr <[email protected]>
microsoft · Aug 4, 2021 · 4ba22a6 · 4ba22a6
1 parent 20f0962
commit 4ba22a6
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 8 deletions.
diff --git a/strings/base_types.h b/strings/base_types.h
@@ -20,7 +20,7 @@ namespace winrt::impl
     };
 
     template <typename T>
-    constexpr uint8_t hex_to_uint(T const c) noexcept
+    constexpr uint8_t hex_to_uint(T const c)
     {
         if (c >= '0' && c <= '9')
         {
@@ -36,22 +36,22 @@ namespace winrt::impl
         }
         else 
         {
-            abort();
+            throw std::invalid_argument("Character is not a hexadecimal digit");
         }
     }
 
     template <typename T>
-    constexpr uint8_t hex_to_uint8(T const a, T const b) noexcept
+    constexpr uint8_t hex_to_uint8(T const a, T const b)
     {
         return (hex_to_uint(a) << 4) | hex_to_uint(b);
     }
 
-    constexpr uint16_t uint8_to_uint16(uint8_t a, uint8_t b) noexcept
+    constexpr uint16_t uint8_to_uint16(uint8_t a, uint8_t b)
     {
         return (static_cast<uint16_t>(a) << 8) | static_cast<uint16_t>(b);
     }
 
-    constexpr uint32_t uint8_to_uint32(uint8_t a, uint8_t b, uint8_t c, uint8_t d) noexcept
+    constexpr uint32_t uint8_to_uint32(uint8_t a, uint8_t b, uint8_t c, uint8_t d)
     {
         return (static_cast<uint32_t>(uint8_to_uint16(a, b)) << 16) |
                 static_cast<uint32_t>(uint8_to_uint16(c, d));
@@ -85,11 +85,11 @@ WINRT_EXPORT namespace winrt
     private:
 
         template <typename TStringView>
-        static constexpr guid parse(TStringView const value) noexcept
+        static constexpr guid parse(TStringView const value)
         {
             if (value.size() != 36 || value[8] != '-' || value[13] != '-' || value[18] != '-' || value[23] != '-')
             {
-                abort();
+                throw std::invalid_argument("value is not a valid GUID string");
             }
 
             return
@@ -179,7 +179,7 @@ WINRT_EXPORT namespace winrt
     {
         return !(left == right);
     }
-    
+
     inline bool operator<(guid const& left, guid const& right) noexcept
     {
         return memcmp(&left, &right, sizeof(left)) < 0;

diff --git a/test/test/guid.cpp b/test/test/guid.cpp
@@ -0,0 +1,29 @@
+#include "pch.h"
+
+TEST_CASE("guid")
+{
+    constexpr winrt::guid expected{ 0x00112233, 0x4455, 0x6677, { 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff } };
+
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data1 == expected.Data1);
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data2 == expected.Data2);
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data3 == expected.Data3);
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data4[0] == expected.Data4[0]);
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data4[1] == expected.Data4[1]);
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data4[2] == expected.Data4[2]);
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data4[3] == expected.Data4[3]);
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data4[4] == expected.Data4[4]);
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data4[5] == expected.Data4[5]);
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data4[6] == expected.Data4[6]);
+    STATIC_REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff").Data4[7] == expected.Data4[7]);
+
+    REQUIRE(winrt::guid("00112233-4455-6677-8899-aabbccddeeff") == expected);
+    REQUIRE(winrt::guid({ "{00112233-4455-6677-8899-aabbccddeeff}" + 1, 36 }) == expected);
+
+    REQUIRE_THROWS_AS(winrt::guid(""), std::invalid_argument);
+    REQUIRE_THROWS_AS(winrt::guid("not a guid"), std::invalid_argument);
+    REQUIRE_THROWS_AS(winrt::guid("same length string that's not a guid"), std::invalid_argument);
+    REQUIRE_THROWS_AS(winrt::guid("too long string that's also not a guid"), std::invalid_argument);
+    REQUIRE_THROWS_AS(winrt::guid("00112233-4455-6677-8899-aabbccddeeff with extra"), std::invalid_argument);
+    REQUIRE_THROWS_AS(winrt::guid("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"), std::invalid_argument);
+    REQUIRE_THROWS_AS(winrt::guid("{00112233-4455-6677-8899-aabbccddeeff}"), std::invalid_argument);
+}
diff --git a/test/test/test.vcxproj b/test/test/test.vcxproj
@@ -335,6 +335,7 @@
     <ClCompile Include="delegates.cpp" />
     <ClCompile Include="disconnected.cpp" />
     <ClCompile Include="enum.cpp" />
+    <ClCompile Include="guid.cpp" />
     <ClCompile Include="hresult_class_not_registered.cpp" />
     <ClCompile Include="error_info.cpp" />
     <ClCompile Include="event_deferral.cpp" />