Merge pull request #1725 from evoskuil/master

evoskuil · web-flow · commit 551a5f90ddeb · 2025-10-04T22:23:04.000-04:00
Fix handling of std::filesystem::path re:utf8-everywhere (win32).
diff --git a/Makefile.am b/Makefile.am
@@ -388,6 +388,7 @@ test_libbitcoin_system_test_SOURCES = \
     test/unicode/utf8_everywhere/environment.cpp \
     test/unicode/utf8_everywhere/ifstream.cpp \
     test/unicode/utf8_everywhere/ofstream.cpp \
+    test/unicode/utf8_everywhere/paths.cpp \
     test/unicode/utf8_everywhere/unicode_istream.cpp \
     test/unicode/utf8_everywhere/unicode_ostream.cpp \
     test/utreexo/utreexo.cpp \
diff --git a/builds/cmake/CMakeLists.txt b/builds/cmake/CMakeLists.txt
@@ -867,6 +867,7 @@ if (with-tests)
         "../../test/unicode/utf8_everywhere/environment.cpp"
         "../../test/unicode/utf8_everywhere/ifstream.cpp"
         "../../test/unicode/utf8_everywhere/ofstream.cpp"
+        "../../test/unicode/utf8_everywhere/paths.cpp"
         "../../test/unicode/utf8_everywhere/unicode_istream.cpp"
         "../../test/unicode/utf8_everywhere/unicode_ostream.cpp"
         "../../test/utreexo/utreexo.cpp"
diff --git a/builds/msvc/vs2022/libbitcoin-system-test/libbitcoin-system-test.vcxproj b/builds/msvc/vs2022/libbitcoin-system-test/libbitcoin-system-test.vcxproj
@@ -317,6 +317,7 @@
     <ClCompile Include="..\..\..\..\test\unicode\utf8_everywhere\environment.cpp" />
     <ClCompile Include="..\..\..\..\test\unicode\utf8_everywhere\ifstream.cpp" />
     <ClCompile Include="..\..\..\..\test\unicode\utf8_everywhere\ofstream.cpp" />
+    <ClCompile Include="..\..\..\..\test\unicode\utf8_everywhere\paths.cpp" />
     <ClCompile Include="..\..\..\..\test\unicode\utf8_everywhere\unicode_istream.cpp" />
     <ClCompile Include="..\..\..\..\test\unicode\utf8_everywhere\unicode_ostream.cpp" />
     <ClCompile Include="..\..\..\..\test\utreexo\utreexo.cpp" />
diff --git a/builds/msvc/vs2022/libbitcoin-system-test/libbitcoin-system-test.vcxproj.filters b/builds/msvc/vs2022/libbitcoin-system-test/libbitcoin-system-test.vcxproj.filters
@@ -615,6 +615,9 @@
     <ClCompile Include="..\..\..\..\test\unicode\utf8_everywhere\ofstream.cpp">
       <Filter>src\unicode\utf8_everywhere</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\..\..\test\unicode\utf8_everywhere\paths.cpp">
+      <Filter>src\unicode\utf8_everywhere</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\..\..\test\unicode\utf8_everywhere\unicode_istream.cpp">
       <Filter>src\unicode\utf8_everywhere</Filter>
     </ClCompile>
diff --git a/include/bitcoin/system/config/parser.hpp b/include/bitcoin/system/config/parser.hpp
@@ -39,6 +39,8 @@ class BC_API parser
         const std::string& message) NOEXCEPT;
     static bool get_option(variables_map& variables,
         const std::string& name) NOEXCEPT;
+
+    /// Path is read as u8string (c++20) and held internally as wide on win32.
     static std::filesystem::path get_config_option(variables_map& variables,
         const std::string& name) NOEXCEPT;
 
