Issue #1874 Cast to unsigned char for isspace. (#1888)

* Issue #1874 Cast to unsigned char for isspace. Signed-off-by: pylee <[email protected]> * Add unit test. Signed-off-by: pylee <[email protected]> * Add test comment as suggested in code review. Signed-off-by: pylee <[email protected]> --------- Signed-off-by: pylee <[email protected]> Signed-off-by: Doug Walker <[email protected]> Co-authored-by: Doug Walker <[email protected]>
AcademySoftwareFoundation · Nov 5, 2023 · ed85207 · ed85207
1 parent 45544ce
commit ed85207
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 2 deletions.
diff --git a/src/utils/StringUtils.h b/src/utils/StringUtils.h
@@ -106,7 +106,7 @@ inline std::string LeftTrim(std::string str, char c)
 // Starting from the left, trim all the space characters i.e. space, tabulation, etc.
 inline std::string LeftTrim(std::string str)
 {
-    const auto it = std::find_if(str.begin(), str.end(), [](char ch) { return !std::isspace(ch); });
+    const auto it = std::find_if(str.begin(), str.end(), [](char ch) { return !std::isspace(static_cast<unsigned char>(ch)); });
     str.erase(str.begin(), it);
     return str;
 }
@@ -123,7 +123,7 @@ inline std::string RightTrim(std::string str, char c)
 inline std::string RightTrim(std::string str)
 {
     const auto it =
-        std::find_if(str.rbegin(), str.rend(), [](char ch) { return !std::isspace(ch); });
+        std::find_if(str.rbegin(), str.rend(), [](char ch) { return !std::isspace(static_cast<unsigned char>(ch)); });
     str.erase(it.base(), str.end());
     return str;
 }

diff --git a/tests/utils/StringUtils_tests.cpp b/tests/utils/StringUtils_tests.cpp
@@ -50,6 +50,12 @@ OCIO_ADD_TEST(StringUtils, trim)
         const std::string str = StringUtils::Trim(ref);
         OCIO_CHECK_EQUAL(str, "lOwEr 1*& ctfG");
     }
+
+    {
+        // Test that no assert happens when the Trim argument is not an unsigned char (see issue #1874).
+        constexpr char ref2[]{ -1, -2, -3, '\0' };
+        const std::string str = StringUtils::Trim(ref2);
+    }
 }
 
 OCIO_ADD_TEST(StringUtils, split)