Fixes Unicode paths on Windows (#1363) (#1527)

* Fixes Unicode paths on Windows

Unlike most other platforms, Windows' Unicode is standardized around
UTF-16, an encoding not compatible with "char *" arrays common in
C/C++. As such, to support Unicode correctly when using Win32 APIs,
strings must be converted to and from UTF-16 and the Unicode
versions of the APIs must be used over the ANSI versions.

This commit introduces the following:

- Utility funcions for converting between UTF-8 and UTF-16LE on all
  platforms:
  - Platform::Utf8ToUtf16
  - Platform::Utf16ToUtf8

- Adds test for these conversion functions to ensure the conversion
  to and from UTF-8 and UTF-16LE is correct.

- Utility wrappers for "std::ifstream" that automatically convert
  to and from UTF-16 so that filenames requiring Unicode encoding
  function correctly:
  - Platform::CreateInputFileStream
  - Platform::OpenInputFileStream

- Moves the file default compute hash function to the Platform
  class (Platform::CreateFileContentHash) and switches to the
  Win32 UTF-16 variant on Windows.

- Adds the "UNICODE" macro before including "Windows.h" which
  ensures all functions called are the Unicode variants instead of
  the default ANSI variants.
  - Implicitly changes functions such as GetEnvironmentVariable and
    SetEnvironmentVariable to their Unicode variants.

- Changes the following environment variable related functions to
  their Win32 Unicode variants on Windows:
  - environ -> _wenviron
  - _putenv_s -> _wputenv_s

- Updates tests using SetEnvironmentVariable to use wide string
  literals since that function has been switched to the Unicode
  variant.

Signed-off-by: itsmattkc <[email protected]>

* Moved UNICODE and _UNICODE definitions to CMake

Signed-off-by: itsmattkc <[email protected]>

* handle empty strings and use const references

Signed-off-by: itsmattkc <[email protected]>

* only use wenviron when unicode is enabled

Signed-off-by: itsmattkc <[email protected]>

* add ANSI Win32 version of Getenv

Signed-off-by: itsmattkc <[email protected]>

* Add CMake option for compiling with Win32 Unicode support

Signed-off-by: itsmattkc <[email protected]>

* Throw exception if UTF functions are called on non-Windows platforms

Signed-off-by: itsmattkc <[email protected]>

* Throw OCIO Exception

Signed-off-by: itsmattkc <[email protected]>

* Don't run UTF-8/16 conversion test on non-Windows

Signed-off-by: itsmattkc <[email protected]>

* Fix CMake error

Signed-off-by: itsmattkc <[email protected]>

* minor CMake adjustment

Signed-off-by: itsmattkc <[email protected]>

* Clarified CMake option for Win32 unicode

Signed-off-by: itsmattkc <[email protected]>

* minor improvements

Signed-off-by: itsmattkc <[email protected]>

* fix macro in cpu test cmake

Signed-off-by: itsmattkc <[email protected]>

Co-authored-by: itsmattkc <[email protected]>

Author: hodoulp

CMakeLists.txt

@@ -139,6 +139,8 @@ option(OCIO_BUILD_JAVA "Specify whether to build java bindings" OFF)
 
 option(OCIO_WARNING_AS_ERROR "Set build error level for CI testing" OFF)
 
+option(OCIO_USE_WINDOWS_UNICODE "On Windows only, compile with Unicode support" WIN32)
+
 
 ###############################################################################
 # Optimization / internal linking preferences

src/OpenColorIO/CMakeLists.txt

@@ -280,6 +280,15 @@ if(WIN32)
         PRIVATE
             XML_STATIC
     )
+
+    if (OCIO_USE_WINDOWS_UNICODE)
+        # Add Unicode definitions to use Unicode functions
+        target_compile_definitions(OpenColorIO
+            PRIVATE
+                UNICODE
+                _UNICODE
+        )
+    endif()
 endif()
 
 set_target_properties(OpenColorIO PROPERTIES

src/OpenColorIO/Config.cpp

@@ -1117,7 +1117,7 @@ ConstConfigRcPtr Config::CreateFromFile(const char * filename)
         throw ExceptionMissingFile ("The config filepath is missing.");
     }
 