diff --git a/include/bitcoin/system/data/string.hpp b/include/bitcoin/system/data/string.hpp
@@ -41,7 +41,6 @@ BC_API std::string join(const string_list& tokens,
 
 /// Split text into tokens by the specified delimiter(s).
 /// Compression removes all but a last empty token (after trimming).
-/// Compression removes all but a last empty token (after trimming).
 BC_API string_list split(const std::string& text, bool compress=true) NOEXCEPT;
 BC_API string_list split(const std::string& text,
     const std::string& delimiter, bool trim=true, bool compress=true) NOEXCEPT;
diff --git a/include/bitcoin/system/unicode/utf8_everywhere/paths.hpp b/include/bitcoin/system/unicode/utf8_everywhere/paths.hpp
@@ -25,29 +25,25 @@
 namespace libbitcoin {
 namespace system {
 
+/// std::filesystem::path maintains path internally as unicode, constructing
+/// from the current ANSI code page on Win32. This must be avoided when using
+/// utf8-everywhere. These utilities simplify proper contruction and extraction.
+BC_API std::string cast_to_string(const std::u8string& value) NOEXCEPT;
+BC_API std::u8string cast_to_u8string(const std::string& value) NOEXCEPT;
+BC_API std::string from_path(const std::filesystem::path& value) NOEXCEPT;
+BC_API std::filesystem::path to_path(const std::string& value) NOEXCEPT;
+
 /// Get the default configuration file path with subdirectory.
 BC_API std::filesystem::path default_config_path(
     const std::filesystem::path& subdirectory) NOEXCEPT;
 
-/// These allow std::*fstream(to_extended_path(std::filesystem::path)),
-/// due to a vc++ fstream extension that accepts std::wstring path.
-
-#ifdef HAVE_MSC
-
-/// Convert to std::wstring path, extended for win32.
+/// Extend the path and return it as another.
+/// std::filesystem::path does not extend long paths, so we provide this.
 /// Not thread safe. If another thread calls SetCurrentDirectory during this
 /// call, the result may be corrupted as static storage is used for directory.
-BC_API std::wstring to_extended_path(
-    const std::filesystem::path& path) NOEXCEPT;
-
-#else
-
-/// Return std::string path, for consistency with std::wstring version.
-BC_API std::string to_extended_path(
+BC_API std::filesystem::path extended_path(
     const std::filesystem::path& path) NOEXCEPT;
 
-#endif
-
 } // namespace system
 } // namespace libbitcoin
 
diff --git a/src/config/parser.cpp b/src/config/parser.cpp
@@ -21,7 +21,8 @@
 #include <filesystem>
 #include <sstream>
 #include <bitcoin/system/data/data.hpp>
-#include <bitcoin/system/unicode/utf8_everywhere/ifstream.hpp>
+#include <bitcoin/system/unicode/unicode.hpp>
+#include <bitcoin/system/unicode/utf8_everywhere/utf8_everywhere.hpp>
 
 namespace libbitcoin {
 namespace system {
@@ -51,7 +52,8 @@ std::filesystem::path parser::get_config_option(variables_map& variables,
     if (config.empty())
         return {};
 
-    return config.as<std::filesystem::path>();
+    // Capture as utf8 so std::filesystem::path does assume ansi code page.
+    return { config.as<std::u8string>() };
 }
 
 bool parser::get_option(variables_map& variables,
@@ -88,27 +90,26 @@ void parser::load_environment_variables(variables_map& variables,
 bool parser::load_configuration_variables(variables_map& variables,
     const std::string& option_name) THROWS
 {
-    const auto config_settings = load_settings();
-    const auto config_path = get_config_option(variables, option_name);
+    const auto settings = load_settings();
+    const auto path = get_config_option(variables, option_name);
+    const auto extended = extended_path(path);
 
     // If the existence test errors out we pretend there's no file :/.
-    std::error_code code;
-    if (!config_path.empty() && exists(config_path, code))
+    std::error_code code{};
+    if (!path.empty() && std::filesystem::exists(extended, code))
     {
-        const auto& path = config_path.string();
-        ifstream file(path);
-
+        ifstream file{ path };
         if (!file.good())
-            throw ifstream_exception(path.c_str());
+            throw ifstream_exception{ from_path(extended).c_str() };
 
-        const auto config = parse_config_file(file, config_settings);
+        const auto config = parse_config_file(file, settings);
         store(config, variables);
         return true;
     }
 
     // Loading from an empty stream causes the defaults to populate.
-    std::istringstream stream;
-    const auto config = parse_config_file(stream, config_settings);
+    std::istringstream stream{};
+    const auto config = parse_config_file(stream, settings);
     store(config, variables);
     return false;
 }
diff --git a/src/unicode/utf8_everywhere/ifstream.cpp b/src/unicode/utf8_everywhere/ifstream.cpp
@@ -25,10 +25,9 @@
 namespace libbitcoin {
 namespace system {
  
-// VC++ EXTENSION: "construct with wide-named file".
 ifstream::ifstream(const std::filesystem::path& path,
     std::ifstream::openmode mode) THROWS
-  : std::ifstream(to_extended_path(path), mode)
+  : std::ifstream(extended_path(path), mode)
 {
     // This opens the stream, and creates file system file if out/app mode.
 }
diff --git a/src/unicode/utf8_everywhere/ofstream.cpp b/src/unicode/utf8_everywhere/ofstream.cpp
@@ -24,11 +24,10 @@
 
 namespace libbitcoin {
 namespace system {
-    
-// VC++ EXTENSION: "construct with wide-named file".
+
 ofstream::ofstream(const std::filesystem::path& path,
     std::ofstream::openmode mode) THROWS
-  : std::ofstream(to_extended_path(path), mode)
+  : std::ofstream(extended_path(path), mode)
 {
     // This opens the stream, and creates file system file if out/app mode.
 }
diff --git a/src/unicode/utf8_everywhere/paths.cpp b/src/unicode/utf8_everywhere/paths.cpp
@@ -29,13 +29,41 @@
 namespace libbitcoin {
 namespace system {
 
-#ifdef HAVE_MSC
-static std::string config_directory() NOEXCEPT
+std::string cast_to_string(const std::u8string& value) NOEXCEPT
+{
+    BC_PUSH_WARNING(NO_REINTERPRET_CAST)
+    return { reinterpret_cast<const char*>(value.c_str()) };
+    BC_POP_WARNING()
+}
+
+std::u8string cast_to_u8string(const std::string& value) NOEXCEPT
+{
+    BC_PUSH_WARNING(NO_REINTERPRET_CAST)
+    return { reinterpret_cast<const char8_t*>(value.c_str()) };
+    BC_POP_WARNING()
+}
+
+std::string from_path(const std::filesystem::path& value) NOEXCEPT
+{
+    BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
+    return cast_to_string(value.u8string());
+    BC_POP_WARNING()
+}
+
+std::filesystem::path to_path(const std::string& value) NOEXCEPT
+{
+    BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
+    return { cast_to_u8string(value) };
+    BC_POP_WARNING()
+}
+
+#if defined(HAVE_MSC)
+static std::wstring config_directory() NOEXCEPT
 {
     wchar_t directory[MAX_PATH];
     const auto result = SHGetFolderPathW(NULL, CSIDL_COMMON_APPDATA, NULL,
         SHGFP_TYPE_CURRENT, &directory[0]);
-    return SUCCEEDED(result) ? to_utf8(&directory[0]) : "";
+    return SUCCEEDED(result) ? &directory[0] : L"";
 }
 #elif defined(SYSCONFDIR)
 static std::string config_directory() NOEXCEPT { return SYSCONFDIR; }
@@ -47,30 +75,33 @@ std::filesystem::path default_config_path(
     const std::filesystem::path& subdirectory) NOEXCEPT
 {
     BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
-    static const auto directory = std::filesystem::path{ config_directory() };
-    return directory / subdirectory;
+    static const auto folder = std::filesystem::path{ config_directory() };
+    return folder / subdirectory;
     BC_POP_WARNING()
 }
 
-#ifdef HAVE_MSC
+#if defined(HAVE_MSC)
 
-// docs.microsoft.com/windows/win32/api/fileapi/nf-fileapi-getfullpathnamew
-static std::wstring to_fully_qualified_path(
-    const std::filesystem::path& path) NOEXCEPT
+// Helper for to_fully_qualified_path.
+inline auto replace_all(std::wstring text, wchar_t from, wchar_t to) NOEXCEPT
 {
-    const auto replace_all = [](std::string text, char from, char to) NOEXCEPT
+    for (auto position = text.find(from); position != std::string::npos;
+        position = text.find(from, add1(position)))
     {
-        for (auto position = text.find(from); position != std::string::npos;
-            position = text.find(from, position + sizeof(char)))
-        {
-            text.replace(position, sizeof(char), { to });
-        }
+        text.replace(position, one, { to });
+    }
 
-        return text;
-    };
+    return text;
+};
 
+// docs.microsoft.com/windows/win32/api/fileapi/nf-fileapi-getfullpathnamew
+static std::wstring to_fully_qualified_path(
+    const std::filesystem::path& path) NOEXCEPT
+{
     // Separator normalization required by use of length extender.
-    const auto normal = to_utf16(replace_all(path.string(), '/', '\\'));
+    BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
+    const auto normal = replace_all(path.wstring(), '/', '\\');
+    BC_POP_WARNING()
 
     // GetFullPathName is not thread safe. If another thread calls
     // SetCurrentDirectory during a call of GetFullPathName the value may be
@@ -93,26 +124,20 @@ static std::wstring to_fully_qualified_path(
     return { directory.begin(), std::next(directory.begin(), size) };
 }
 
-// Use to_extended_path with APIs that compile to wide with HAVE_MSC defined
-// and to UTF8 with HAVE_MSC undefined. This includes some boost APIs and some
-// Win32 API extensions to std libs - such as std::ofstream and std::ifstream.
-// Otherwise use in any Win32 (W) APIs with HAVE_MSC defined, such as we do in 
-// interprocess_lock::open_file -> CreateFileW, since the boost wrapper only
-// calls CreateFileA. The length extension prefix requires Win32 (W) APIs.
-std::wstring to_extended_path(const std::filesystem::path& path) NOEXCEPT
+std::filesystem::path extended_path(const std::filesystem::path& path) NOEXCEPT
 {
     // The length extension prefix works only with a fully-qualified path.
     // However this includes "considered relative" paths (with ".." segments).
     // That is of no consequence here because those will also be converted.
     const auto full = to_fully_qualified_path(path);
-    return (full.length() > MAX_PATH) ? L"\\\\?\\" + full : full;
+    return { (full.length() > MAX_PATH) ? L"\\\\?\\" + full : full };
 }
 
 #else
 
-std::string to_extended_path(const std::filesystem::path& path) NOEXCEPT
+std::filesystem::path extended_path(const std::filesystem::path& path) NOEXCEPT
 {
-    return path.string();
+    return path;
 }
 
 #endif // HAVE_MSC
diff --git a/test/test.cpp b/test/test.cpp
@@ -42,7 +42,7 @@ bool clear(const std::filesystem::path& file_directory) NOEXCEPT
     // remove_all returns count removed, and error code if fails.
     // create_directories returns true if path exists or created.
     // used for setup, with no expectations of file/directory existence.
-    const auto path = to_extended_path(file_directory);
+    const auto path = extended_path(file_directory);
     code ec;
     BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
     std::filesystem::remove_all(path, ec);
@@ -54,7 +54,7 @@ bool create(const std::filesystem::path& file_path) NOEXCEPT
 {
     // Creates and returns true if file already existed (and no error).
     BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
-    std::ofstream file(to_extended_path(file_path));
+    std::ofstream file(extended_path(file_path));
     return file.good();
     BC_POP_WARNING()
 }
@@ -63,7 +63,7 @@ bool exists(const std::filesystem::path& file_path) NOEXCEPT
 {
     // Returns true only if file existed.
     BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
-    std::ifstream file(to_extended_path(file_path));
+    std::ifstream file(extended_path(file_path));
     return file.good();
     BC_POP_WARNING()
 }
@@ -73,7 +73,7 @@ bool remove(const std::filesystem::path& file_path) NOEXCEPT
     // Deletes and returns false if file did not exist (or error).
     code ec;
     BC_PUSH_WARNING(NO_THROW_IN_NOEXCEPT)
-    return std::filesystem::remove(to_extended_path(file_path), ec);
+    return std::filesystem::remove(extended_path(file_path), ec);
     BC_POP_WARNING()
 }
 
diff --git a/test/unicode/conversion.cpp b/test/unicode/conversion.cpp
@@ -18,9 +18,12 @@
  */
 #include "../test.hpp"
 
+// MSVC literals in a UTF8 source file.
 // Use of L and U is not recommended as it will only work for ASCII when
 // the source file does not have a BOM (which we avoid for other reasons).
-// We use it below to simplify creation of ASCII test vectors.
+// We use it below to simplify creation of ASCII test vectors. The prefix
+// types the data but doesn't convert it at compile time (interestingly
+// intellisense picks up the conversion propery.
 
 BOOST_AUTO_TEST_SUITE(conversion_tests)
 
diff --git a/test/unicode/utf8_everywhere/environment.cpp b/test/unicode/utf8_everywhere/environment.cpp
@@ -356,17 +356,7 @@ BOOST_AUTO_TEST_CASE(utf8_environment__allocate_environment_args__null_terminati
     free_environment(narrow_args);
 }
 
-// to_extended_path
-
-////BOOST_AUTO_TEST_CASE(utf8_environment__to_extended_path__always__expected)
-////{
-////    ////std::string to_extended_path(const path& path);
-////}
-////
-////BOOST_AUTO_TEST_CASE(utf8_environment__to_extended_path16__always__expected)
-////{
-////    ////std::wstring to_extended_path16(const std::string& path);
-////}
+// extended_path
 
 #endif // HAVE_MSC
 
diff --git a/test/unicode/utf8_everywhere/paths.cpp b/test/unicode/utf8_everywhere/paths.cpp

Original file line number	Diff line number	Diff line change
`@@ -25,10 +25,9 @@`
`25`	`25`	`namespace libbitcoin {`
`26`	`26`	`namespace system {`
`27`	`27`
`28`		`-// VC++ EXTENSION: "construct with wide-named file".`
`29`	`28`	`ifstream::ifstream(const std::filesystem::path& path,`
`30`	`29`	`std::ifstream::openmode mode) THROWS`
`31`		`- : std::ifstream(to_extended_path(path), mode)`
	`30`	`+ : std::ifstream(extended_path(path), mode)`
`32`	`31`	`{`
`33`	`32`	`// This opens the stream, and creates file system file if out/app mode.`
`34`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,11 +24,10 @@`
`24`	`24`
`25`	`25`	`namespace libbitcoin {`
`26`	`26`	`namespace system {`
`27`		`-`
`28`		`-// VC++ EXTENSION: "construct with wide-named file".`
	`27`	`+`
`29`	`28`	`ofstream::ofstream(const std::filesystem::path& path,`
`30`	`29`	`std::ofstream::openmode mode) THROWS`
`31`		`- : std::ofstream(to_extended_path(path), mode)`
	`30`	`+ : std::ofstream(extended_path(path), mode)`
`32`	`31`	`{`
`33`	`32`	`// This opens the stream, and creates file system file if out/app mode.`
`34`	`33`	`}`