From 144405160af9d9ad9bdf37df41295c5bc111f29f Mon Sep 17 00:00:00 2001 From: Codex Date: Tue, 10 Nov 2026 01:19:06 -0800 Subject: [PATCH 1/4] fix(plugin-catalog): preserve Unicode paths in failure diagnostics --- CHANGELOG.md | 6 ++++ pj_plugins/src/plugin_catalog.cpp | 16 ++++++----- pj_plugins/tests/plugin_catalog_test.cpp | 35 ++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5290b1b..8c97ffe7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,12 @@ SceneEntities and PlotMarkers: - `PlotMarkers`: the decoder drops a marker whose `kind` it does not know instead of drawing it as a time region; wire numbers unchanged. +Plugin catalog: + +- Preserve Unicode paths in plugin catalog failure diagnostics on Windows without + converting through the active code page. Invalid plugins and directories now + return their intended errors instead of throwing during path formatting. + ## [0.40.0] Host contract: extended: pj.mcp_endpoint.v1, PJ_mcp_endpoint_host_vtable_t::acquire_endpoint, diff --git a/pj_plugins/src/plugin_catalog.cpp b/pj_plugins/src/plugin_catalog.cpp index 83bbd32c..7c18c9a1 100644 --- a/pj_plugins/src/plugin_catalog.cpp +++ b/pj_plugins/src/plugin_catalog.cpp @@ -44,7 +44,7 @@ struct ManifestCandidate { }; bool hasDsoSuffix(const std::filesystem::path& path) { - return path.extension().string() == kDsoSuffix; + return detail::pathForLegacyAccessor(path.extension()) == kDsoSuffix; } bool nativeSymbolPresent(void* handle, const char* symbol) noexcept { @@ -402,13 +402,13 @@ std::string_view toString(PluginFamily family) noexcept { Expected inspectPluginDso(const std::filesystem::path& dso_path) { if (!hasDsoSuffix(dso_path)) { - return unexpected(fmt::format("not a platform plugin DSO: {}", dso_path.string())); + return unexpected(fmt::format("not a platform plugin DSO: {}", detail::pathForLegacyAccessor(dso_path))); } detail::LibraryPathIdentity recorded_path; auto raw_handle = detail::loadLibraryHandle(dso_path, &recorded_path); if (!raw_handle) { - return unexpected(fmt::format("{}: {}", dso_path.string(), raw_handle.error())); + return unexpected(fmt::format("{}: {}", detail::pathForLegacyAccessor(dso_path), raw_handle.error())); } return inspectPluginDso(detail::adoptLibraryHandle(*raw_handle), recorded_path.load_path); } @@ -416,9 +416,11 @@ Expected inspectPluginDso(const std::filesystem::path& dso_pat Expected inspectPluginDso( const std::shared_ptr& handle, const std::filesystem::path& dso_path) { if (!hasDsoSuffix(dso_path)) { - return unexpected(fmt::format("not a platform plugin DSO: {}", dso_path.string())); + return unexpected(fmt::format("not a platform plugin DSO: {}", detail::pathForLegacyAccessor(dso_path))); } - auto with_path = [&](const std::string& error) { return fmt::format("{}: {}", dso_path.string(), error); }; + auto with_path = [&](const std::string& error) { + return fmt::format("{}: {}", detail::pathForLegacyAccessor(dso_path), error); + }; if (handle == nullptr) { return unexpected(with_path("library not loaded")); } @@ -487,10 +489,10 @@ Expected> exportedPluginFamilies( Expected scanPluginDsos(const std::filesystem::path& directory) { std::error_code ec; if (!std::filesystem::exists(directory, ec)) { - return unexpected(fmt::format("plugin directory not found: {}", directory.string())); + return unexpected(fmt::format("plugin directory not found: {}", detail::pathForLegacyAccessor(directory))); } if (!std::filesystem::is_directory(directory, ec)) { - return unexpected(fmt::format("plugin path is not a directory: {}", directory.string())); + return unexpected(fmt::format("plugin path is not a directory: {}", detail::pathForLegacyAccessor(directory))); } PluginScanResult result; diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index 3fcb8545..730db640 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -68,6 +68,41 @@ class PluginCatalogTest : public ::testing::Test { std::filesystem::path dir_; }; +// Unicode errors must be UTF-8 on every Windows ACP, not only UTF-8 systems. +TEST_F(PluginCatalogTest, UnicodeFailureDiagnosticsPreservePath) { + const auto directory = dir_ / std::filesystem::path(u8"\u8cc7\u6599-\U0001f680"); + const auto utf8 = directory.u8string(); + const std::string expected(utf8.begin(), utf8.end()); + auto missing = scanPluginDsos(directory); + ASSERT_FALSE(missing.has_value()); + EXPECT_NE(missing.error().find(expected), std::string::npos); + std::filesystem::create_directories(directory); + const auto broken = directory / pluginFileName("broken"); + { + std::ofstream output(broken); + ASSERT_TRUE(output.is_open()); + output << "not a shared library"; + ASSERT_TRUE(output.good()); + } + auto invalid = inspectPluginDso(broken); + ASSERT_FALSE(invalid.has_value()); + EXPECT_NE(invalid.error().find(expected), std::string::npos); + auto unopened = inspectPluginDso(std::shared_ptr{}, broken); + ASSERT_FALSE(unopened.has_value()); + EXPECT_NE(unopened.error().find(expected), std::string::npos); + auto unsupported = inspectPluginDso(directory / std::filesystem::path(u8"file.\U0001f680")); + ASSERT_FALSE(unsupported.has_value()); + EXPECT_NE(unsupported.error().find(expected), std::string::npos); + auto scan = scanPluginDsos(directory); + ASSERT_TRUE(scan.has_value()) << scan.error(); + EXPECT_TRUE(scan->plugins.empty()); + ASSERT_EQ(scan->diagnostics.size(), 1u); + EXPECT_NE(scan->diagnostics.front().message.find(expected), std::string::npos); + auto not_directory = scanPluginDsos(broken); + ASSERT_FALSE(not_directory.has_value()); + EXPECT_NE(not_directory.error().find(expected), std::string::npos); +} + TEST_F(PluginCatalogTest, MissingDirectoryReturnsError) { auto result = scanPluginDsos("/nonexistent/path/xyz"); EXPECT_FALSE(result.has_value()); From 5e37ad563b0b073d9a1d643e0568f593b8f8fd08 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 14 Nov 2026 08:01:56 +0100 Subject: [PATCH 2/4] test(plugin-catalog): skip the Unicode diagnostics test where the name cannot be stored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test created its "資料-🚀" directory with the throwing overload of create_directories, so on a filesystem that rejects supplementary-plane characters or a non-Unicode encoding it failed in setup although the catalog behaved correctly. It now uses the error_code overload and skips for an illegal byte sequence or invalid argument; any other I/O error still fails. Found by a Codex review of this pull request. Co-Authored-By: Claude Fable 5.2 --- pj_plugins/tests/plugin_catalog_test.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index 730db640..087770b5 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include "detail/library_loader.hpp" @@ -76,7 +77,15 @@ TEST_F(PluginCatalogTest, UnicodeFailureDiagnosticsPreservePath) { auto missing = scanPluginDsos(directory); ASSERT_FALSE(missing.has_value()); EXPECT_NE(missing.error().find(expected), std::string::npos); - std::filesystem::create_directories(directory); + // Some filesystems cannot store this name (no supplementary-plane characters, + // or a non-Unicode encoding). That is not a catalog failure: skip the cases + // that need the directory, but still fail on any other I/O error. + std::error_code created; + std::filesystem::create_directories(directory, created); + if (created == std::errc::illegal_byte_sequence || created == std::errc::invalid_argument) { + GTEST_SKIP() << "filesystem cannot store the Unicode test name: " << created.message(); + } + ASSERT_FALSE(created) << created.message(); const auto broken = directory / pluginFileName("broken"); { std::ofstream output(broken); From 3cc321d56af75ce0fceece4edd5a4a0049ddfc5c Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 14 Nov 2026 14:36:11 +0100 Subject: [PATCH 3/4] fix(plugin-catalog): the scan never throws on a file name or an unreadable directory Review follow-ups on the Unicode diagnostics fix, plus one scan defect found during that review. The suffix check converted each file's extension to a narrow string. It is now compared as a path, so no file name can make the scan throw there. scanPluginDsos threw "recursive_directory_iterator cannot pop" on the first subdirectory it could not read, on every platform: after a failed increment the iterator is already at the end. An unreadable directory is now found before the scan descends into it, reported as a diagnostic, and skipped. The Unicode test asserts the "not a platform plugin DSO" text and calls the handle overload with a non-plugin path, which nothing exercised. Co-Authored-By: Claude Fable 5.2 --- pj_plugins/src/plugin_catalog.cpp | 21 ++++++++-- pj_plugins/tests/plugin_catalog_test.cpp | 49 +++++++++++++++++++++++- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/pj_plugins/src/plugin_catalog.cpp b/pj_plugins/src/plugin_catalog.cpp index 7c18c9a1..55d27c2c 100644 --- a/pj_plugins/src/plugin_catalog.cpp +++ b/pj_plugins/src/plugin_catalog.cpp @@ -43,8 +43,10 @@ struct ManifestCandidate { std::string manifest_json; }; +// Compared as paths, so no file name is ever narrowed: a name that cannot be +// converted must not make the scan throw. bool hasDsoSuffix(const std::filesystem::path& path) { - return detail::pathForLegacyAccessor(path.extension()) == kDsoSuffix; + return path.extension() == std::filesystem::path(kDsoSuffix); } bool nativeSymbolPresent(void* handle, const char* symbol) noexcept { @@ -520,12 +522,25 @@ Expected scanPluginDsos(const std::filesystem::path& directory result.diagnostics.push_back({entry.path(), fmt::format("stat failed: {}", entry_ec.message())}); } + // increment() descends into a directory entry, and when that fails the + // iterator is left at the end: the scan is over. So a directory that + // cannot be opened is found here, reported, and not descended into. + std::error_code type_ec; + if (it.recursion_pending() && entry.is_directory(type_ec) && !entry.is_symlink(type_ec)) { + std::error_code open_ec; + const std::filesystem::directory_iterator probe(entry.path(), open_ec); + if (open_ec) { + result.diagnostics.push_back({entry.path(), fmt::format("directory iteration failed: {}", open_ec.message())}); + it.disable_recursion_pending(); + } + } + std::error_code inc_ec; it.increment(inc_ec); if (inc_ec) { + // The iterator is at the end now; pop() on it would throw. result.diagnostics.push_back({entry.path(), fmt::format("directory iteration failed: {}", inc_ec.message())}); - // Skip the unreadable subtree but continue with the rest of the scan. - it.pop(); + break; } } diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index 087770b5..e90db44f 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -26,6 +26,8 @@ #define WIN32_LEAN_AND_MEAN #endif #include +#else +#include #endif namespace PJ { @@ -99,9 +101,18 @@ TEST_F(PluginCatalogTest, UnicodeFailureDiagnosticsPreservePath) { auto unopened = inspectPluginDso(std::shared_ptr{}, broken); ASSERT_FALSE(unopened.has_value()); EXPECT_NE(unopened.error().find(expected), std::string::npos); - auto unsupported = inspectPluginDso(directory / std::filesystem::path(u8"file.\U0001f680")); + // A suffix made of a supplementary-plane character is refused as "not a + // plugin", by both overloads, before anything is loaded. + const auto not_a_plugin = directory / std::filesystem::path(u8"file.\U0001f680"); + auto unsupported = inspectPluginDso(not_a_plugin); ASSERT_FALSE(unsupported.has_value()); + EXPECT_NE(unsupported.error().find("not a platform plugin DSO"), std::string::npos) << unsupported.error(); EXPECT_NE(unsupported.error().find(expected), std::string::npos); + auto unsupported_handle = inspectPluginDso(std::shared_ptr{}, not_a_plugin); + ASSERT_FALSE(unsupported_handle.has_value()); + EXPECT_NE(unsupported_handle.error().find("not a platform plugin DSO"), std::string::npos) + << unsupported_handle.error(); + EXPECT_NE(unsupported_handle.error().find(expected), std::string::npos); auto scan = scanPluginDsos(directory); ASSERT_TRUE(scan.has_value()) << scan.error(); EXPECT_TRUE(scan->plugins.empty()); @@ -228,6 +239,42 @@ TEST_F(PluginCatalogTest, ForwardedEntryPointIsRejected) { } #endif +#if !defined(_WIN32) +// A subdirectory the scan cannot read (an extension folder owned by another +// user) is reported and skipped. It must not end the scan, and it must not +// throw: the scan's contract is to return diagnostics. +TEST_F(PluginCatalogTest, ScanReportsAnUnreadableSubdirectoryAndContinues) { + if (::geteuid() == 0) { + GTEST_SKIP() << "root can read every directory"; + } + const std::filesystem::path locked = dir_ / "a_locked"; + const std::filesystem::path later = dir_ / "z_later"; + std::filesystem::create_directories(locked); + std::filesystem::create_directories(later); + std::ofstream(later / pluginFileName("broken")) << "not a shared library"; + std::filesystem::permissions(locked, std::filesystem::perms::none); + struct Restore { + std::filesystem::path path; + ~Restore() { + std::error_code ec; // so TearDown can remove the tree whatever the test did + std::filesystem::permissions(path, std::filesystem::perms::owner_all, ec); + } + } restore{locked}; + + auto scan = scanPluginDsos(dir_); + ASSERT_TRUE(scan.has_value()) << scan.error(); + EXPECT_TRUE(scan->plugins.empty()); + bool reported_locked = false; + bool reached_later = false; + for (const auto& diagnostic : scan->diagnostics) { + reported_locked = reported_locked || diagnostic.path == locked; + reached_later = reached_later || diagnostic.path == later / pluginFileName("broken"); + } + EXPECT_TRUE(reported_locked) << "the unreadable directory is named in a diagnostic"; + EXPECT_TRUE(reached_later) << "the scan went on to the entries after it"; +} +#endif + TEST_F(PluginCatalogTest, UnicodeExtensionPathLoadsOnWindows) { const std::filesystem::path unicode_dir = dir_ / std::filesystem::path(u8"插件-π"); std::filesystem::create_directories(unicode_dir); From 3a45b6702dd16a82188d240c7c670253efc24ead Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 14 Nov 2026 14:36:12 +0100 Subject: [PATCH 4/4] fix(source): cache lock and flush errors no longer throw on Windows Unicode roots file_lock.cpp and fs_durability.cpp built their error text with path.string(), which on Windows narrows through the active code page and throws for a name it cannot represent. "Lock is held elsewhere" is an ordinary outcome during cache cleanup and eviction, so a cache root with such a name threw there instead of reporting contention. A private pathUtf8() helper replaces those conversions and the two inline copies of the same idiom in request_cache.cpp. POSIX output is unchanged. The changelog entry now covers the catalog and the cache fixes together and says that these messages are UTF-8 on Windows. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 18 +++++++++---- pj_base/src/source/file_lock.cpp | 12 +++++---- pj_base/src/source/fs_durability.cpp | 10 ++++--- pj_base/src/source/path_utf8.hpp | 27 +++++++++++++++++++ pj_base/src/source/request_cache.cpp | 8 +++--- .../descriptor_import_request_cache_test.cpp | 17 ++++++++++++ 6 files changed, 74 insertions(+), 18 deletions(-) create mode 100644 pj_base/src/source/path_utf8.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 671023dd..0e8851a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,11 +101,19 @@ SceneEntities and PlotMarkers: - `PlotMarkers`: the decoder drops a marker whose `kind` it does not know instead of drawing it as a time region; wire numbers unchanged. -Plugin catalog: - -- Preserve Unicode paths in plugin catalog failure diagnostics on Windows without - converting through the active code page. Invalid plugins and directories now - return their intended errors instead of throwing during path formatting. +Plugin catalog and source cache, errors that threw instead of being returned: + +- On Windows, a plugin path that the active code page cannot represent no longer makes + `inspectPluginDso` or `scanPluginDsos` throw. They formatted their own error text with + `path.string()`, and the scan checked each file's suffix the same way, so one such file anywhere + under the plugin folder made the whole scan throw. Paths in these messages are now UTF-8 on + Windows whatever the code page; a consumer that decoded them as the local code page must decode + UTF-8. +- `scanPluginDsos` reports a subdirectory it cannot read and goes on with the rest. It threw + (`recursive_directory_iterator cannot pop`) on the first unreadable directory, on every platform. +- The source request cache has the same Windows fix for its lock and flush errors. "Lock is held + elsewhere", an ordinary outcome during cleanup and eviction, threw under a cache root the code + page cannot represent. Removed: diff --git a/pj_base/src/source/file_lock.cpp b/pj_base/src/source/file_lock.cpp index 571035bc..5e4772cb 100644 --- a/pj_base/src/source/file_lock.cpp +++ b/pj_base/src/source/file_lock.cpp @@ -6,6 +6,8 @@ #include #include +#include "source/path_utf8.hpp" + #if defined(_WIN32) // windows.h min/max macros break std::max / numeric_limits<>::max. #ifndef NOMINMAX @@ -46,7 +48,7 @@ std::optional tryAcquireHandle( FILE_ATTRIBUTE_NORMAL, nullptr); if (handle == INVALID_HANDLE_VALUE) { if (error != nullptr) { - *error = "could not open lock file " + path.string() + " (error " + std::to_string(::GetLastError()) + ")"; + *error = "could not open lock file " + pathUtf8(path) + " (error " + std::to_string(::GetLastError()) + ")"; } return std::nullopt; } @@ -58,7 +60,7 @@ std::optional tryAcquireHandle( *contended = true; // FAIL_IMMEDIATELY: the region is held elsewhere } if (error != nullptr) { - *error = "lock is held elsewhere: " + path.string(); + *error = "lock is held elsewhere: " + pathUtf8(path); } return std::nullopt; } @@ -67,7 +69,7 @@ std::optional tryAcquireHandle( const int fd = ::open(path.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0600); if (fd < 0) { if (error != nullptr) { - *error = "could not open lock file " + path.string() + ": " + + *error = "could not open lock file " + pathUtf8(path) + ": " + std::error_code(errno, std::generic_category()).message(); } return std::nullopt; @@ -80,8 +82,8 @@ std::optional tryAcquireHandle( *contended = held_elsewhere; } if (error != nullptr) { - *error = held_elsewhere ? "lock is held elsewhere: " + path.string() - : "flock failed on " + path.string() + ": " + + *error = held_elsewhere ? "lock is held elsewhere: " + pathUtf8(path) + : "flock failed on " + pathUtf8(path) + ": " + std::error_code(flock_errno, std::generic_category()).message(); } return std::nullopt; diff --git a/pj_base/src/source/fs_durability.cpp b/pj_base/src/source/fs_durability.cpp index efe307d4..393f61f5 100644 --- a/pj_base/src/source/fs_durability.cpp +++ b/pj_base/src/source/fs_durability.cpp @@ -6,6 +6,8 @@ #include #include +#include "source/path_utf8.hpp" + #if defined(_WIN32) // windows.h min/max macros break std::max / numeric_limits<>::max. #ifndef NOMINMAX @@ -44,21 +46,21 @@ bool syncFile(const fs::path& file, std::string* error) { file.c_str(), GENERIC_WRITE, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); if (handle == INVALID_HANDLE_VALUE) { if (error != nullptr) { - *error = "could not reopen for flush: " + file.string(); + *error = "could not reopen for flush: " + pathUtf8(file); } return false; } const bool ok = ::FlushFileBuffers(handle) != 0; ::CloseHandle(handle); if (!ok && error != nullptr) { - *error = "FlushFileBuffers failed: " + file.string(); + *error = "FlushFileBuffers failed: " + pathUtf8(file); } return ok; #else const int fd = ::open(file.c_str(), O_RDONLY | O_CLOEXEC); if (fd < 0) { if (error != nullptr) { - *error = "could not reopen for fsync: " + file.string() + ": " + + *error = "could not reopen for fsync: " + pathUtf8(file) + ": " + std::error_code(errno, std::generic_category()).message(); } return false; @@ -67,7 +69,7 @@ bool syncFile(const fs::path& file, std::string* error) { const int fsync_errno = errno; ::close(fd); if (!ok && error != nullptr) { - *error = "fsync failed: " + file.string() + ": " + std::error_code(fsync_errno, std::generic_category()).message(); + *error = "fsync failed: " + pathUtf8(file) + ": " + std::error_code(fsync_errno, std::generic_category()).message(); } return ok; #endif diff --git a/pj_base/src/source/path_utf8.hpp b/pj_base/src/source/path_utf8.hpp new file mode 100644 index 00000000..b1507323 --- /dev/null +++ b/pj_base/src/source/path_utf8.hpp @@ -0,0 +1,27 @@ +#pragma once +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +namespace PJ { +namespace sdk { +namespace source { +namespace detail { + +/// The path as UTF-8, for error text and name checks. +/// +/// Use this instead of `path.string()`: on Windows that narrows through the +/// active code page and throws `std::system_error` for a name the code page +/// cannot represent, so a function meant to return an error throws instead. +/// On POSIX the result is the same bytes `string()` returns. +[[nodiscard]] inline std::string pathUtf8(const std::filesystem::path& path) { + const std::u8string utf8 = path.u8string(); + return {utf8.begin(), utf8.end()}; +} + +} // namespace detail +} // namespace source +} // namespace sdk +} // namespace PJ diff --git a/pj_base/src/source/request_cache.cpp b/pj_base/src/source/request_cache.cpp index 949954ae..f3ac27d7 100644 --- a/pj_base/src/source/request_cache.cpp +++ b/pj_base/src/source/request_cache.cpp @@ -11,6 +11,7 @@ #include "pj_base/sdk/testing/request_cache_probe.hpp" #include "source/file_lock.hpp" #include "source/fs_durability.hpp" +#include "source/path_utf8.hpp" #if defined(_WIN32) // windows.h min/max macros break std::max / numeric_limits<>::max. @@ -85,10 +86,9 @@ class CacheLayout { /// Managed names only: "" and ".partial."; /// sidecars, foreign files and malformed digests are kOther. [[nodiscard]] Classified classify(const fs::path& file) const { - // u8string(): a foreign Unicode name classifies as kOther instead of + // pathUtf8(): a foreign Unicode name classifies as kOther instead of // throwing through .string() on a non-ACP Windows code page. - const std::u8string u8name = file.filename().u8string(); - const std::string name(u8name.begin(), u8name.end()); + const std::string name = detail::pathUtf8(file.filename()); const std::size_t hex = spec_.identity.hexChars(); const auto digestOk = [&name, hex] { return name.size() >= hex && std::all_of( @@ -238,7 +238,7 @@ Expected RequestArtifactCache::WriteTrans impl.lock.reset(); return unexpected(CacheError{std::move(reason)}); }; - const std::string what = "cache commit rejected " + impl.partial.filename().string() + ": "; + const std::string what = "cache commit rejected " + detail::pathUtf8(impl.partial.filename()) + ": "; if (!impl.validator) { return fail(what + "no artifact validator configured"); } diff --git a/pj_base/tests/descriptor_import_request_cache_test.cpp b/pj_base/tests/descriptor_import_request_cache_test.cpp index 25cc65e8..b8d0b24a 100644 --- a/pj_base/tests/descriptor_import_request_cache_test.cpp +++ b/pj_base/tests/descriptor_import_request_cache_test.cpp @@ -434,6 +434,23 @@ TEST_F(RequestCacheTest, UnicodeCacheRootRoundTripsAndCleans) { EXPECT_FALSE(fs::exists(ucache.pathFor(identityFor(kHexA)))); } +TEST_F(RequestCacheTest, UnicodeCacheRootReportsContentionInsteadOfThrowing) { + // "Lock is held elsewhere" is an ordinary outcome, and its message names the + // lock file. On Windows that name went through path.string(), which throws + // for a root the active code page cannot represent. + const fs::path uroot = root() / fs::path(u8"пж-缓存-ü"); + fs::create_directories(uroot); + RequestArtifactCache ucache(CacheSpec{uroot, kSuffix, IdentityScheme{kPrefix, 32}}, acceptAll()); + auto first = ucache.beginWrite(identityFor(kHexA)); + ASSERT_TRUE(first) << first.error().message; + auto second = ucache.beginWrite(identityFor(kHexA)); + ASSERT_FALSE(second); + EXPECT_TRUE(second.error().retryable); + const std::u8string u8root = uroot.filename().u8string(); + EXPECT_NE(second.error().message.find(std::string(u8root.begin(), u8root.end())), std::string::npos) + << second.error().message; +} + TEST_F(RequestCacheTest, OutOfRangeDigestWidthStillRoundTrips) { // The spec's scheme normalizes the width exactly like identity minting, so // a provider that misconfigures digest_hex_chars still gets a coherent