-    std::ifstream istream(filename);
+    std::ifstream istream = Platform::CreateInputFileStream(filename, std::ios_base::in);
     if (istream.fail())
     {
         std::ostringstream os;

src/OpenColorIO/ContextVariableUtils.cpp

@@ -6,6 +6,7 @@
 
 #include "ContextVariableUtils.h"
 #include "utils/StringUtils.h"
+#include "Platform.h"
 
 
 #if defined(__APPLE__) && !defined(__IPHONE__)
@@ -19,6 +20,12 @@ extern char ** environ;
 namespace
 {
 
+#if defined(_WIN32) && defined(UNICODE)
+inline wchar_t ** GetEnviron()
+{
+    return _wenviron;
+}
+#else
 inline char ** GetEnviron()
 {
 #if __IPHONE__
@@ -30,6 +37,7 @@ inline char ** GetEnviron()
     return environ;
 #endif
 }
+#endif
 
 } // anon.
 
@@ -71,11 +79,28 @@ void LoadEnvironment(EnvMap & map, bool update)
 {
     // First, add or update the context variables with existing env. variables.
 
+#if defined(_WIN32) && defined(UNICODE)
+    if (GetEnviron() == NULL) {
+        // If the program starts with "main" instead of "wmain", then wenviron returns NULL until
+        // the first call to either wgetenv or wputenv. Calling wgetenv, even with an empty
+        // variable name, will populate wenviron correctly. We also use wgetenv_s (which requires
+        // a valid size pointer) to suppress safety warnings about wgetenv during the compile.
+        size_t sz;
+        _wgetenv_s(&sz, NULL, 0, L"");
+    }
+
+    for (wchar_t **env = GetEnviron(); *env != NULL; ++env)
+    {
+        // Split environment up into std::map[name] = value.
+
+        const std::string env_str = Platform::Utf16ToUtf8((wchar_t*)*env);
+#else
     for (char **env = GetEnviron(); *env != NULL; ++env)
     {
         // Split environment up into std::map[name] = value.
 
         const std::string env_str = (char*)*env;
+#endif
         const int pos = static_cast<int>(env_str.find_first_of('='));
 
         const std::string name  = env_str.substr(0, pos);

src/OpenColorIO/PathUtils.cpp

@@ -4,12 +4,12 @@
 
 #include <iostream>
 #include <map>
-#include <sys/stat.h>
 
 #include <OpenColorIO/OpenColorIO.h>
 
 #include "Mutex.h"
 #include "PathUtils.h"
+#include "Platform.h"
 #include "pystring/pystring.h"
 #include "utils/StringUtils.h"
 
@@ -26,39 +26,9 @@ namespace OCIO_NAMESPACE
 {
 namespace
 {
-// Here is the explanation of the stat() method:
-// https://pubs.opengroup.org/onlinepubs/009695299/basedefs/sys/stat.h.html
-// "The st_ino and st_dev fields taken together uniquely identify the file within the system."
-//
-// However there are limitations to the stat() support on some Windows file systems:
-// https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions?redirectedfrom=MSDN&view=vs-2019
-// "The inode, and therefore st_ino, has no meaning in the FAT, HPFS, or NTFS file systems."
-
-// That's the default hash method implementation to compute a hash key based on a file content.
-std::string DefaultComputeHash(const std::string &filename)
-{
-    struct stat fileInfo;
-    if (stat(filename.c_str(), &fileInfo) == 0)
-    {
-        // Treat the st_dev (i.e. device) + st_ino (i.e. inode) as a proxy for the contents.
-
-        std::ostringstream fasthash;
-        fasthash << fileInfo.st_dev << ":";
-#ifdef _WIN32
-        // TODO: The hard-linked files are then not correctly supported on Windows platforms.
-        fasthash << std::hash<std::string>{}(filename);
-#else
-        fasthash << fileInfo.st_ino;
-#endif
-        return fasthash.str();
-    }
-
-    return "";
-}
-
 // The global variable holds the hash function to use.
 // It could be changed using SetComputeHashFunction() to customize the implementation.
-ComputeHashFunction g_hashFunction = DefaultComputeHash;
+ComputeHashFunction g_hashFunction = Platform::CreateFileContentHash;
 
 // We mutex both the main map and each item individually, so that
 // the potentially slow stat calls dont block other lookups to already
@@ -86,7 +56,7 @@ void SetComputeHashFunction(ComputeHashFunction hashFunction)
 
 void ResetComputeHashFunction()
 {
-    g_hashFunction = DefaultComputeHash;
+    g_hashFunction = Platform::CreateFileContentHash;
 }
 
 std::string GetFastFileHash(const std::string & filename)

src/OpenColorIO/Platform.cpp

@@ -1,8 +1,11 @@
 // SPDX-License-Identifier: BSD-3-Clause
 // Copyright Contributors to the OpenColorIO Project.
 
+#include <codecvt>
+#include <locale>
 #include <random>
 #include <sstream>
+#include <sys/stat.h>
 #include <vector>
 
 #include <OpenColorIO/OpenColorIO.h>
@@ -49,12 +52,36 @@ bool Getenv(const char * name, std::string & value)
         return false;
     }
 
-#ifdef _WIN32
-    if(uint32_t size = GetEnvironmentVariable(name, nullptr, 0))
+#if defined(_WIN32)
+    // Define working strings, converting to UTF-16 if necessary
+#ifdef UNICODE
+    std::wstring name_str = Utf8ToUtf16(name);
+    std::wstring value_str;
+#else
+    std::string name_str = name;
+    std::string value_str;
+#endif
+
+    if(uint32_t size = GetEnvironmentVariable(name_str.c_str(), nullptr, 0))
     {
-        std::vector<char> buffer(size);
-        GetEnvironmentVariable(name, buffer.data(), size);
-        value = std::string(buffer.data());
+        value_str.resize(size);
+
+        GetEnvironmentVariable(name_str.c_str(), &value_str[0], size);
+
+        // GetEnvironmentVariable is designed for raw pointer strings and therefore requires that
+        // the destination buffer be long enough to place a null terminator at the end of it. Since
+        // we're using std::wstrings here, the null terminator is unnecessary (and causes false
+        // negatives in unit tests since the extra character makes it "non-equal" to normally
+        // defined std::wstrings). Therefore, we pop the last character off (the null terminator)
+        // to ensure that the string conforms to expectations.
+        value_str.pop_back();
+
+        // Return value, converting to UTF-8 if necessary
+#ifdef UNICODE
+        value = Utf16ToUtf8(value_str);
+#else
+        value = value_str;
+#endif
         return true;
     }
     else
@@ -81,7 +108,13 @@ void Setenv(const char * name, const std::string & value)
     // exists. To avoid the ambiguity, use Unsetenv() when the env. variable removal if needed.
 
 #ifdef _WIN32
+
+#ifdef UNICODE
+    _wputenv_s(Utf8ToUtf16(name).c_str(), Utf8ToUtf16(value).c_str());
+#else
     _putenv_s(name, value.c_str());
+#endif
+
 #else
     ::setenv(name, value.c_str(), 1);
 #endif
@@ -95,8 +128,14 @@ void Unsetenv(const char * name)
     }
 
 #ifdef _WIN32
+
+#ifdef UNICODE
     // Note that the Windows _putenv_s() removes the env. variable if the value is empty.
+    _wputenv_s(Utf8ToUtf16(name).c_str(), L"");
+#else
     _putenv_s(name, "");
+#endif
+
 #else
     ::unsetenv(name);
 #endif
@@ -203,6 +242,91 @@ std::string CreateTempFilename(const std::string & filenameExt)
     return filename;
 }
 
+std::ifstream CreateInputFileStream(const char * filename, std::ios_base::openmode mode)
+{
+#if defined(_WIN32) && defined(UNICODE)
+    return std::ifstream(Utf8ToUtf16(filename).c_str(), mode);
+#else
+    return std::ifstream(filename, mode);
+#endif
+}
+
+void OpenInputFileStream(std::ifstream & stream, const char * filename, std::ios_base::openmode mode)
+{
+#if defined(_WIN32) && defined(UNICODE)
+    stream.open(Utf8ToUtf16(filename).c_str(), mode);
+#else
+    stream.open(filename, mode);
+#endif
+}
+
+std::wstring Utf8ToUtf16(const std::string & str)
+{
+    if (str.empty()) {
+        return std::wstring();
+    }
+
+#ifdef _WIN32
+    int sz = MultiByteToWideChar(CP_UTF8, 0, &str[0], (int)str.size(), NULL, 0);
+    std::wstring wstr(sz, 0);
+    MultiByteToWideChar(CP_UTF8, 0, &str[0], (int)str.size(), &wstr[0], sz);
+    return wstr;
+#else
+    throw Exception("Only supported by the Windows platform.");
+#endif
+}
+
+std::string Utf16ToUtf8(const std::wstring & wstr)
+{
+    if (wstr.empty()) {
+        return std::string();
+    }
+
+#ifdef _WIN32
+    int sz = WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), NULL, 0, NULL, NULL);
+    std::string str(sz, 0);
+    WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), &str[0], sz, NULL, NULL);
+    return str;
+#else
+    throw Exception("Only supported by the Windows platform.");
+#endif
+}
+
+// Here is the explanation of the stat() method:
+// https://pubs.opengroup.org/onlinepubs/009695299/basedefs/sys/stat.h.html
+// "The st_ino and st_dev fields taken together uniquely identify the file within the system."
+//
+// However there are limitations to the stat() support on some Windows file systems:
+// https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions?redirectedfrom=MSDN&view=vs-2019
+// "The inode, and therefore st_ino, has no meaning in the FAT, HPFS, or NTFS file systems."
+
+// That's the default hash method implementation to compute a hash key based on a file content.
+std::string CreateFileContentHash(const std::string &filename)
+{
+#if defined(_WIN32) && defined(UNICODE)
+    struct _stat fileInfo;
+    if (_wstat(Platform::Utf8ToUtf16(filename).c_str(), &fileInfo) == 0)
+#else
+    struct stat fileInfo;
+    if (stat(filename.c_str(), &fileInfo) == 0)
+#endif
+    {
+        // Treat the st_dev (i.e. device) + st_ino (i.e. inode) as a proxy for the contents.
+
+        std::ostringstream fasthash;
+        fasthash << fileInfo.st_dev << ":";
+#ifdef _WIN32
+        // TODO: The hard-linked files are then not correctly supported on Windows platforms.
+        fasthash << std::hash<std::string>{}(filename);
+#else
+        fasthash << fileInfo.st_ino;
+#endif
+        return fasthash.str();
+    }
+
+    return "";
+}
+
 
 
 } // Platform

src/OpenColorIO/Platform.h

@@ -20,6 +20,7 @@
 #endif // _WIN32
 
 
+#include <fstream>
 #include <string>
 
 
@@ -28,6 +29,20 @@
 
 #define sscanf sscanf_s
 
+// Define std::tstring as a wstring when Unicode is enabled and a regular string otherwise
+namespace std
+{
+#ifdef _UNICODE
+typedef wstring tstring;
+typedef wostringstream tostringstream;
+#define LogDebugT(x) LogDebug(Platform::Utf16ToUtf8(x))
+#else
+typedef string tstring;
+typedef ostringstream tostringstream;
+#define LogDebugT(x) LogDebug(x)
+#endif
+}
+
 #endif // _WIN32
 
 
@@ -73,6 +88,21 @@ void AlignedFree(void * memBlock);
 //       the file if created.
 std::string CreateTempFilename(const std::string & filenameExt);
 
+// Create an input file stream (std::ifstream) using a UTF-8 filename on any platform.
+std::ifstream CreateInputFileStream(const char * filename, std::ios_base::openmode mode);
+
+// Open an input file stream (std::ifstream) using a UTF-8 filename on any platform.
+void OpenInputFileStream(std::ifstream & stream, const char * filename, std::ios_base::openmode mode);
+
+// Create a unique hash of a file provided as a UTF-8 filename on any platform.
+std::string CreateFileContentHash(const std::string &filename);
+
+// Convert UTF-8 string to UTF-16LE.
+std::wstring Utf8ToUtf16(const std::string & str);
+
+// Convert UTF-16LE string to UTF-8.
+std::string Utf16ToUtf8(const std::wstring & str);
+
 }
 
 } // namespace OCIO_NAMESPACE