From 0e88008bcb7751af3cd444680a27a883ba6f2fbe Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Tue, 11 Aug 2026 08:02:29 +0200 Subject: [PATCH 1/5] fix(pj_plugins): entry-point symbol provenance + modern platform loaders (0.23.0) Resolve pj_plugin_abi_version and the PJ_get_*_vtable entry points strictly from the candidate DSO: dladdr + filesystem::equivalent identity on Linux, RTLD_FIRST on macOS, module-scoped GetProcAddress unchanged on Windows. Replace LoadLibraryExA + LOAD_WITH_ALTERED_SEARCH_PATH with LoadLibraryExW + DLL_LOAD_DIR|DEFAULT_DIRS in both loaders (CWD and PATH no longer resolve dependencies), make loader signatures path-typed with internal absolute-path normalization, and delete the ACP/UTF-8 contract mismatch. Add the single-open adoption API (non-owning adopt, loadFromHandle on all four families, handle-based inspection, provenance- checked family enumeration) for the admission helper. Standardize plugin install rpaths ($ORIGIN / @loader_path). Tests: two-DSO provenance fixtures in both directions, Unicode paths, CWD/PATH decoys. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0165qLfnJYSYb51NQMdPidZr --- CHANGELOG.md | 19 +++ VERSION | 2 +- cmake/PjPluginManifest.cmake | 10 ++ pj_plugins/CMakeLists.txt | 64 ++++++++- .../pj_plugins/host/dialog_library.hpp | 20 +++ .../dialog_protocol/src/dialog_library.cpp | 22 +++- pj_plugins/docs/ARCHITECTURE.md | 7 +- .../pj_plugins/host/data_source_library.hpp | 20 +++ .../host/message_parser_library.hpp | 20 +++ .../pj_plugins/host/plugin_catalog.hpp | 16 +++ .../pj_plugins/host/toolbox_library.hpp | 20 +++ pj_plugins/src/data_source_library.cpp | 28 +++- pj_plugins/src/detail/library_loader.hpp | 122 +++++++++++++++--- .../detail/native_parser_module_loader.hpp | 22 +--- pj_plugins/src/message_parser_library.cpp | 28 +++- pj_plugins/src/native_parser_module.cpp | 3 +- pj_plugins/src/plugin_catalog.cpp | 87 ++++++++----- pj_plugins/src/toolbox_library.cpp | 28 +++- .../tests/dependency_search_candidate.cpp | 85 ++++++++++++ .../tests/dependency_search_dependency.cpp | 16 +++ pj_plugins/tests/entry_point_donor.cpp | 81 ++++++++++++ .../tests/entry_point_via_dependency.cpp | 20 +++ .../tests/entry_point_with_own_exports.cpp | 87 +++++++++++++ pj_plugins/tests/plugin_catalog_test.cpp | 119 +++++++++++++++++ 24 files changed, 859 insertions(+), 87 deletions(-) create mode 100644 pj_plugins/tests/dependency_search_candidate.cpp create mode 100644 pj_plugins/tests/dependency_search_dependency.cpp create mode 100644 pj_plugins/tests/entry_point_donor.cpp create mode 100644 pj_plugins/tests/entry_point_via_dependency.cpp create mode 100644 pj_plugins/tests/entry_point_with_own_exports.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index ee15478d..80bf057b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,25 @@ All notable changes to `plotjuggler_sdk` are recorded here. Versioning policy is in [`CLAUDE.md`](./CLAUDE.md) → "Release Versioning". +## [0.23.0] + +### Fix: entry-point symbol provenance and modern platform loaders (MINOR) + +Plugin admission now proves that the ABI marker and family vtable getter are +defined by the candidate DSO itself instead of accepting definitions from a +dependency. POSIX uses defining-object identity, macOS restricts handle-scoped +lookups to the first image, and Windows uses filesystem-native wide paths with +package-scoped dependency search. Plugin install rpaths now resolve bundled +dependencies relative to the plugin on Linux and macOS. + +New filesystem-path overloads and already-open-handle adoption APIs let hosts +validate, inspect, and instantiate a candidate through one native module open. +Static initializers now run once per admission instead of up to three times. + +There is no C-ABI or protocol change: `PJ_ABI_VERSION`, all family protocol +versions, vtable layouts, and `abi/baseline.abi` remain unchanged. The release +is MINOR because the installed C++ host API gains additive overloads. + ## [0.22.0] ### Feature: extensible parser routing and functional parser modules (MINOR) diff --git a/VERSION b/VERSION index 21574090..ca222b7c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.22.0 +0.23.0 diff --git a/cmake/PjPluginManifest.cmake b/cmake/PjPluginManifest.cmake index 34670c57..0ffcd5ca 100644 --- a/cmake/PjPluginManifest.cmake +++ b/cmake/PjPluginManifest.cmake @@ -61,6 +61,16 @@ function(pj_emit_plugin_manifest TARGET) C_VISIBILITY_PRESET hidden VISIBILITY_INLINES_HIDDEN ON ) + if(APPLE) + set_target_properties(${TARGET} PROPERTIES + INSTALL_RPATH "@loader_path" + MACOSX_RPATH ON + ) + elseif(UNIX) + set_target_properties(${TARGET} PROPERTIES + INSTALL_RPATH "$ORIGIN" + ) + endif() target_link_options(${TARGET} PRIVATE $<$:-Wl,-Bsymbolic-functions> ) diff --git a/pj_plugins/CMakeLists.txt b/pj_plugins/CMakeLists.txt index 2144d4c7..2083b9d9 100644 --- a/pj_plugins/CMakeLists.txt +++ b/pj_plugins/CMakeLists.txt @@ -343,6 +343,58 @@ target_compile_features(legacy_macro_dialog_plugin PRIVATE cxx_std_20) target_compile_options(legacy_macro_dialog_plugin PRIVATE ${PJ_WARNING_FLAGS}) target_link_libraries(legacy_macro_dialog_plugin PRIVATE pj_dialog_protocol) +# First two-DSO loader fixtures: one malformed candidate whose entry points +# come only from a dependency, and one well-formed candidate that defines its +# own entry points while retaining the same dependency. +add_library(entry_point_donor SHARED tests/entry_point_donor.cpp) +target_compile_features(entry_point_donor PRIVATE cxx_std_20) +target_compile_options(entry_point_donor PRIVATE ${PJ_WARNING_FLAGS}) +target_link_libraries(entry_point_donor PRIVATE pj_base) + +add_library(entry_point_via_dependency_plugin SHARED tests/entry_point_via_dependency.cpp) +target_compile_features(entry_point_via_dependency_plugin PRIVATE cxx_std_20) +target_compile_options(entry_point_via_dependency_plugin PRIVATE ${PJ_WARNING_FLAGS}) +target_link_libraries(entry_point_via_dependency_plugin PRIVATE entry_point_donor pj_base) + +add_library(entry_point_with_own_exports_plugin SHARED tests/entry_point_with_own_exports.cpp) +target_compile_features(entry_point_with_own_exports_plugin PRIVATE cxx_std_20) +target_compile_options(entry_point_with_own_exports_plugin PRIVATE ${PJ_WARNING_FLAGS}) +target_link_libraries(entry_point_with_own_exports_plugin PRIVATE entry_point_donor pj_base) + +# Dependency-search fixtures. The real and decoy dependencies intentionally +# share one filename but live in separate build directories. +add_library(dependency_search_real SHARED tests/dependency_search_dependency.cpp) +target_compile_features(dependency_search_real PRIVATE cxx_std_20) +target_compile_options(dependency_search_real PRIVATE ${PJ_WARNING_FLAGS}) +set_target_properties(dependency_search_real PROPERTIES OUTPUT_NAME pj_dependency_search_fixture) + +add_library(dependency_search_decoy SHARED tests/dependency_search_dependency.cpp) +target_compile_features(dependency_search_decoy PRIVATE cxx_std_20) +target_compile_options(dependency_search_decoy PRIVATE ${PJ_WARNING_FLAGS}) +target_compile_definitions(dependency_search_decoy PRIVATE PJ_DEPENDENCY_SEARCH_DECOY) +set_target_properties(dependency_search_decoy PROPERTIES + OUTPUT_NAME pj_dependency_search_fixture + ARCHIVE_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/dependency_search_decoy" + LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/dependency_search_decoy" + RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/dependency_search_decoy" +) + +add_library(dependency_search_candidate_plugin SHARED tests/dependency_search_candidate.cpp) +target_compile_features(dependency_search_candidate_plugin PRIVATE cxx_std_20) +target_compile_options(dependency_search_candidate_plugin PRIVATE ${PJ_WARNING_FLAGS}) +target_link_libraries(dependency_search_candidate_plugin PRIVATE dependency_search_real pj_base) +if(APPLE) + # Keep this copied fixture package-relative without CMake appending the + # absolute build directory, which would invalidate the decoy-only case. + set_target_properties(dependency_search_candidate_plugin PROPERTIES SKIP_BUILD_RPATH TRUE) + target_link_options(dependency_search_candidate_plugin PRIVATE "LINKER:-rpath,@loader_path") +elseif(UNIX) + set_target_properties(dependency_search_candidate_plugin PROPERTIES + BUILD_RPATH "$ORIGIN" + BUILD_RPATH_USE_ORIGIN TRUE + ) +endif() + # --------------------------------------------------------------------------- # Tests # --------------------------------------------------------------------------- @@ -565,17 +617,25 @@ target_compile_definitions(plugin_catalog_test PRIVATE PJ_MISSING_ID_PLUGIN_PATH="$" PJ_INVALID_OPTIONAL_PLUGIN_PATH="$" PJ_MISSING_REQUIRED_SLOTS_PLUGIN_PATH="$" + PJ_ENTRY_POINT_VIA_DEPENDENCY_PLUGIN_PATH="$" + PJ_ENTRY_POINT_WITH_OWN_EXPORTS_PLUGIN_PATH="$" + PJ_DEPENDENCY_SEARCH_CANDIDATE_PATH="$" + PJ_DEPENDENCY_SEARCH_REAL_PATH="$" + PJ_DEPENDENCY_SEARCH_DECOY_PATH="$" ) target_compile_options(plugin_catalog_test PRIVATE ${PJ_WARNING_FLAGS}) +target_include_directories(plugin_catalog_test PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src) target_link_libraries(plugin_catalog_test PRIVATE - pj_plugin_catalog GTest::gtest_main + pj_plugin_catalog pj_data_source_host GTest::gtest_main ) add_dependencies(plugin_catalog_test mock_data_source_plugin mock_json_parser_plugin mock_toolbox_plugin mock_dialog_plugin missing_id_data_source_plugin invalid_optional_manifest_data_source_plugin missing_required_slots_plugin static_manifest_dialog_plugin legacy_macro_dialog_plugin - old_dialog_vtable_plugin missing_dialog_required_slots_plugin) + old_dialog_vtable_plugin missing_dialog_required_slots_plugin + entry_point_via_dependency_plugin entry_point_with_own_exports_plugin + dependency_search_candidate_plugin dependency_search_decoy) add_test(NAME plugin_catalog_test COMMAND plugin_catalog_test) endif() # PJ_BUILD_TESTS diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/host/dialog_library.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/host/dialog_library.hpp index 781c8832..a265ccb4 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/host/dialog_library.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/host/dialog_library.hpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -32,6 +33,25 @@ class DialogLibrary { /// Load a dialog plugin from @p path. Returns an error string on failure. [[nodiscard]] static Expected load(std::string_view path); + /// Preserve an unambiguous load call for existing narrow string paths. + [[nodiscard]] static Expected load(const char* path) { + return load(std::string_view(path)); + } + + /// Preserve an unambiguous load call for existing `std::string` paths. + [[nodiscard]] static Expected load(const std::string& path) { + return load(std::string_view(path)); + } + + /// Load a dialog plugin from a filesystem-native @p path. + [[nodiscard]] static Expected load(const std::filesystem::path& path); + + /// Validate and retain an already-open @p handle whose file is @p origin. + /// The library shares the caller-supplied handle ownership and does not open + /// or close a separate native module during validation. + [[nodiscard]] static Expected loadFromHandle( + std::shared_ptr handle, const std::filesystem::path& origin); + /// True if the library was loaded and the vtable resolved successfully. [[nodiscard]] bool valid() const { return handle_ != nullptr && vtable_ != nullptr; diff --git a/pj_plugins/dialog_protocol/src/dialog_library.cpp b/pj_plugins/dialog_protocol/src/dialog_library.cpp index 24987a37..f6414d06 100644 --- a/pj_plugins/dialog_protocol/src/dialog_library.cpp +++ b/pj_plugins/dialog_protocol/src/dialog_library.cpp @@ -34,17 +34,31 @@ DialogLibrary& DialogLibrary::operator=(DialogLibrary&& other) noexcept { } Expected DialogLibrary::load(std::string_view path) { + auto library = load(std::filesystem::path(path)); + if (library) { + library->path_ = std::string(path); + } + return library; +} + +Expected DialogLibrary::load(const std::filesystem::path& path) { auto raw_handle = detail::loadLibraryHandle(path); if (!raw_handle) { return unexpected(raw_handle.error()); } - auto handle = detail::adoptLibraryHandle(*raw_handle); + return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), path); +} - if (auto abi = detail::checkPluginAbiVersion(handle.get()); !abi) { +Expected DialogLibrary::loadFromHandle( + std::shared_ptr handle, const std::filesystem::path& origin) { + if (handle == nullptr) { + return unexpected("library not loaded"); + } + if (auto abi = detail::checkPluginAbiVersion(handle.get(), origin); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_dialog_vtable"); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_dialog_vtable", origin); if (!sym) { return unexpected(sym.error()); } @@ -64,7 +78,7 @@ Expected DialogLibrary::load(std::string_view path) { return unexpected(status.error()); } - return DialogLibrary(std::move(handle), vtable, std::string(path)); + return DialogLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(origin)); } void DialogLibrary::reset() { diff --git a/pj_plugins/docs/ARCHITECTURE.md b/pj_plugins/docs/ARCHITECTURE.md index 053bdb1c..303696c6 100644 --- a/pj_plugins/docs/ARCHITECTURE.md +++ b/pj_plugins/docs/ARCHITECTURE.md @@ -215,6 +215,10 @@ previously-circulated pre-v4 design included): - **No more RTLD_DEEPBIND.** The loader uses `RTLD_NOW | RTLD_LOCAL` only (DEEPBIND was a documented ASAN/allocator-interposition trap). Plugin-local symbol isolation is left to `-fvisibility=hidden`. +- **Declined loader alternatives.** Admission does not use + `RTLD_NODELETE` as its lifetime contract, `RTLD_DEEPBIND`, `dlmopen`, or a + manifest-format change. Shared handle ownership controls lifetime, while + candidate-file provenance is checked directly at each boot-level symbol. Structural shape inherited from the pre-v4 design work (carries the service registry, error out-params, and typed borrowed-dialog patterns): @@ -449,7 +453,8 @@ These live in `sdk/detail/*_trampolines.hpp`. Each family has a loader that: 1. Calls `dlopen` (or `LoadLibrary` on Windows) on the `.so` path. -2. Calls `dlsym` for the entry point symbol. +2. Resolves the ABI marker and entry point, then verifies that each symbol's + defining object is the candidate DSO itself rather than a dependency. 3. Validates `protocol_version` and `struct_size`. 4. Stores the vtable pointer for creating handles. diff --git a/pj_plugins/include/pj_plugins/host/data_source_library.hpp b/pj_plugins/include/pj_plugins/host/data_source_library.hpp index 4b55ff0a..88d559b0 100644 --- a/pj_plugins/include/pj_plugins/host/data_source_library.hpp +++ b/pj_plugins/include/pj_plugins/host/data_source_library.hpp @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -51,6 +52,25 @@ class DataSourceLibrary { /// Load a plugin from @p path. Returns an error string on failure. [[nodiscard]] static Expected load(std::string_view path); + /// Preserve an unambiguous load call for existing narrow string paths. + [[nodiscard]] static Expected load(const char* path) { + return load(std::string_view(path)); + } + + /// Preserve an unambiguous load call for existing `std::string` paths. + [[nodiscard]] static Expected load(const std::string& path) { + return load(std::string_view(path)); + } + + /// Load a plugin from a filesystem-native @p path. + [[nodiscard]] static Expected load(const std::filesystem::path& path); + + /// Validate and retain an already-open @p handle whose file is @p origin. + /// The library shares the caller-supplied handle ownership and does not open + /// or close a separate native module during validation. + [[nodiscard]] static Expected loadFromHandle( + std::shared_ptr handle, const std::filesystem::path& origin); + /// Wrap a statically-linked plugin vtable (no dlopen; for WASM/static builds). /// @p vtable must have static storage duration (valid for the program lifetime). [[nodiscard]] static Expected loadStatic( diff --git a/pj_plugins/include/pj_plugins/host/message_parser_library.hpp b/pj_plugins/include/pj_plugins/host/message_parser_library.hpp index 35300f87..1398511f 100644 --- a/pj_plugins/include/pj_plugins/host/message_parser_library.hpp +++ b/pj_plugins/include/pj_plugins/host/message_parser_library.hpp @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -51,6 +52,25 @@ class MessageParserLibrary { /// Load a plugin from @p path. Returns an error string on failure. [[nodiscard]] static Expected load(std::string_view path); + /// Preserve an unambiguous load call for existing narrow string paths. + [[nodiscard]] static Expected load(const char* path) { + return load(std::string_view(path)); + } + + /// Preserve an unambiguous load call for existing `std::string` paths. + [[nodiscard]] static Expected load(const std::string& path) { + return load(std::string_view(path)); + } + + /// Load a plugin from a filesystem-native @p path. + [[nodiscard]] static Expected load(const std::filesystem::path& path); + + /// Validate and retain an already-open @p handle whose file is @p origin. + /// The library shares the caller-supplied handle ownership and does not open + /// or close a separate native module during validation. + [[nodiscard]] static Expected loadFromHandle( + std::shared_ptr handle, const std::filesystem::path& origin); + /// Wrap a statically-linked plugin vtable (no dlopen; for WASM/static builds). /// @p vtable must have static storage duration (valid for the program lifetime). [[nodiscard]] static Expected loadStatic( diff --git a/pj_plugins/include/pj_plugins/host/plugin_catalog.hpp b/pj_plugins/include/pj_plugins/host/plugin_catalog.hpp index 903e37a3..098a8735 100644 --- a/pj_plugins/include/pj_plugins/host/plugin_catalog.hpp +++ b/pj_plugins/include/pj_plugins/host/plugin_catalog.hpp @@ -16,7 +16,9 @@ #include #include +#include #include +#include #include #include "pj_base/expected.hpp" @@ -81,6 +83,11 @@ struct PluginScanResult { /// Inspect one DSO and return its embedded plugin descriptor. [[nodiscard]] Expected inspectPluginDso(const std::filesystem::path& dso_path); +/// Inspect one DSO through an already-open @p handle originating at @p dso_path. +/// The supplied shared handle remains owned by the caller and is not reopened. +[[nodiscard]] Expected inspectPluginDso( + const std::shared_ptr& handle, const std::filesystem::path& dso_path); + /// Recursively scan a directory for platform plugin DSOs. Invalid candidates are /// reported in diagnostics while discovery continues. [[nodiscard]] Expected scanPluginDsos(const std::filesystem::path& directory); @@ -88,4 +95,13 @@ struct PluginScanResult { /// Human-readable name for a plugin family. [[nodiscard]] std::string_view toString(PluginFamily family) noexcept; +namespace detail { + +/// Return the plugin families whose getter symbols are defined by @p dso_path +/// itself. Missing getters and getters supplied only by dependencies are omitted. +[[nodiscard]] std::vector exportedPluginFamilies( + const std::shared_ptr& handle, const std::filesystem::path& dso_path); + +} // namespace detail + } // namespace PJ diff --git a/pj_plugins/include/pj_plugins/host/toolbox_library.hpp b/pj_plugins/include/pj_plugins/host/toolbox_library.hpp index c0e00ea6..4c2d494e 100644 --- a/pj_plugins/include/pj_plugins/host/toolbox_library.hpp +++ b/pj_plugins/include/pj_plugins/host/toolbox_library.hpp @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -51,6 +52,25 @@ class ToolboxLibrary { /// Load a plugin from @p path. Returns an error string on failure. [[nodiscard]] static Expected load(std::string_view path); + /// Preserve an unambiguous load call for existing narrow string paths. + [[nodiscard]] static Expected load(const char* path) { + return load(std::string_view(path)); + } + + /// Preserve an unambiguous load call for existing `std::string` paths. + [[nodiscard]] static Expected load(const std::string& path) { + return load(std::string_view(path)); + } + + /// Load a plugin from a filesystem-native @p path. + [[nodiscard]] static Expected load(const std::filesystem::path& path); + + /// Validate and retain an already-open @p handle whose file is @p origin. + /// The library shares the caller-supplied handle ownership and does not open + /// or close a separate native module during validation. + [[nodiscard]] static Expected loadFromHandle( + std::shared_ptr handle, const std::filesystem::path& origin); + /// Wrap a statically-linked plugin vtable (no dlopen; for WASM/static builds). /// @p vtable must have static storage duration (valid for the program lifetime). [[nodiscard]] static Expected loadStatic( diff --git a/pj_plugins/src/data_source_library.cpp b/pj_plugins/src/data_source_library.cpp index 87ef1908..9d6aa668 100644 --- a/pj_plugins/src/data_source_library.cpp +++ b/pj_plugins/src/data_source_library.cpp @@ -45,17 +45,31 @@ DataSourceLibrary& DataSourceLibrary::operator=(DataSourceLibrary&& other) noexc } Expected DataSourceLibrary::load(std::string_view path) { + auto library = load(std::filesystem::path(path)); + if (library) { + library->path_ = std::string(path); + } + return library; +} + +Expected DataSourceLibrary::load(const std::filesystem::path& path) { auto raw_handle = detail::loadLibraryHandle(path); if (!raw_handle) { return unexpected(raw_handle.error()); } - auto handle = detail::adoptLibraryHandle(*raw_handle); + return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), path); +} - if (auto abi = detail::checkPluginAbiVersion(handle.get()); !abi) { +Expected DataSourceLibrary::loadFromHandle( + std::shared_ptr handle, const std::filesystem::path& origin) { + if (handle == nullptr) { + return unexpected("library not loaded"); + } + if (auto abi = detail::checkPluginAbiVersion(handle.get(), origin); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_data_source_vtable"); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_data_source_vtable", origin); if (!sym) { return unexpected(sym.error()); } @@ -77,7 +91,7 @@ Expected DataSourceLibrary::load(std::string_view path) { return unexpected(status.error()); } - return DataSourceLibrary(std::move(handle), vtable, std::string(path)); + return DataSourceLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(origin)); } Expected DataSourceLibrary::loadStatic( @@ -119,7 +133,11 @@ Expected DataSourceLibrary::resolveDialogVtable() con if (path_ == "static://") { return unexpected("static DataSource has no registered dialog vtable"); } - auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable"); +#if defined(_WIN32) + auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", {}); +#else + auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", std::filesystem::path(path_)); +#endif if (!sym) { return unexpected(sym.error()); } diff --git a/pj_plugins/src/detail/library_loader.hpp b/pj_plugins/src/detail/library_loader.hpp index 8b2626a4..8142d2ff 100644 --- a/pj_plugins/src/detail/library_loader.hpp +++ b/pj_plugins/src/detail/library_loader.hpp @@ -3,11 +3,18 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include #include -#include +#include #if defined(_WIN32) +#ifndef NOMINMAX +#define NOMINMAX +#endif +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif #include #else #include @@ -18,15 +25,31 @@ namespace PJ::detail { -inline Expected loadLibraryHandle(std::string_view path) { +/// Encode a native path for the legacy narrow `Library::path()` accessor. +inline std::string pathForLegacyAccessor(const std::filesystem::path& path) { +#if defined(_WIN32) + const auto utf8 = path.u8string(); + return std::string(utf8.begin(), utf8.end()); +#else + return path.string(); +#endif +} + +inline Expected loadLibraryHandle(const std::filesystem::path& path) { + std::error_code path_error; + const std::filesystem::path absolute_path = std::filesystem::absolute(path, path_error); + if (path_error) { #if defined(_WIN32) - // LOAD_WITH_ALTERED_SEARCH_PATH adds the directory of the loaded DLL to the - // search path for resolving its dependencies — matches dlopen's default on - // Linux. Without it, deps are only searched in the .exe directory, System32 - // and PATH, so plugins cannot ship their own sibling DLLs - HMODULE module = LoadLibraryExA(std::string(path).c_str(), nullptr, LOAD_WITH_ALTERED_SEARCH_PATH); + return unexpected("cannot make library path absolute: " + path_error.message()); +#else + return unexpected("cannot make library path absolute '" + path.string() + "': " + path_error.message()); +#endif + } +#if defined(_WIN32) + HMODULE module = LoadLibraryExW( + absolute_path.c_str(), nullptr, LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); if (module == nullptr) { - return unexpected("LoadLibraryExA failed (error " + std::to_string(GetLastError()) + ")"); + return unexpected("LoadLibraryExW failed (error " + std::to_string(GetLastError()) + ")"); } return reinterpret_cast(module); #else @@ -51,7 +74,13 @@ inline Expected loadLibraryHandle(std::string_view path) { // malloc/pthread/system calls are NOT defined in the plugin so they still // reach the host — ASAN malloc interposition works correctly. int flags = RTLD_NOW | RTLD_LOCAL; - void* handle = dlopen(std::string(path).c_str(), flags); +#if defined(__APPLE__) + // Restrict handle-scoped lookups to the candidate image. Dylibs that rely on + // -reexport_library no longer resolve through this handle; that stricter + // admission behavior is intentional and provenance diagnostics stay explicit. + flags |= RTLD_FIRST; +#endif + void* handle = dlopen(absolute_path.c_str(), flags); if (handle == nullptr) { const char* error = dlerror(); return unexpected(error == nullptr ? "" : error); @@ -60,8 +89,55 @@ inline Expected loadLibraryHandle(std::string_view path) { #endif } -/// Resolve a named symbol from a loaded library handle. -inline Expected resolveSymbol(void* handle, const char* symbol_name) { +/// Return the filesystem object that defines @p symbol on POSIX platforms. +inline Expected symbolOwner(void* symbol) { +#if defined(_WIN32) + (void)symbol; + return std::filesystem::path{}; +#else + Dl_info info{}; + if (symbol == nullptr || dladdr(symbol, &info) == 0 || info.dli_fname == nullptr || info.dli_fname[0] == '\0') { + return unexpected("dladdr failed to identify the defining object"); + } + return std::filesystem::path(info.dli_fname); +#endif +} + +/// Verify that @p symbol is defined by @p candidate_path, not a dependency. +inline Expected verifySymbolProvenance( + void* symbol, const char* symbol_name, const std::filesystem::path& candidate_path) { +#if defined(_WIN32) + (void)symbol; + (void)symbol_name; + (void)candidate_path; + return {}; +#else + auto owner = symbolOwner(symbol); + if (!owner) { + return unexpected( + "cannot prove provenance for symbol '" + std::string(symbol_name) + "' in candidate '" + + candidate_path.string() + "': " + owner.error()); + } + + std::error_code equivalent_error; + const bool equivalent = std::filesystem::equivalent(*owner, candidate_path, equivalent_error); + if (equivalent_error) { + return unexpected( + "cannot prove provenance for symbol '" + std::string(symbol_name) + "': defining object '" + owner->string() + + "', candidate '" + candidate_path.string() + "': " + equivalent_error.message()); + } + if (!equivalent) { + return unexpected( + "symbol '" + std::string(symbol_name) + "' resolved from dependency '" + owner->string() + + "', not candidate '" + candidate_path.string() + "'"); + } + return {}; +#endif +} + +/// Resolve a named symbol and prove that it is defined by @p candidate_path. +inline Expected resolveSymbol( + void* handle, const char* symbol_name, const std::filesystem::path& candidate_path) { if (handle == nullptr) { return unexpected("library not loaded"); } @@ -71,23 +147,33 @@ inline Expected resolveSymbol(void* handle, const char* symbol_name) { std::string name(symbol_name); return unexpected(name + " not found"); } - return reinterpret_cast(symbol); + void* resolved = reinterpret_cast(symbol); #else dlerror(); void* symbol = dlsym(handle, symbol_name); const char* err = dlerror(); if (err != nullptr) { +#if defined(__APPLE__) + return unexpected( + "cannot prove provenance for symbol '" + std::string(symbol_name) + "' in candidate '" + + candidate_path.string() + "': RTLD_FIRST lookup failed: " + err); +#else return unexpected(err); +#endif } - return symbol; + void* resolved = symbol; #endif + if (auto provenance = verifySymbolProvenance(resolved, symbol_name, candidate_path); !provenance) { + return unexpected(provenance.error()); + } + return resolved; } /// Verify the plugin exports `pj_plugin_abi_version` and its value equals /// PJ_ABI_VERSION. Must be called BEFORE the family vtable is fetched — the /// vtable layout is only meaningful once the boot-level ABI matches. -inline Expected checkPluginAbiVersion(void* handle) { - auto sym = resolveSymbol(handle, "pj_plugin_abi_version"); +inline Expected checkPluginAbiVersion(void* handle, const std::filesystem::path& candidate_path) { + auto sym = resolveSymbol(handle, "pj_plugin_abi_version", candidate_path); if (!sym) { return unexpected("plugin missing pj_plugin_abi_version symbol: " + sym.error()); } @@ -115,4 +201,10 @@ inline std::shared_ptr adoptLibraryHandle(void* handle) { return std::shared_ptr(handle, [](void* loaded_handle) { closeLibraryHandle(loaded_handle); }); } +/// Wrap an already-open library handle with a no-op deleter so process exit can +/// reclaim it after all SDK admission passes share the same native open. +inline std::shared_ptr adoptLibraryHandleNonOwning(void* handle) { + return std::shared_ptr(handle, [](void*) {}); +} + } // namespace PJ::detail diff --git a/pj_plugins/src/detail/native_parser_module_loader.hpp b/pj_plugins/src/detail/native_parser_module_loader.hpp index 8dd4254e..a511fe58 100644 --- a/pj_plugins/src/detail/native_parser_module_loader.hpp +++ b/pj_plugins/src/detail/native_parser_module_loader.hpp @@ -2,11 +2,10 @@ // Copyright 2026 Davide Faconti // SPDX-License-Identifier: Apache-2.0 -#include #include #include +#include #include -#include #if defined(_WIN32) #ifndef NOMINMAX @@ -26,29 +25,16 @@ namespace PJ::detail { using NativeModuleHandle = void*; -inline Expected openNativeParserModule(std::string_view path) { +inline Expected openNativeParserModule(const std::filesystem::path& path) { #if defined(_WIN32) - if (path.size() > static_cast(INT_MAX)) { - return unexpected("native parser-module path is too long"); - } - const int required = - MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, path.data(), static_cast(path.size()), nullptr, 0); - if (required <= 0) { - return unexpected("native parser-module path is not valid UTF-8"); - } - std::wstring wide_path(static_cast(required), L'\0'); - if (MultiByteToWideChar( - CP_UTF8, MB_ERR_INVALID_CHARS, path.data(), static_cast(path.size()), wide_path.data(), required) == 0) { - return unexpected("native parser-module path conversion failed"); - } HMODULE module = - LoadLibraryExW(wide_path.c_str(), nullptr, LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); + LoadLibraryExW(path.c_str(), nullptr, LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); if (module == nullptr) { return unexpected("LoadLibraryExW failed (error " + std::to_string(GetLastError()) + ")"); } return reinterpret_cast(module); #else - void* handle = dlopen(std::string(path).c_str(), RTLD_LOCAL | RTLD_NOW); + void* handle = dlopen(path.c_str(), RTLD_LOCAL | RTLD_NOW); if (handle == nullptr) { const char* error = dlerror(); return unexpected(error == nullptr ? "dlopen failed" : error); diff --git a/pj_plugins/src/message_parser_library.cpp b/pj_plugins/src/message_parser_library.cpp index 5e2b58f5..e77b2895 100644 --- a/pj_plugins/src/message_parser_library.cpp +++ b/pj_plugins/src/message_parser_library.cpp @@ -45,17 +45,31 @@ MessageParserLibrary& MessageParserLibrary::operator=(MessageParserLibrary&& oth } Expected MessageParserLibrary::load(std::string_view path) { + auto library = load(std::filesystem::path(path)); + if (library) { + library->path_ = std::string(path); + } + return library; +} + +Expected MessageParserLibrary::load(const std::filesystem::path& path) { auto raw_handle = detail::loadLibraryHandle(path); if (!raw_handle) { return unexpected(raw_handle.error()); } - auto handle = detail::adoptLibraryHandle(*raw_handle); + return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), path); +} - if (auto abi = detail::checkPluginAbiVersion(handle.get()); !abi) { +Expected MessageParserLibrary::loadFromHandle( + std::shared_ptr handle, const std::filesystem::path& origin) { + if (handle == nullptr) { + return unexpected("library not loaded"); + } + if (auto abi = detail::checkPluginAbiVersion(handle.get(), origin); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_message_parser_vtable"); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_message_parser_vtable", origin); if (!sym) { return unexpected(sym.error()); } @@ -75,7 +89,7 @@ Expected MessageParserLibrary::load(std::string_view path) return unexpected(status.error()); } - return MessageParserLibrary(std::move(handle), vtable, std::string(path)); + return MessageParserLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(origin)); } Expected MessageParserLibrary::loadStatic( @@ -115,7 +129,11 @@ Expected MessageParserLibrary::resolveDialogVtable() if (path_ == "static://") { return unexpected("static MessageParser has no registered dialog vtable"); } - auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable"); +#if defined(_WIN32) + auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", {}); +#else + auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", std::filesystem::path(path_)); +#endif if (!sym) { return unexpected(sym.error()); } diff --git a/pj_plugins/src/native_parser_module.cpp b/pj_plugins/src/native_parser_module.cpp index d1830562..7ff75ab4 100644 --- a/pj_plugins/src/native_parser_module.cpp +++ b/pj_plugins/src/native_parser_module.cpp @@ -3,6 +3,7 @@ #include "pj_plugins/host/native_parser_module.hpp" +#include #include #include #include @@ -61,7 +62,7 @@ NativeParserModule::NativeParserModule(std::shared_ptr NativeParserModule::load( std::string_view path, DiagnosticSink sink, std::string diagnostic_source) { - auto handle_result = detail::openNativeParserModule(path); + auto handle_result = detail::openNativeParserModule(std::filesystem::path(path)); if (!handle_result) { return rejectLoad(path, sink, diagnostic_source, "failed to open native parser module: " + handle_result.error()); } diff --git a/pj_plugins/src/plugin_catalog.cpp b/pj_plugins/src/plugin_catalog.cpp index 1f06ca41..e5033ac2 100644 --- a/pj_plugins/src/plugin_catalog.cpp +++ b/pj_plugins/src/plugin_catalog.cpp @@ -39,12 +39,6 @@ struct ManifestCandidate { std::string manifest_json; }; -struct LibraryHandleCloser { - void operator()(void* handle) const { - detail::closeLibraryHandle(handle); - } -}; - bool hasDsoSuffix(const std::filesystem::path& path) { return path.extension().string() == kDsoSuffix; } @@ -54,9 +48,9 @@ bool hasDsoSuffix(const std::filesystem::path& path) { // Only the family-specific types and constants vary. template Expected probeDirectVtable( - void* handle, const char* symbol, const char* family_name, uint32_t expected_protocol, size_t min_vtable_size, - PluginFamily family) { - auto sym = detail::resolveSymbol(handle, symbol); + void* handle, const std::filesystem::path& origin, const char* symbol, const char* family_name, + uint32_t expected_protocol, size_t min_vtable_size, PluginFamily family) { + auto sym = detail::resolveSymbol(handle, symbol, origin); if (!sym) { return unexpected(sym.error()); } @@ -76,26 +70,26 @@ Expected probeDirectVtable( return ManifestCandidate{family, vt->manifest_json == nullptr ? "" : vt->manifest_json}; } -Expected tryDataSource(void* handle) { +Expected tryDataSource(void* handle, const std::filesystem::path& origin) { return probeDirectVtable( - handle, "PJ_get_data_source_vtable", "DataSource", PJ_DATA_SOURCE_PROTOCOL_VERSION, + handle, origin, "PJ_get_data_source_vtable", "DataSource", PJ_DATA_SOURCE_PROTOCOL_VERSION, PJ_DATA_SOURCE_MIN_VTABLE_SIZE, PluginFamily::kDataSource); } -Expected tryMessageParser(void* handle) { +Expected tryMessageParser(void* handle, const std::filesystem::path& origin) { return probeDirectVtable( - handle, "PJ_get_message_parser_vtable", "MessageParser", PJ_MESSAGE_PARSER_PROTOCOL_VERSION, + handle, origin, "PJ_get_message_parser_vtable", "MessageParser", PJ_MESSAGE_PARSER_PROTOCOL_VERSION, PJ_MESSAGE_PARSER_MIN_VTABLE_SIZE, PluginFamily::kMessageParser); } -Expected tryToolbox(void* handle) { +Expected tryToolbox(void* handle, const std::filesystem::path& origin) { return probeDirectVtable( - handle, "PJ_get_toolbox_vtable", "Toolbox", PJ_TOOLBOX_PLUGIN_PROTOCOL_VERSION, PJ_TOOLBOX_MIN_VTABLE_SIZE, - PluginFamily::kToolbox); + handle, origin, "PJ_get_toolbox_vtable", "Toolbox", PJ_TOOLBOX_PLUGIN_PROTOCOL_VERSION, + PJ_TOOLBOX_MIN_VTABLE_SIZE, PluginFamily::kToolbox); } -Expected tryDialog(void* handle) { - auto sym = detail::resolveSymbol(handle, "PJ_get_dialog_vtable"); +Expected tryDialog(void* handle, const std::filesystem::path& origin) { + auto sym = detail::resolveSymbol(handle, "PJ_get_dialog_vtable", origin); if (!sym) { return unexpected(sym.error()); } @@ -127,28 +121,28 @@ Expected tryDialog(void* handle) { return ManifestCandidate{PluginFamily::kDialog, std::move(manifest_json)}; } -Expected findEmbeddedManifest(void* handle) { +Expected findEmbeddedManifest(void* handle, const std::filesystem::path& origin) { std::vector errors; - if (auto candidate = tryDataSource(handle)) { + if (auto candidate = tryDataSource(handle, origin)) { return *candidate; } else { errors.push_back(fmt::format("data_source: {}", candidate.error())); } - if (auto candidate = tryMessageParser(handle)) { + if (auto candidate = tryMessageParser(handle, origin)) { return *candidate; } else { errors.push_back(fmt::format("message_parser: {}", candidate.error())); } - if (auto candidate = tryToolbox(handle)) { + if (auto candidate = tryToolbox(handle, origin)) { return *candidate; } else { errors.push_back(fmt::format("toolbox: {}", candidate.error())); } - if (auto candidate = tryDialog(handle)) { + if (auto candidate = tryDialog(handle, origin)) { return *candidate; } else { errors.push_back(fmt::format("dialog: {}", candidate.error())); @@ -312,19 +306,29 @@ Expected inspectPluginDso(const std::filesystem::path& dso_pat if (!hasDsoSuffix(dso_path)) { return unexpected(fmt::format("not a platform plugin DSO: {}", dso_path.string())); } - auto with_path = [&](const std::string& error) { return fmt::format("{}: {}", dso_path.string(), error); }; - auto handle = detail::loadLibraryHandle(dso_path.string()); - if (!handle) { - return unexpected(with_path(handle.error())); + auto raw_handle = detail::loadLibraryHandle(dso_path); + if (!raw_handle) { + return unexpected(fmt::format("{}: {}", dso_path.string(), raw_handle.error())); } - std::unique_ptr library(*handle); + return inspectPluginDso(detail::adoptLibraryHandle(*raw_handle), dso_path); +} - if (auto abi = detail::checkPluginAbiVersion(library.get()); !abi) { +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())); + } + auto with_path = [&](const std::string& error) { return fmt::format("{}: {}", dso_path.string(), error); }; + if (handle == nullptr) { + return unexpected(with_path("library not loaded")); + } + + if (auto abi = detail::checkPluginAbiVersion(handle.get(), dso_path); !abi) { return unexpected(with_path(abi.error())); } - auto candidate = findEmbeddedManifest(library.get()); + auto candidate = findEmbeddedManifest(handle.get(), dso_path); if (!candidate) { return unexpected(with_path(candidate.error())); } @@ -336,6 +340,29 @@ Expected inspectPluginDso(const std::filesystem::path& dso_pat return *descriptor; } +namespace detail { + +std::vector exportedPluginFamilies( + const std::shared_ptr& handle, const std::filesystem::path& dso_path) { + std::vector families; + if (handle == nullptr) { + return families; + } + + auto append_if_owned = [&](const char* symbol, PluginFamily family) { + if (resolveSymbol(handle.get(), symbol, dso_path)) { + families.push_back(family); + } + }; + append_if_owned("PJ_get_data_source_vtable", PluginFamily::kDataSource); + append_if_owned("PJ_get_message_parser_vtable", PluginFamily::kMessageParser); + append_if_owned("PJ_get_toolbox_vtable", PluginFamily::kToolbox); + append_if_owned("PJ_get_dialog_vtable", PluginFamily::kDialog); + return families; +} + +} // namespace detail + Expected scanPluginDsos(const std::filesystem::path& directory) { std::error_code ec; if (!std::filesystem::exists(directory, ec)) { diff --git a/pj_plugins/src/toolbox_library.cpp b/pj_plugins/src/toolbox_library.cpp index 9986a1da..c07c35ba 100644 --- a/pj_plugins/src/toolbox_library.cpp +++ b/pj_plugins/src/toolbox_library.cpp @@ -45,17 +45,31 @@ ToolboxLibrary& ToolboxLibrary::operator=(ToolboxLibrary&& other) noexcept { } Expected ToolboxLibrary::load(std::string_view path) { + auto library = load(std::filesystem::path(path)); + if (library) { + library->path_ = std::string(path); + } + return library; +} + +Expected ToolboxLibrary::load(const std::filesystem::path& path) { auto raw_handle = detail::loadLibraryHandle(path); if (!raw_handle) { return unexpected(raw_handle.error()); } - auto handle = detail::adoptLibraryHandle(*raw_handle); + return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), path); +} - if (auto abi = detail::checkPluginAbiVersion(handle.get()); !abi) { +Expected ToolboxLibrary::loadFromHandle( + std::shared_ptr handle, const std::filesystem::path& origin) { + if (handle == nullptr) { + return unexpected("library not loaded"); + } + if (auto abi = detail::checkPluginAbiVersion(handle.get(), origin); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_toolbox_vtable"); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_toolbox_vtable", origin); if (!sym) { return unexpected(sym.error()); } @@ -75,7 +89,7 @@ Expected ToolboxLibrary::load(std::string_view path) { return unexpected(status.error()); } - return ToolboxLibrary(std::move(handle), vtable, std::string(path)); + return ToolboxLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(origin)); } Expected ToolboxLibrary::loadStatic( @@ -115,7 +129,11 @@ Expected ToolboxLibrary::resolveDialogVtable() const if (path_ == "static://") { return unexpected("static Toolbox has no registered dialog vtable"); } - auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable"); +#if defined(_WIN32) + auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", {}); +#else + auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", std::filesystem::path(path_)); +#endif if (!sym) { return unexpected(sym.error()); } diff --git a/pj_plugins/tests/dependency_search_candidate.cpp b/pj_plugins/tests/dependency_search_candidate.cpp new file mode 100644 index 00000000..7ed09eb7 --- /dev/null +++ b/pj_plugins/tests/dependency_search_candidate.cpp @@ -0,0 +1,85 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include "pj_base/data_source_protocol.h" +#include "pj_base/plugin_abi_export.hpp" + +#if defined(_WIN32) +#define PJ_FIXTURE_IMPORT __declspec(dllimport) +#else +#define PJ_FIXTURE_IMPORT +#endif + +extern "C" PJ_FIXTURE_IMPORT const char* pj_dependency_search_manifest() noexcept; + +namespace { + +void* create() noexcept { + return reinterpret_cast(0x1); +} + +void destroy(void*) noexcept {} + +uint64_t capabilities(void*) noexcept { + return 0; +} + +bool ok(void*, PJ_service_registry_t, PJ_error_t*) noexcept { + return true; +} + +bool saveConfig(void*, PJ_string_view_t* out_json, PJ_error_t*) noexcept { + static constexpr char kJson[] = "{}"; + if (out_json != nullptr) { + out_json->data = kJson; + out_json->size = 2; + } + return true; +} + +bool loadConfig(void*, PJ_string_view_t, PJ_error_t*) noexcept { + return true; +} + +bool action(void*, PJ_error_t*) noexcept { + return true; +} + +void stop(void*) noexcept {} + +PJ_data_source_state_t state(void*) noexcept { + return PJ_DATA_SOURCE_STATE_IDLE; +} + +PJ_borrowed_dialog_t dialog(void*) noexcept { + return PJ_borrowed_dialog_t{nullptr, nullptr}; +} + +const void* extension(void*, PJ_string_view_t) noexcept { + return nullptr; +} + +} // namespace + +extern "C" PJ_DATA_SOURCE_EXPORT const PJ_data_source_vtable_t* PJ_get_data_source_vtable() noexcept { + static const PJ_data_source_vtable_t vtable = { + .protocol_version = PJ_DATA_SOURCE_PROTOCOL_VERSION, + .struct_size = sizeof(PJ_data_source_vtable_t), + .create = create, + .destroy = destroy, + .manifest_json = pj_dependency_search_manifest(), + .capabilities = capabilities, + .bind = ok, + .save_config = saveConfig, + .load_config = loadConfig, + .start = action, + .stop = stop, + .pause = action, + .resume = action, + .poll = action, + .current_state = state, + .get_dialog = dialog, + .get_plugin_extension = extension, + }; + return &vtable; +} diff --git a/pj_plugins/tests/dependency_search_dependency.cpp b/pj_plugins/tests/dependency_search_dependency.cpp new file mode 100644 index 00000000..79d00e21 --- /dev/null +++ b/pj_plugins/tests/dependency_search_dependency.cpp @@ -0,0 +1,16 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#if defined(_WIN32) +#define PJ_FIXTURE_EXPORT __declspec(dllexport) +#else +#define PJ_FIXTURE_EXPORT __attribute__((visibility("default"))) +#endif + +extern "C" PJ_FIXTURE_EXPORT const char* pj_dependency_search_manifest() noexcept { +#if defined(PJ_DEPENDENCY_SEARCH_DECOY) + return R"({"id":"dependency-search-decoy","name":"Dependency Search Decoy","version":"1.0.0"})"; +#else + return R"({"id":"dependency-search-real","name":"Dependency Search Real","version":"1.0.0"})"; +#endif +} diff --git a/pj_plugins/tests/entry_point_donor.cpp b/pj_plugins/tests/entry_point_donor.cpp new file mode 100644 index 00000000..f8355ff3 --- /dev/null +++ b/pj_plugins/tests/entry_point_donor.cpp @@ -0,0 +1,81 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include "pj_base/data_source_protocol.h" +#include "pj_base/plugin_abi_export.hpp" + +extern "C" PJ_DATA_SOURCE_EXPORT int pj_entry_point_donor_marker() noexcept { + return 1; +} + +namespace { + +void* create() noexcept { + return reinterpret_cast(0x1); +} + +void destroy(void*) noexcept {} + +uint64_t capabilities(void*) noexcept { + return 0; +} + +bool ok(void*, PJ_service_registry_t, PJ_error_t*) noexcept { + return true; +} + +bool saveConfig(void*, PJ_string_view_t* out_json, PJ_error_t*) noexcept { + static constexpr char kJson[] = "{}"; + if (out_json != nullptr) { + out_json->data = kJson; + out_json->size = 2; + } + return true; +} + +bool loadConfig(void*, PJ_string_view_t, PJ_error_t*) noexcept { + return true; +} + +bool action(void*, PJ_error_t*) noexcept { + return true; +} + +void stop(void*) noexcept {} + +PJ_data_source_state_t state(void*) noexcept { + return PJ_DATA_SOURCE_STATE_IDLE; +} + +PJ_borrowed_dialog_t dialog(void*) noexcept { + return PJ_borrowed_dialog_t{nullptr, nullptr}; +} + +const void* extension(void*, PJ_string_view_t) noexcept { + return nullptr; +} + +} // namespace + +extern "C" PJ_DATA_SOURCE_EXPORT const PJ_data_source_vtable_t* PJ_get_data_source_vtable() noexcept { + static const PJ_data_source_vtable_t vtable = { + .protocol_version = PJ_DATA_SOURCE_PROTOCOL_VERSION, + .struct_size = sizeof(PJ_data_source_vtable_t), + .create = create, + .destroy = destroy, + .manifest_json = R"({"id":"entry-point-donor","name":"Entry Point Donor","version":"1.0.0"})", + .capabilities = capabilities, + .bind = ok, + .save_config = saveConfig, + .load_config = loadConfig, + .start = action, + .stop = stop, + .pause = action, + .resume = action, + .poll = action, + .current_state = state, + .get_dialog = dialog, + .get_plugin_extension = extension, + }; + return &vtable; +} diff --git a/pj_plugins/tests/entry_point_via_dependency.cpp b/pj_plugins/tests/entry_point_via_dependency.cpp new file mode 100644 index 00000000..788f9daa --- /dev/null +++ b/pj_plugins/tests/entry_point_via_dependency.cpp @@ -0,0 +1,20 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include "pj_base/plugin_data_api.h" + +#if defined(_WIN32) +#define PJ_FIXTURE_IMPORT __declspec(dllimport) +#else +#define PJ_FIXTURE_IMPORT +#endif + +extern "C" PJ_FIXTURE_IMPORT int pj_entry_point_donor_marker() noexcept; + +namespace { + +// Force the donor to remain in the candidate's dependency closure even when +// the toolchain links shared libraries with --as-needed. +[[maybe_unused]] const int kKeepDonorDependency = pj_entry_point_donor_marker(); + +} // namespace diff --git a/pj_plugins/tests/entry_point_with_own_exports.cpp b/pj_plugins/tests/entry_point_with_own_exports.cpp new file mode 100644 index 00000000..d510a152 --- /dev/null +++ b/pj_plugins/tests/entry_point_with_own_exports.cpp @@ -0,0 +1,87 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include "pj_base/data_source_protocol.h" +#include "pj_base/plugin_abi_export.hpp" + +#if defined(_WIN32) +#define PJ_FIXTURE_IMPORT __declspec(dllimport) +#else +#define PJ_FIXTURE_IMPORT +#endif + +extern "C" PJ_FIXTURE_IMPORT int pj_entry_point_donor_marker() noexcept; + +namespace { + +[[maybe_unused]] const int kKeepDonorDependency = pj_entry_point_donor_marker(); + +void* create() noexcept { + return reinterpret_cast(0x1); +} + +void destroy(void*) noexcept {} + +uint64_t capabilities(void*) noexcept { + return 0; +} + +bool ok(void*, PJ_service_registry_t, PJ_error_t*) noexcept { + return true; +} + +bool saveConfig(void*, PJ_string_view_t* out_json, PJ_error_t*) noexcept { + static constexpr char kJson[] = "{}"; + if (out_json != nullptr) { + out_json->data = kJson; + out_json->size = 2; + } + return true; +} + +bool loadConfig(void*, PJ_string_view_t, PJ_error_t*) noexcept { + return true; +} + +bool action(void*, PJ_error_t*) noexcept { + return true; +} + +void stop(void*) noexcept {} + +PJ_data_source_state_t state(void*) noexcept { + return PJ_DATA_SOURCE_STATE_IDLE; +} + +PJ_borrowed_dialog_t dialog(void*) noexcept { + return PJ_borrowed_dialog_t{nullptr, nullptr}; +} + +const void* extension(void*, PJ_string_view_t) noexcept { + return nullptr; +} + +} // namespace + +extern "C" PJ_DATA_SOURCE_EXPORT const PJ_data_source_vtable_t* PJ_get_data_source_vtable() noexcept { + static const PJ_data_source_vtable_t vtable = { + .protocol_version = PJ_DATA_SOURCE_PROTOCOL_VERSION, + .struct_size = sizeof(PJ_data_source_vtable_t), + .create = create, + .destroy = destroy, + .manifest_json = R"({"id":"entry-point-candidate","name":"Entry Point Candidate","version":"1.0.0"})", + .capabilities = capabilities, + .bind = ok, + .save_config = saveConfig, + .load_config = loadConfig, + .start = action, + .stop = stop, + .pause = action, + .resume = action, + .poll = action, + .current_state = state, + .get_dialog = dialog, + .get_plugin_extension = extension, + }; + return &vtable; +} diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index 0d78a1d9..d3ea8daf 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -9,8 +9,23 @@ #include #include #include +#include #include #include +#include + +#include "detail/library_loader.hpp" +#include "pj_plugins/host/data_source_library.hpp" + +#if defined(_WIN32) +#ifndef NOMINMAX +#define NOMINMAX +#endif +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#include +#endif namespace PJ { namespace { @@ -120,6 +135,110 @@ TEST_F(PluginCatalogTest, InspectRequiredPrefixOnlyDialogDsoDoesNotReadStaticMan EXPECT_EQ(descriptor->family, PluginFamily::kDialog); } +TEST_F(PluginCatalogTest, EntryPointSymbolResolvedFromDependencyIsRejected) { +#if !defined(_WIN32) + auto is_provenance_error = [](const std::string& error) { + return error.find("resolved from dependency") != std::string::npos || + error.find("cannot prove provenance") != std::string::npos; + }; + auto dependency_library = DataSourceLibrary::load(PJ_ENTRY_POINT_VIA_DEPENDENCY_PLUGIN_PATH); + ASSERT_FALSE(dependency_library.has_value()); + EXPECT_TRUE(is_provenance_error(dependency_library.error())) << dependency_library.error(); + + auto dependency_descriptor = inspectPluginDso(PJ_ENTRY_POINT_VIA_DEPENDENCY_PLUGIN_PATH); + ASSERT_FALSE(dependency_descriptor.has_value()); + EXPECT_TRUE(is_provenance_error(dependency_descriptor.error())) << dependency_descriptor.error(); +#endif + + auto library = DataSourceLibrary::load(PJ_ENTRY_POINT_WITH_OWN_EXPORTS_PLUGIN_PATH); + ASSERT_TRUE(library.has_value()) << library.error(); + + auto descriptor = inspectPluginDso(PJ_ENTRY_POINT_WITH_OWN_EXPORTS_PLUGIN_PATH); + ASSERT_TRUE(descriptor.has_value()) << descriptor.error(); + EXPECT_EQ(descriptor->id, "entry-point-candidate"); +} + +TEST_F(PluginCatalogTest, UnicodeExtensionPathLoadsOnWindows) { + const std::filesystem::path unicode_dir = dir_ / std::filesystem::path(u8"插件-π"); + std::filesystem::create_directories(unicode_dir); + const std::filesystem::path plugin_path = unicode_dir / pluginFileName("unicode_plugin"); + std::filesystem::copy_file(PJ_MOCK_DATA_SOURCE_PLUGIN_PATH, plugin_path); + + auto descriptor = inspectPluginDso(plugin_path); + ASSERT_TRUE(descriptor.has_value()) << descriptor.error(); + EXPECT_EQ(descriptor->id, "mock-data-source"); + + auto library = DataSourceLibrary::load(plugin_path); + ASSERT_TRUE(library.has_value()) << library.error(); + EXPECT_TRUE(library->valid()); +} + +TEST_F(PluginCatalogTest, AlreadyOpenHandleSupportsInspectionLoadingAndFamilyQuery) { + const std::filesystem::path plugin_path = PJ_MOCK_DATA_SOURCE_PLUGIN_PATH; + auto raw_handle = detail::loadLibraryHandle(plugin_path); + ASSERT_TRUE(raw_handle.has_value()) << raw_handle.error(); + auto owner = detail::adoptLibraryHandle(*raw_handle); + auto shared_handle = detail::adoptLibraryHandleNonOwning(owner.get()); + + const auto families = detail::exportedPluginFamilies(shared_handle, plugin_path); + EXPECT_EQ(families, std::vector{PluginFamily::kDataSource}); + + auto descriptor = inspectPluginDso(shared_handle, plugin_path); + ASSERT_TRUE(descriptor.has_value()) << descriptor.error(); + EXPECT_EQ(descriptor->id, "mock-data-source"); + + auto library = DataSourceLibrary::loadFromHandle(shared_handle, plugin_path); + ASSERT_TRUE(library.has_value()) << library.error(); + EXPECT_TRUE(library->valid()); +} + +TEST_F(PluginCatalogTest, DependencySearchExcludesCwdAndPath) { + const std::filesystem::path candidate_dir = dir_ / "candidate"; + const std::filesystem::path cwd_decoy_dir = dir_ / "cwd-decoy"; + const std::filesystem::path path_decoy_dir = dir_ / "path-decoy"; + std::filesystem::create_directories(candidate_dir); + std::filesystem::create_directories(cwd_decoy_dir); + std::filesystem::create_directories(path_decoy_dir); + + const std::filesystem::path candidate_source = PJ_DEPENDENCY_SEARCH_CANDIDATE_PATH; + const std::filesystem::path real_source = PJ_DEPENDENCY_SEARCH_REAL_PATH; + const std::filesystem::path decoy_source = PJ_DEPENDENCY_SEARCH_DECOY_PATH; + const std::filesystem::path candidate = candidate_dir / candidate_source.filename(); + const std::filesystem::path sibling = candidate_dir / real_source.filename(); + std::filesystem::copy_file(candidate_source, candidate); + std::filesystem::copy_file(real_source, sibling); + std::filesystem::copy_file(decoy_source, cwd_decoy_dir / real_source.filename()); + std::filesystem::copy_file(decoy_source, path_decoy_dir / real_source.filename()); + + const std::filesystem::path original_cwd = std::filesystem::current_path(); +#if defined(_WIN32) + std::optional old_path; + const DWORD old_path_size = GetEnvironmentVariableW(L"PATH", nullptr, 0); + if (old_path_size > 0) { + std::wstring value(old_path_size, L'\0'); + const DWORD copied = GetEnvironmentVariableW(L"PATH", value.data(), old_path_size); + ASSERT_GT(copied, 0U); + value.resize(copied); + old_path = std::move(value); + } + ASSERT_NE(SetEnvironmentVariableW(L"PATH", path_decoy_dir.c_str()), 0); +#endif + std::filesystem::current_path(cwd_decoy_dir); + + auto sibling_result = inspectPluginDso(candidate); + std::filesystem::remove(sibling); + auto decoy_only_result = DataSourceLibrary::load(candidate); + + std::filesystem::current_path(original_cwd); +#if defined(_WIN32) + ASSERT_NE(SetEnvironmentVariableW(L"PATH", old_path.has_value() ? old_path->c_str() : nullptr), 0); +#endif + + ASSERT_TRUE(sibling_result.has_value()) << sibling_result.error(); + EXPECT_EQ(sibling_result->id, "dependency-search-real"); + EXPECT_FALSE(decoy_only_result.has_value()); +} + TEST_F(PluginCatalogTest, MissingIdManifestIsRejected) { auto descriptor = inspectPluginDso(PJ_MISSING_ID_PLUGIN_PATH); ASSERT_FALSE(descriptor.has_value()); From c77f3841c283fe60fb6bfc3ca0fd4d6760709937 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Tue, 11 Aug 2026 08:42:16 +0200 Subject: [PATCH 2/5] fix(pj_plugins): review-round hardening for loader provenance (X1-X6) Record the normalized absolute dlopen path and prove provenance against it byte-first (deferred dialog resolution survives file deletion and CWD changes); restore the UTF-8 contract of the narrow native-parser API on Windows; normalize native-parser paths to absolute; verify Windows provenance via GetModuleHandleExW(FROM_ADDRESS) so PE forwarded exports are rejected; route native-parser symbols through the shared provenance resolver with RTLD_FIRST on macOS. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0165qLfnJYSYb51NQMdPidZr --- CHANGELOG.md | 13 +++- pj_plugins/CMakeLists.txt | 19 +++++ .../dialog_protocol/src/dialog_library.cpp | 21 +++--- pj_plugins/docs/ARCHITECTURE.md | 20 ++++- pj_plugins/src/data_source_library.cpp | 21 +++--- pj_plugins/src/detail/library_loader.hpp | 67 ++++++++++++++--- .../detail/native_parser_module_loader.hpp | 68 +++++++---------- pj_plugins/src/message_parser_library.cpp | 21 +++--- pj_plugins/src/native_parser_module.cpp | 10 ++- pj_plugins/src/plugin_catalog.cpp | 20 +++-- pj_plugins/src/toolbox_library.cpp | 21 +++--- pj_plugins/tests/entry_point_forwarder.cpp | 4 + pj_plugins/tests/entry_point_forwarder.def | 3 + .../tests/native_parser_module_test.cpp | 64 ++++++++++++++++ pj_plugins/tests/plugin_catalog_test.cpp | 12 +++ .../tests/source_dialog_integration_test.cpp | 75 +++++++++++++++++++ 16 files changed, 354 insertions(+), 105 deletions(-) create mode 100644 pj_plugins/tests/entry_point_forwarder.cpp create mode 100644 pj_plugins/tests/entry_point_forwarder.def diff --git a/CHANGELOG.md b/CHANGELOG.md index 80bf057b..c025c5a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,17 @@ Plugin admission now proves that the ABI marker and family vtable getter are defined by the candidate DSO itself instead of accepting definitions from a dependency. POSIX uses defining-object identity, macOS restricts handle-scoped lookups to the first image, and Windows uses filesystem-native wide paths with -package-scoped dependency search. Plugin install rpaths now resolve bundled -dependencies relative to the plugin on Linux and macOS. +package-scoped dependency search and defining-module checks that reject +forwarded PE exports. Recorded normalized absolute load paths let deferred +dialog-vtable provenance accept exact defining-path matches without re-stating +the candidate, so staged deletion and later working-directory changes are +safe. Plugin install rpaths now resolve bundled dependencies relative to the +plugin on Linux and macOS. + +Native functional parser modules now share the same absolute-path open and +symbol-provenance checks, including `RTLD_FIRST` on macOS. Their narrow load +API retains its explicit UTF-8 contract on Windows and rejects invalid UTF-8 +before calling the platform loader. New filesystem-path overloads and already-open-handle adoption APIs let hosts validate, inspect, and instantiate a candidate through one native module open. diff --git a/pj_plugins/CMakeLists.txt b/pj_plugins/CMakeLists.txt index 2083b9d9..01b42cca 100644 --- a/pj_plugins/CMakeLists.txt +++ b/pj_plugins/CMakeLists.txt @@ -361,6 +361,17 @@ target_compile_features(entry_point_with_own_exports_plugin PRIVATE cxx_std_20) target_compile_options(entry_point_with_own_exports_plugin PRIVATE ${PJ_WARNING_FLAGS}) target_link_libraries(entry_point_with_own_exports_plugin PRIVATE entry_point_donor pj_base) +if(WIN32) + add_library(entry_point_forwarder_plugin SHARED + tests/entry_point_forwarder.cpp + tests/entry_point_forwarder.def + ) + target_compile_features(entry_point_forwarder_plugin PRIVATE cxx_std_20) + target_compile_options(entry_point_forwarder_plugin PRIVATE ${PJ_WARNING_FLAGS}) + target_link_libraries(entry_point_forwarder_plugin PRIVATE pj_base) + add_dependencies(entry_point_forwarder_plugin entry_point_donor) +endif() + # Dependency-search fixtures. The real and decoy dependencies intentionally # share one filename but live in separate build directories. add_library(dependency_search_real SHARED tests/dependency_search_dependency.cpp) @@ -623,6 +634,11 @@ target_compile_definitions(plugin_catalog_test PRIVATE PJ_DEPENDENCY_SEARCH_REAL_PATH="$" PJ_DEPENDENCY_SEARCH_DECOY_PATH="$" ) +if(WIN32) + target_compile_definitions(plugin_catalog_test PRIVATE + PJ_ENTRY_POINT_FORWARDER_PLUGIN_PATH="$" + ) +endif() target_compile_options(plugin_catalog_test PRIVATE ${PJ_WARNING_FLAGS}) target_include_directories(plugin_catalog_test PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src) target_link_libraries(plugin_catalog_test PRIVATE @@ -636,6 +652,9 @@ add_dependencies(plugin_catalog_test mock_data_source_plugin old_dialog_vtable_plugin missing_dialog_required_slots_plugin entry_point_via_dependency_plugin entry_point_with_own_exports_plugin dependency_search_candidate_plugin dependency_search_decoy) +if(WIN32) + add_dependencies(plugin_catalog_test entry_point_forwarder_plugin) +endif() add_test(NAME plugin_catalog_test COMMAND plugin_catalog_test) endif() # PJ_BUILD_TESTS diff --git a/pj_plugins/dialog_protocol/src/dialog_library.cpp b/pj_plugins/dialog_protocol/src/dialog_library.cpp index f6414d06..b7907c53 100644 --- a/pj_plugins/dialog_protocol/src/dialog_library.cpp +++ b/pj_plugins/dialog_protocol/src/dialog_library.cpp @@ -34,19 +34,16 @@ DialogLibrary& DialogLibrary::operator=(DialogLibrary&& other) noexcept { } Expected DialogLibrary::load(std::string_view path) { - auto library = load(std::filesystem::path(path)); - if (library) { - library->path_ = std::string(path); - } - return library; + return load(std::filesystem::path(path)); } Expected DialogLibrary::load(const std::filesystem::path& path) { - auto raw_handle = detail::loadLibraryHandle(path); + std::filesystem::path loaded_path; + auto raw_handle = detail::loadLibraryHandle(path, &loaded_path); if (!raw_handle) { return unexpected(raw_handle.error()); } - return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), path); + return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), loaded_path); } Expected DialogLibrary::loadFromHandle( @@ -54,11 +51,15 @@ Expected DialogLibrary::loadFromHandle( if (handle == nullptr) { return unexpected("library not loaded"); } - if (auto abi = detail::checkPluginAbiVersion(handle.get(), origin); !abi) { + auto loaded_path = detail::normalizedAbsoluteLibraryPath(origin); + if (!loaded_path) { + return unexpected(loaded_path.error()); + } + if (auto abi = detail::checkPluginAbiVersion(handle.get(), *loaded_path); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_dialog_vtable", origin); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_dialog_vtable", *loaded_path); if (!sym) { return unexpected(sym.error()); } @@ -78,7 +79,7 @@ Expected DialogLibrary::loadFromHandle( return unexpected(status.error()); } - return DialogLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(origin)); + return DialogLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(*loaded_path)); } void DialogLibrary::reset() { diff --git a/pj_plugins/docs/ARCHITECTURE.md b/pj_plugins/docs/ARCHITECTURE.md index 303696c6..0b780973 100644 --- a/pj_plugins/docs/ARCHITECTURE.md +++ b/pj_plugins/docs/ARCHITECTURE.md @@ -452,9 +452,17 @@ These live in `sdk/detail/*_trampolines.hpp`. ## 5. Host Loaders Each family has a loader that: -1. Calls `dlopen` (or `LoadLibrary` on Windows) on the `.so` path. +1. Lexically normalizes the candidate to an absolute filesystem path, passes + that exact path to `dlopen` (or `LoadLibraryExW` on Windows), and records it + in the library object for later symbol resolution. 2. Resolves the ABI marker and entry point, then verifies that each symbol's - defining object is the candidate DSO itself rather than a dependency. + defining object is the candidate DSO itself rather than a dependency. On + POSIX, an exact byte match between `dladdr().dli_fname` and the recorded load + path succeeds without re-reading the filesystem; `equivalent()` is only the + fallback for different path spellings. On Windows, the defining `HMODULE` + is recovered from the resolved address with `GetModuleHandleExW(... + FROM_ADDRESS ...)` and compared to the candidate handle, which also rejects + forwarded PE exports. 3. Validates `protocol_version` and `struct_size`. 4. Stores the vtable pointer for creating handles. @@ -467,7 +475,13 @@ Each family has a loader that: Loaders also provide `resolveDialogVtable()` to find the dialog vtable in a plugin `.so` that exports both a family vtable and a dialog vtable (e.g. a -DataSource with an embedded dialog). +DataSource with an embedded dialog). These deferred lookups use the recorded +load path, so they remain valid after the candidate file is removed or the +process working directory changes. + +Native functional parser modules use the same absolute-path normalization, +package-scoped platform open, and defining-module provenance checks for every +required ABI export. Their narrow path API is explicitly UTF-8 on Windows. ### 5.1 Host-side diagnostic propagation diff --git a/pj_plugins/src/data_source_library.cpp b/pj_plugins/src/data_source_library.cpp index 9d6aa668..deadcd8f 100644 --- a/pj_plugins/src/data_source_library.cpp +++ b/pj_plugins/src/data_source_library.cpp @@ -45,19 +45,16 @@ DataSourceLibrary& DataSourceLibrary::operator=(DataSourceLibrary&& other) noexc } Expected DataSourceLibrary::load(std::string_view path) { - auto library = load(std::filesystem::path(path)); - if (library) { - library->path_ = std::string(path); - } - return library; + return load(std::filesystem::path(path)); } Expected DataSourceLibrary::load(const std::filesystem::path& path) { - auto raw_handle = detail::loadLibraryHandle(path); + std::filesystem::path loaded_path; + auto raw_handle = detail::loadLibraryHandle(path, &loaded_path); if (!raw_handle) { return unexpected(raw_handle.error()); } - return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), path); + return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), loaded_path); } Expected DataSourceLibrary::loadFromHandle( @@ -65,11 +62,15 @@ Expected DataSourceLibrary::loadFromHandle( if (handle == nullptr) { return unexpected("library not loaded"); } - if (auto abi = detail::checkPluginAbiVersion(handle.get(), origin); !abi) { + auto loaded_path = detail::normalizedAbsoluteLibraryPath(origin); + if (!loaded_path) { + return unexpected(loaded_path.error()); + } + if (auto abi = detail::checkPluginAbiVersion(handle.get(), *loaded_path); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_data_source_vtable", origin); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_data_source_vtable", *loaded_path); if (!sym) { return unexpected(sym.error()); } @@ -91,7 +92,7 @@ Expected DataSourceLibrary::loadFromHandle( return unexpected(status.error()); } - return DataSourceLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(origin)); + return DataSourceLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(*loaded_path)); } Expected DataSourceLibrary::loadStatic( diff --git a/pj_plugins/src/detail/library_loader.hpp b/pj_plugins/src/detail/library_loader.hpp index 8142d2ff..bfce9b16 100644 --- a/pj_plugins/src/detail/library_loader.hpp +++ b/pj_plugins/src/detail/library_loader.hpp @@ -35,9 +35,12 @@ inline std::string pathForLegacyAccessor(const std::filesystem::path& path) { #endif } -inline Expected loadLibraryHandle(const std::filesystem::path& path) { +/// Produce the normalized absolute spelling used for the native loader call. +/// This is deliberately lexical: loading does not require the candidate to +/// remain stat-able after the native module handle has been acquired. +inline Expected normalizedAbsoluteLibraryPath(const std::filesystem::path& path) { std::error_code path_error; - const std::filesystem::path absolute_path = std::filesystem::absolute(path, path_error); + std::filesystem::path absolute_path = std::filesystem::absolute(path, path_error); if (path_error) { #if defined(_WIN32) return unexpected("cannot make library path absolute: " + path_error.message()); @@ -45,12 +48,24 @@ inline Expected loadLibraryHandle(const std::filesystem::path& path) { return unexpected("cannot make library path absolute '" + path.string() + "': " + path_error.message()); #endif } + return absolute_path.lexically_normal(); +} + +inline Expected loadLibraryHandle( + const std::filesystem::path& path, std::filesystem::path* loaded_path = nullptr) { + auto absolute_path = normalizedAbsoluteLibraryPath(path); + if (!absolute_path) { + return unexpected(absolute_path.error()); + } #if defined(_WIN32) HMODULE module = LoadLibraryExW( - absolute_path.c_str(), nullptr, LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); + absolute_path->c_str(), nullptr, LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); if (module == nullptr) { return unexpected("LoadLibraryExW failed (error " + std::to_string(GetLastError()) + ")"); } + if (loaded_path != nullptr) { + *loaded_path = *absolute_path; + } return reinterpret_cast(module); #else // RTLD_NOW — resolve all symbols now; fail-fast on missing ones. @@ -80,11 +95,14 @@ inline Expected loadLibraryHandle(const std::filesystem::path& path) { // admission behavior is intentional and provenance diagnostics stay explicit. flags |= RTLD_FIRST; #endif - void* handle = dlopen(absolute_path.c_str(), flags); + void* handle = dlopen(absolute_path->c_str(), flags); if (handle == nullptr) { const char* error = dlerror(); return unexpected(error == nullptr ? "" : error); } + if (loaded_path != nullptr) { + *loaded_path = *absolute_path; + } return handle; #endif } @@ -103,15 +121,40 @@ inline Expected symbolOwner(void* symbol) { #endif } -/// Verify that @p symbol is defined by @p candidate_path, not a dependency. +/// Verify that @p symbol is defined by @p candidate_handle/path, not a +/// dependency. POSIX accepts an exact recorded loader-path match without any +/// filesystem access, then uses equivalent() only for different spellings. inline Expected verifySymbolProvenance( - void* symbol, const char* symbol_name, const std::filesystem::path& candidate_path) { + void* candidate_handle, void* symbol, const char* symbol_name, const std::filesystem::path& candidate_path) { #if defined(_WIN32) - (void)symbol; - (void)symbol_name; - (void)candidate_path; + HMODULE owner = nullptr; + if (symbol == nullptr || GetModuleHandleExW( + GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, + reinterpret_cast(symbol), &owner) == 0) { + return unexpected( + "cannot prove provenance for symbol '" + std::string(symbol_name) + "': GetModuleHandleExW failed (error " + + std::to_string(GetLastError()) + ")"); + } + const auto candidate = reinterpret_cast(candidate_handle); + if (owner != candidate) { + auto module_path = [](HMODULE module) { + std::wstring buffer(32768, L'\0'); + const DWORD length = GetModuleFileNameW(module, buffer.data(), static_cast(buffer.size())); + if (length == 0 || length >= static_cast(buffer.size())) { + return std::string(""); + } + buffer.resize(length); + return pathForLegacyAccessor(std::filesystem::path(buffer)); + }; + const std::string candidate_name = + candidate_path.empty() ? module_path(candidate) : pathForLegacyAccessor(candidate_path); + return unexpected( + "symbol '" + std::string(symbol_name) + "' resolved from dependency '" + module_path(owner) + + "', not candidate '" + candidate_name + "'"); + } return {}; #else + (void)candidate_handle; auto owner = symbolOwner(symbol); if (!owner) { return unexpected( @@ -119,6 +162,10 @@ inline Expected verifySymbolProvenance( candidate_path.string() + "': " + owner.error()); } + if (owner->native() == candidate_path.native()) { + return {}; + } + std::error_code equivalent_error; const bool equivalent = std::filesystem::equivalent(*owner, candidate_path, equivalent_error); if (equivalent_error) { @@ -163,7 +210,7 @@ inline Expected resolveSymbol( } void* resolved = symbol; #endif - if (auto provenance = verifySymbolProvenance(resolved, symbol_name, candidate_path); !provenance) { + if (auto provenance = verifySymbolProvenance(handle, resolved, symbol_name, candidate_path); !provenance) { return unexpected(provenance.error()); } return resolved; diff --git a/pj_plugins/src/detail/native_parser_module_loader.hpp b/pj_plugins/src/detail/native_parser_module_loader.hpp index a511fe58..d9441387 100644 --- a/pj_plugins/src/detail/native_parser_module_loader.hpp +++ b/pj_plugins/src/detail/native_parser_module_loader.hpp @@ -2,66 +2,52 @@ // Copyright 2026 Davide Faconti // SPDX-License-Identifier: Apache-2.0 +#include #include -#include #include #include +#include -#if defined(_WIN32) -#ifndef NOMINMAX -#define NOMINMAX -#endif -#ifndef WIN32_LEAN_AND_MEAN -#define WIN32_LEAN_AND_MEAN -#endif -#include -#else -#include -#endif - +#include "detail/library_loader.hpp" #include "pj_base/expected.hpp" namespace PJ::detail { using NativeModuleHandle = void*; -inline Expected openNativeParserModule(const std::filesystem::path& path) { +/// Open a narrow native parser-module path. Narrow parser-module paths are +/// UTF-8 by contract, including on Windows where filesystem::path(char*) would +/// otherwise interpret them using the active ANSI code page. +inline Expected openNativeParserModule( + std::string_view path, std::filesystem::path* loaded_path = nullptr) { #if defined(_WIN32) - HMODULE module = - LoadLibraryExW(path.c_str(), nullptr, LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); - if (module == nullptr) { - return unexpected("LoadLibraryExW failed (error " + std::to_string(GetLastError()) + ")"); + if (path.size() > static_cast(INT_MAX)) { + return unexpected("native parser-module path is too long"); } - return reinterpret_cast(module); -#else - void* handle = dlopen(path.c_str(), RTLD_LOCAL | RTLD_NOW); - if (handle == nullptr) { - const char* error = dlerror(); - return unexpected(error == nullptr ? "dlopen failed" : error); + const int required = + MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, path.data(), static_cast(path.size()), nullptr, 0); + if (required <= 0) { + return unexpected("native parser-module path is not valid UTF-8"); } - return handle; + std::wstring wide_path(static_cast(required), L'\0'); + if (MultiByteToWideChar( + CP_UTF8, MB_ERR_INVALID_CHARS, path.data(), static_cast(path.size()), wide_path.data(), required) == 0) { + return unexpected("native parser-module path conversion failed"); + } + return loadLibraryHandle(std::filesystem::path(wide_path), loaded_path); +#else + return loadLibraryHandle(std::filesystem::path(std::string(path)), loaded_path); #endif } -inline Expected resolveNativeParserModuleSymbol(NativeModuleHandle handle, const char* name) { +/// Resolve a required parser-module export and apply the same defining-module +/// provenance policy as the family plugin loaders. +inline Expected resolveNativeParserModuleSymbol( + NativeModuleHandle handle, const char* name, const std::filesystem::path& candidate_path) { if (handle == nullptr) { return unexpected("native parser module is not loaded"); } -#if defined(_WIN32) - FARPROC symbol = GetProcAddress(reinterpret_cast(handle), name); - if (symbol == nullptr) { - return unexpected(std::string(name) + " not found"); - } - return reinterpret_cast(symbol); -#else - dlerror(); - void* symbol = dlsym(handle, name); - const char* error = dlerror(); - if (error != nullptr) { - return unexpected(std::string(name) + " not found: " + error); - } - return symbol; -#endif + return resolveSymbol(handle, name, candidate_path); } } // namespace PJ::detail diff --git a/pj_plugins/src/message_parser_library.cpp b/pj_plugins/src/message_parser_library.cpp index e77b2895..c6ecfd48 100644 --- a/pj_plugins/src/message_parser_library.cpp +++ b/pj_plugins/src/message_parser_library.cpp @@ -45,19 +45,16 @@ MessageParserLibrary& MessageParserLibrary::operator=(MessageParserLibrary&& oth } Expected MessageParserLibrary::load(std::string_view path) { - auto library = load(std::filesystem::path(path)); - if (library) { - library->path_ = std::string(path); - } - return library; + return load(std::filesystem::path(path)); } Expected MessageParserLibrary::load(const std::filesystem::path& path) { - auto raw_handle = detail::loadLibraryHandle(path); + std::filesystem::path loaded_path; + auto raw_handle = detail::loadLibraryHandle(path, &loaded_path); if (!raw_handle) { return unexpected(raw_handle.error()); } - return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), path); + return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), loaded_path); } Expected MessageParserLibrary::loadFromHandle( @@ -65,11 +62,15 @@ Expected MessageParserLibrary::loadFromHandle( if (handle == nullptr) { return unexpected("library not loaded"); } - if (auto abi = detail::checkPluginAbiVersion(handle.get(), origin); !abi) { + auto loaded_path = detail::normalizedAbsoluteLibraryPath(origin); + if (!loaded_path) { + return unexpected(loaded_path.error()); + } + if (auto abi = detail::checkPluginAbiVersion(handle.get(), *loaded_path); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_message_parser_vtable", origin); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_message_parser_vtable", *loaded_path); if (!sym) { return unexpected(sym.error()); } @@ -89,7 +90,7 @@ Expected MessageParserLibrary::loadFromHandle( return unexpected(status.error()); } - return MessageParserLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(origin)); + return MessageParserLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(*loaded_path)); } Expected MessageParserLibrary::loadStatic( diff --git a/pj_plugins/src/native_parser_module.cpp b/pj_plugins/src/native_parser_module.cpp index 7ff75ab4..2bcba3f3 100644 --- a/pj_plugins/src/native_parser_module.cpp +++ b/pj_plugins/src/native_parser_module.cpp @@ -47,8 +47,9 @@ Expected rejectLoad( } template -Expected resolve(detail::NativeModuleHandle handle, const char* name) { - auto symbol = detail::resolveNativeParserModuleSymbol(handle, name); +Expected resolve( + detail::NativeModuleHandle handle, const char* name, const std::filesystem::path& candidate_path) { + auto symbol = detail::resolveNativeParserModuleSymbol(handle, name, candidate_path); if (!symbol) { return unexpected(symbol.error()); } @@ -62,7 +63,8 @@ NativeParserModule::NativeParserModule(std::shared_ptr NativeParserModule::load( std::string_view path, DiagnosticSink sink, std::string diagnostic_source) { - auto handle_result = detail::openNativeParserModule(std::filesystem::path(path)); + std::filesystem::path loaded_path; + auto handle_result = detail::openNativeParserModule(path, &loaded_path); if (!handle_result) { return rejectLoad(path, sink, diagnostic_source, "failed to open native parser module: " + handle_result.error()); } @@ -75,7 +77,7 @@ Expected NativeParserModule::load( #define PJ_RESOLVE_MODULE_EXPORT(member, type, name) \ do { \ - auto resolved = resolve(handle, name); \ + auto resolved = resolve(handle, name, loaded_path); \ if (!resolved) { \ return rejectLoad(path, sink, diagnostic_source, resolved.error()); \ } \ diff --git a/pj_plugins/src/plugin_catalog.cpp b/pj_plugins/src/plugin_catalog.cpp index e5033ac2..81a7e394 100644 --- a/pj_plugins/src/plugin_catalog.cpp +++ b/pj_plugins/src/plugin_catalog.cpp @@ -307,11 +307,12 @@ Expected inspectPluginDso(const std::filesystem::path& dso_pat return unexpected(fmt::format("not a platform plugin DSO: {}", dso_path.string())); } - auto raw_handle = detail::loadLibraryHandle(dso_path); + std::filesystem::path loaded_path; + auto raw_handle = detail::loadLibraryHandle(dso_path, &loaded_path); if (!raw_handle) { return unexpected(fmt::format("{}: {}", dso_path.string(), raw_handle.error())); } - return inspectPluginDso(detail::adoptLibraryHandle(*raw_handle), dso_path); + return inspectPluginDso(detail::adoptLibraryHandle(*raw_handle), loaded_path); } Expected inspectPluginDso( @@ -324,11 +325,16 @@ Expected inspectPluginDso( return unexpected(with_path("library not loaded")); } - if (auto abi = detail::checkPluginAbiVersion(handle.get(), dso_path); !abi) { + auto loaded_path = detail::normalizedAbsoluteLibraryPath(dso_path); + if (!loaded_path) { + return unexpected(with_path(loaded_path.error())); + } + + if (auto abi = detail::checkPluginAbiVersion(handle.get(), *loaded_path); !abi) { return unexpected(with_path(abi.error())); } - auto candidate = findEmbeddedManifest(handle.get(), dso_path); + auto candidate = findEmbeddedManifest(handle.get(), *loaded_path); if (!candidate) { return unexpected(with_path(candidate.error())); } @@ -348,9 +354,13 @@ std::vector exportedPluginFamilies( if (handle == nullptr) { return families; } + auto loaded_path = normalizedAbsoluteLibraryPath(dso_path); + if (!loaded_path) { + return families; + } auto append_if_owned = [&](const char* symbol, PluginFamily family) { - if (resolveSymbol(handle.get(), symbol, dso_path)) { + if (resolveSymbol(handle.get(), symbol, *loaded_path)) { families.push_back(family); } }; diff --git a/pj_plugins/src/toolbox_library.cpp b/pj_plugins/src/toolbox_library.cpp index c07c35ba..4ef4edcd 100644 --- a/pj_plugins/src/toolbox_library.cpp +++ b/pj_plugins/src/toolbox_library.cpp @@ -45,19 +45,16 @@ ToolboxLibrary& ToolboxLibrary::operator=(ToolboxLibrary&& other) noexcept { } Expected ToolboxLibrary::load(std::string_view path) { - auto library = load(std::filesystem::path(path)); - if (library) { - library->path_ = std::string(path); - } - return library; + return load(std::filesystem::path(path)); } Expected ToolboxLibrary::load(const std::filesystem::path& path) { - auto raw_handle = detail::loadLibraryHandle(path); + std::filesystem::path loaded_path; + auto raw_handle = detail::loadLibraryHandle(path, &loaded_path); if (!raw_handle) { return unexpected(raw_handle.error()); } - return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), path); + return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), loaded_path); } Expected ToolboxLibrary::loadFromHandle( @@ -65,11 +62,15 @@ Expected ToolboxLibrary::loadFromHandle( if (handle == nullptr) { return unexpected("library not loaded"); } - if (auto abi = detail::checkPluginAbiVersion(handle.get(), origin); !abi) { + auto loaded_path = detail::normalizedAbsoluteLibraryPath(origin); + if (!loaded_path) { + return unexpected(loaded_path.error()); + } + if (auto abi = detail::checkPluginAbiVersion(handle.get(), *loaded_path); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_toolbox_vtable", origin); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_toolbox_vtable", *loaded_path); if (!sym) { return unexpected(sym.error()); } @@ -89,7 +90,7 @@ Expected ToolboxLibrary::loadFromHandle( return unexpected(status.error()); } - return ToolboxLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(origin)); + return ToolboxLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(*loaded_path)); } Expected ToolboxLibrary::loadStatic( diff --git a/pj_plugins/tests/entry_point_forwarder.cpp b/pj_plugins/tests/entry_point_forwarder.cpp new file mode 100644 index 00000000..a19c1084 --- /dev/null +++ b/pj_plugins/tests/entry_point_forwarder.cpp @@ -0,0 +1,4 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include "pj_base/plugin_abi_export.hpp" diff --git a/pj_plugins/tests/entry_point_forwarder.def b/pj_plugins/tests/entry_point_forwarder.def new file mode 100644 index 00000000..52f49a04 --- /dev/null +++ b/pj_plugins/tests/entry_point_forwarder.def @@ -0,0 +1,3 @@ +LIBRARY entry_point_forwarder_plugin +EXPORTS + PJ_get_data_source_vtable=entry_point_donor.PJ_get_data_source_vtable diff --git a/pj_plugins/tests/native_parser_module_test.cpp b/pj_plugins/tests/native_parser_module_test.cpp index ad8bdb92..8604f8be 100644 --- a/pj_plugins/tests/native_parser_module_test.cpp +++ b/pj_plugins/tests/native_parser_module_test.cpp @@ -5,6 +5,9 @@ #include +#include +#include +#include #include #include @@ -15,6 +18,42 @@ namespace PJ { namespace { +class NativeModuleTemporaryDirectory { + public: + NativeModuleTemporaryDirectory() + : path_( + std::filesystem::temp_directory_path() / + ("pj_native_module_" + + std::to_string(static_cast(std::chrono::steady_clock::now().time_since_epoch().count())))) { + std::filesystem::create_directories(path_); + } + + ~NativeModuleTemporaryDirectory() { + std::error_code error; + std::filesystem::remove_all(path_, error); + } + + const std::filesystem::path& path() const noexcept { + return path_; + } + + private: + std::filesystem::path path_; +}; + +class NativeModuleCurrentPathGuard { + public: + NativeModuleCurrentPathGuard() : original_(std::filesystem::current_path()) {} + + ~NativeModuleCurrentPathGuard() { + std::error_code error; + std::filesystem::current_path(original_, error); + } + + private: + std::filesystem::path original_; +}; + TEST(NativeParserModule, LoadsCompleteAbiAndCopiesManifestForCatalogAdmission) { std::vector diagnostics; auto module = NativeParserModule::load( @@ -34,6 +73,31 @@ TEST(NativeParserModule, LoadsCompleteAbiAndCopiesManifestForCatalogAdmission) { EXPECT_EQ(catalog.claims().size(), pj_fixture::kClaimCount); } +TEST(NativeParserModule, NativeParserNarrowPathIsUtf8) { + NativeModuleTemporaryDirectory temporary; + const std::filesystem::path unicode_directory = temporary.path() / std::filesystem::path(u8"módulo-解析"); + std::filesystem::create_directories(unicode_directory); + const std::filesystem::path module_path = + unicode_directory / std::filesystem::path(PJ_NATIVE_MODULE_FIXTURE_PATH).filename(); + std::filesystem::copy_file(PJ_NATIVE_MODULE_FIXTURE_PATH, module_path); + + NativeModuleCurrentPathGuard current_path; + std::filesystem::current_path(temporary.path()); + const auto utf8_path = module_path.lexically_relative(temporary.path()).u8string(); + const std::string narrow_path(utf8_path.begin(), utf8_path.end()); + auto module = NativeParserModule::load(narrow_path); + ASSERT_TRUE(module.has_value()) << module.error(); + EXPECT_EQ(module->path(), narrow_path); + EXPECT_NE(module->manifestJson().find("org.plotjuggler.test.native-module"), std::string_view::npos); + +#if defined(_WIN32) + const std::string invalid_utf8 = "invalid-\xff.dll"; + auto invalid = NativeParserModule::load(invalid_utf8); + ASSERT_FALSE(invalid.has_value()); + EXPECT_NE(invalid.error().find("valid UTF-8"), std::string::npos) << invalid.error(); +#endif +} + TEST(NativeParserModule, RejectsEachLoaderFailureWithOneDiagnostic) { for (const std::string path : { PJ_NATIVE_MODULE_MISSING_EXPORT_PATH, diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index d3ea8daf..a9e80918 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -158,6 +158,18 @@ TEST_F(PluginCatalogTest, EntryPointSymbolResolvedFromDependencyIsRejected) { EXPECT_EQ(descriptor->id, "entry-point-candidate"); } +#if defined(_WIN32) +TEST_F(PluginCatalogTest, ForwardedEntryPointIsRejected) { + auto library = DataSourceLibrary::load(PJ_ENTRY_POINT_FORWARDER_PLUGIN_PATH); + ASSERT_FALSE(library.has_value()); + EXPECT_NE(library.error().find("resolved from dependency"), std::string::npos) << library.error(); + + auto descriptor = inspectPluginDso(PJ_ENTRY_POINT_FORWARDER_PLUGIN_PATH); + ASSERT_FALSE(descriptor.has_value()); + EXPECT_NE(descriptor.error().find("resolved from dependency"), std::string::npos) << descriptor.error(); +} +#endif + TEST_F(PluginCatalogTest, UnicodeExtensionPathLoadsOnWindows) { const std::filesystem::path unicode_dir = dir_ / std::filesystem::path(u8"插件-π"); std::filesystem::create_directories(unicode_dir); diff --git a/pj_plugins/tests/source_dialog_integration_test.cpp b/pj_plugins/tests/source_dialog_integration_test.cpp index 6ecc714c..048b893b 100644 --- a/pj_plugins/tests/source_dialog_integration_test.cpp +++ b/pj_plugins/tests/source_dialog_integration_test.cpp @@ -3,6 +3,9 @@ #include +#include +#include +#include #include #include #include @@ -21,6 +24,46 @@ namespace { +class TemporaryDirectory { + public: + TemporaryDirectory() + : path_( + std::filesystem::temp_directory_path() / + ("pj_dialog_loader_" + + std::to_string(static_cast(std::chrono::steady_clock::now().time_since_epoch().count())))) { + std::filesystem::create_directories(path_); + } + + ~TemporaryDirectory() { + std::error_code error; + std::filesystem::remove_all(path_, error); + } + + const std::filesystem::path& path() const noexcept { + return path_; + } + + private: + std::filesystem::path path_; +}; + +class CurrentPathGuard { + public: + CurrentPathGuard() : original_(std::filesystem::current_path()) {} + + ~CurrentPathGuard() { + std::error_code error; + std::filesystem::current_path(original_, error); + } + + const std::filesystem::path& original() const noexcept { + return original_; + } + + private: + std::filesystem::path original_; +}; + // --- Test 1: Load combined .so --- TEST(SourceDialogIntegration, LoadCombinedPlugin) { @@ -51,6 +94,38 @@ TEST(SourceDialogIntegration, ResolveDialogVtable) { EXPECT_EQ((*dialog_vt)->protocol_version, PJ_DIALOG_PROTOCOL_VERSION); } +TEST(SourceDialogIntegration, DialogVtableSurvivesCandidateFileDeletion) { + TemporaryDirectory temporary; + const std::filesystem::path candidate = + temporary.path() / std::filesystem::path(PJ_MOCK_SOURCE_WITH_DIALOG_PLUGIN_PATH).filename(); + std::filesystem::copy_file(PJ_MOCK_SOURCE_WITH_DIALOG_PLUGIN_PATH, candidate); + + auto lib = PJ::DataSourceLibrary::load(candidate); + ASSERT_TRUE(lib) << lib.error(); + ASSERT_TRUE(std::filesystem::remove(candidate)); + + auto dialog_vtable = lib->resolveDialogVtable(); + ASSERT_TRUE(dialog_vtable) << dialog_vtable.error(); + EXPECT_EQ((*dialog_vtable)->protocol_version, PJ_DIALOG_PROTOCOL_VERSION); +} + +TEST(SourceDialogIntegration, DialogVtableSurvivesCwdChangeAfterRelativeLoad) { + TemporaryDirectory temporary; + const std::filesystem::path candidate = + temporary.path() / std::filesystem::path(PJ_MOCK_SOURCE_WITH_DIALOG_PLUGIN_PATH).filename(); + std::filesystem::copy_file(PJ_MOCK_SOURCE_WITH_DIALOG_PLUGIN_PATH, candidate); + + CurrentPathGuard current_path; + std::filesystem::current_path(temporary.path()); + auto lib = PJ::DataSourceLibrary::load(candidate.filename()); + std::filesystem::current_path(current_path.original()); + + ASSERT_TRUE(lib) << lib.error(); + auto dialog_vtable = lib->resolveDialogVtable(); + ASSERT_TRUE(dialog_vtable) << dialog_vtable.error(); + EXPECT_EQ((*dialog_vtable)->protocol_version, PJ_DIALOG_PROTOCOL_VERSION); +} + // --- Test 4: Borrowed dialog context --- TEST(SourceDialogIntegration, BorrowedDialogContext) { From 350b5527c5a8643762af1609efcc6fd7b3c6afa0 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Tue, 11 Aug 2026 09:22:17 +0200 Subject: [PATCH 3/5] fix(pj_plugins): CI round - PE forwarder .def syntax + dual-form recorded provenance paths The forwarder fixture uses true forwarder syntax against the donor DLL basename; load-time identity records both the exact absolute loader argument and its weakly_canonical form so dyld's realpath dli_fname byte-matches without live filesystem access (deletion test now also loads through a symlinked directory to reproduce the macOS shape on Linux). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0165qLfnJYSYb51NQMdPidZr --- CHANGELOG.md | 11 ++-- pj_plugins/CMakeLists.txt | 15 +++++ .../dialog_protocol/src/dialog_library.cpp | 18 ++--- pj_plugins/docs/ARCHITECTURE.md | 19 +++--- .../pj_plugins/host/data_source_library.hpp | 10 ++- .../host/message_parser_library.hpp | 10 ++- .../pj_plugins/host/toolbox_library.hpp | 10 ++- pj_plugins/src/data_source_library.cpp | 42 +++++++----- pj_plugins/src/detail/library_loader.hpp | 65 +++++++++++++------ .../detail/native_parser_module_loader.hpp | 8 +-- pj_plugins/src/message_parser_library.cpp | 42 +++++++----- pj_plugins/src/native_parser_module.cpp | 8 +-- pj_plugins/src/plugin_catalog.cpp | 34 +++++----- pj_plugins/src/toolbox_library.cpp | 42 +++++++----- pj_plugins/tests/entry_point_forwarder.def | 2 +- .../tests/source_dialog_integration_test.cpp | 45 ++++++++++--- 16 files changed, 255 insertions(+), 126 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c025c5a0..b9710af4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,11 +12,12 @@ defined by the candidate DSO itself instead of accepting definitions from a dependency. POSIX uses defining-object identity, macOS restricts handle-scoped lookups to the first image, and Windows uses filesystem-native wide paths with package-scoped dependency search and defining-module checks that reject -forwarded PE exports. Recorded normalized absolute load paths let deferred -dialog-vtable provenance accept exact defining-path matches without re-stating -the candidate, so staged deletion and later working-directory changes are -safe. Plugin install rpaths now resolve bundled dependencies relative to the -plugin on Linux and macOS. +forwarded PE exports. Recorded normalized absolute load paths and their +best-effort symlink-resolved forms let deferred dialog-vtable provenance accept +either defining-path spelling without re-stating the candidate, so staged +deletion, later working-directory changes, and macOS dyld realpath reporting +are safe. Plugin install rpaths now resolve bundled dependencies relative to +the plugin on Linux and macOS. Native functional parser modules now share the same absolute-path open and symbol-provenance checks, including `RTLD_FIRST` on macOS. Their narrow load diff --git a/pj_plugins/CMakeLists.txt b/pj_plugins/CMakeLists.txt index 01b42cca..1510855f 100644 --- a/pj_plugins/CMakeLists.txt +++ b/pj_plugins/CMakeLists.txt @@ -350,6 +350,9 @@ add_library(entry_point_donor SHARED tests/entry_point_donor.cpp) target_compile_features(entry_point_donor PRIVATE cxx_std_20) target_compile_options(entry_point_donor PRIVATE ${PJ_WARNING_FLAGS}) target_link_libraries(entry_point_donor PRIVATE pj_base) +# The PE forwarder string below names entry_point_donor.dll at runtime. Pin the +# output basename so it cannot drift from the module token in the .def file. +set_target_properties(entry_point_donor PROPERTIES OUTPUT_NAME entry_point_donor) add_library(entry_point_via_dependency_plugin SHARED tests/entry_point_via_dependency.cpp) target_compile_features(entry_point_via_dependency_plugin PRIVATE cxx_std_20) @@ -368,8 +371,19 @@ if(WIN32) ) target_compile_features(entry_point_forwarder_plugin PRIVATE cxx_std_20) target_compile_options(entry_point_forwarder_plugin PRIVATE ${PJ_WARNING_FLAGS}) + # Deliberately do not link entry_point_donor: the .def entry is a PE forwarder, + # not an import that the linker should resolve. Both targets retain CMake's + # common runtime output directory, so entry_point_donor.dll is available when + # GetProcAddress follows the forwarder. target_link_libraries(entry_point_forwarder_plugin PRIVATE pj_base) add_dependencies(entry_point_forwarder_plugin entry_point_donor) + set_target_properties( + entry_point_donor + entry_point_via_dependency_plugin + entry_point_with_own_exports_plugin + entry_point_forwarder_plugin + PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/entry_point_fixtures" + ) endif() # Dependency-search fixtures. The real and decoy dependencies intentionally @@ -430,6 +444,7 @@ target_compile_definitions(source_dialog_integration_test PRIVATE PJ_MOCK_DATA_SOURCE_PLUGIN_PATH="$" ) target_compile_options(source_dialog_integration_test PRIVATE ${PJ_WARNING_FLAGS}) +target_include_directories(source_dialog_integration_test PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src) target_link_libraries(source_dialog_integration_test PRIVATE pj_data_source_host pj_dialog_host pj_base GTest::gtest_main ) diff --git a/pj_plugins/dialog_protocol/src/dialog_library.cpp b/pj_plugins/dialog_protocol/src/dialog_library.cpp index b7907c53..cf857598 100644 --- a/pj_plugins/dialog_protocol/src/dialog_library.cpp +++ b/pj_plugins/dialog_protocol/src/dialog_library.cpp @@ -38,12 +38,12 @@ Expected DialogLibrary::load(std::string_view path) { } Expected DialogLibrary::load(const std::filesystem::path& path) { - std::filesystem::path loaded_path; - auto raw_handle = detail::loadLibraryHandle(path, &loaded_path); + detail::LibraryPathIdentity recorded_path; + auto raw_handle = detail::loadLibraryHandle(path, &recorded_path); if (!raw_handle) { return unexpected(raw_handle.error()); } - return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), loaded_path); + return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), recorded_path.load_path); } Expected DialogLibrary::loadFromHandle( @@ -51,15 +51,15 @@ Expected DialogLibrary::loadFromHandle( if (handle == nullptr) { return unexpected("library not loaded"); } - auto loaded_path = detail::normalizedAbsoluteLibraryPath(origin); - if (!loaded_path) { - return unexpected(loaded_path.error()); + auto recorded_path = detail::recordLibraryPathIdentity(origin); + if (!recorded_path) { + return unexpected(recorded_path.error()); } - if (auto abi = detail::checkPluginAbiVersion(handle.get(), *loaded_path); !abi) { + if (auto abi = detail::checkPluginAbiVersion(handle.get(), *recorded_path); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_dialog_vtable", *loaded_path); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_dialog_vtable", *recorded_path); if (!sym) { return unexpected(sym.error()); } @@ -79,7 +79,7 @@ Expected DialogLibrary::loadFromHandle( return unexpected(status.error()); } - return DialogLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(*loaded_path)); + return DialogLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(recorded_path->load_path)); } void DialogLibrary::reset() { diff --git a/pj_plugins/docs/ARCHITECTURE.md b/pj_plugins/docs/ARCHITECTURE.md index 0b780973..696f56f0 100644 --- a/pj_plugins/docs/ARCHITECTURE.md +++ b/pj_plugins/docs/ARCHITECTURE.md @@ -453,16 +453,17 @@ These live in `sdk/detail/*_trampolines.hpp`. Each family has a loader that: 1. Lexically normalizes the candidate to an absolute filesystem path, passes - that exact path to `dlopen` (or `LoadLibraryExW` on Windows), and records it - in the library object for later symbol resolution. + that exact path to `dlopen` (or `LoadLibraryExW` on Windows), and records + both that spelling and its best-effort `weakly_canonical()` spelling in the + library object for later symbol resolution. 2. Resolves the ABI marker and entry point, then verifies that each symbol's defining object is the candidate DSO itself rather than a dependency. On - POSIX, an exact byte match between `dladdr().dli_fname` and the recorded load + POSIX, an exact byte match between `dladdr().dli_fname` and either recorded path succeeds without re-reading the filesystem; `equivalent()` is only the - fallback for different path spellings. On Windows, the defining `HMODULE` - is recovered from the resolved address with `GetModuleHandleExW(... - FROM_ADDRESS ...)` and compared to the candidate handle, which also rejects - forwarded PE exports. + fallback for genuinely different path spellings. On Windows, the defining + `HMODULE` is recovered from the resolved address with + `GetModuleHandleExW(... FROM_ADDRESS ...)` and compared to the candidate + handle, which also rejects forwarded PE exports. 3. Validates `protocol_version` and `struct_size`. 4. Stores the vtable pointer for creating handles. @@ -476,8 +477,8 @@ Each family has a loader that: Loaders also provide `resolveDialogVtable()` to find the dialog vtable in a plugin `.so` that exports both a family vtable and a dialog vtable (e.g. a DataSource with an embedded dialog). These deferred lookups use the recorded -load path, so they remain valid after the candidate file is removed or the -process working directory changes. +load-time paths, so they remain valid after the candidate file is removed, the +process working directory changes, or dyld reports a symlink-resolved filename. Native functional parser modules use the same absolute-path normalization, package-scoped platform open, and defining-module provenance checks for every diff --git a/pj_plugins/include/pj_plugins/host/data_source_library.hpp b/pj_plugins/include/pj_plugins/host/data_source_library.hpp index 88d559b0..817ebf3c 100644 --- a/pj_plugins/include/pj_plugins/host/data_source_library.hpp +++ b/pj_plugins/include/pj_plugins/host/data_source_library.hpp @@ -31,6 +31,10 @@ namespace PJ { +namespace detail { +struct LibraryPathIdentity; +} + /** * Loads a DataSource plugin shared library and provides factory access. * @@ -100,8 +104,11 @@ class DataSourceLibrary { } private: + [[nodiscard]] static Expected loadFromHandleWithIdentity( + std::shared_ptr handle, const detail::LibraryPathIdentity& origin); + DataSourceLibrary( - std::shared_ptr handle, const PJ_data_source_vtable_t* vtable, std::string path, + std::shared_ptr handle, const PJ_data_source_vtable_t* vtable, std::string path, std::string resolved_path, const PJ_dialog_vtable_t* static_dialog_vtable = nullptr); void reset(); @@ -110,6 +117,7 @@ class DataSourceLibrary { const PJ_data_source_vtable_t* vtable_ = nullptr; const PJ_dialog_vtable_t* static_dialog_vtable_ = nullptr; std::string path_; + std::string resolved_path_; }; } // namespace PJ diff --git a/pj_plugins/include/pj_plugins/host/message_parser_library.hpp b/pj_plugins/include/pj_plugins/host/message_parser_library.hpp index 1398511f..c20e15d3 100644 --- a/pj_plugins/include/pj_plugins/host/message_parser_library.hpp +++ b/pj_plugins/include/pj_plugins/host/message_parser_library.hpp @@ -31,6 +31,10 @@ namespace PJ { +namespace detail { +struct LibraryPathIdentity; +} + /** * Loads a MessageParser plugin shared library and provides factory access. * @@ -100,9 +104,12 @@ class MessageParserLibrary { } private: + [[nodiscard]] static Expected loadFromHandleWithIdentity( + std::shared_ptr handle, const detail::LibraryPathIdentity& origin); + MessageParserLibrary( std::shared_ptr handle, const PJ_message_parser_vtable_t* vtable, std::string path, - const PJ_dialog_vtable_t* static_dialog_vtable = nullptr); + std::string resolved_path, const PJ_dialog_vtable_t* static_dialog_vtable = nullptr); void reset(); @@ -110,6 +117,7 @@ class MessageParserLibrary { const PJ_message_parser_vtable_t* vtable_ = nullptr; const PJ_dialog_vtable_t* static_dialog_vtable_ = nullptr; std::string path_; + std::string resolved_path_; }; } // namespace PJ diff --git a/pj_plugins/include/pj_plugins/host/toolbox_library.hpp b/pj_plugins/include/pj_plugins/host/toolbox_library.hpp index 4c2d494e..a850c19f 100644 --- a/pj_plugins/include/pj_plugins/host/toolbox_library.hpp +++ b/pj_plugins/include/pj_plugins/host/toolbox_library.hpp @@ -31,6 +31,10 @@ namespace PJ { +namespace detail { +struct LibraryPathIdentity; +} + /** * Loads a Toolbox plugin shared library and provides factory access. * @@ -100,8 +104,11 @@ class ToolboxLibrary { } private: + [[nodiscard]] static Expected loadFromHandleWithIdentity( + std::shared_ptr handle, const detail::LibraryPathIdentity& origin); + ToolboxLibrary( - std::shared_ptr handle, const PJ_toolbox_vtable_t* vtable, std::string path, + std::shared_ptr handle, const PJ_toolbox_vtable_t* vtable, std::string path, std::string resolved_path, const PJ_dialog_vtable_t* static_dialog_vtable = nullptr); void reset(); @@ -110,6 +117,7 @@ class ToolboxLibrary { const PJ_toolbox_vtable_t* vtable_ = nullptr; const PJ_dialog_vtable_t* static_dialog_vtable_ = nullptr; std::string path_; + std::string resolved_path_; }; } // namespace PJ diff --git a/pj_plugins/src/data_source_library.cpp b/pj_plugins/src/data_source_library.cpp index deadcd8f..d643afad 100644 --- a/pj_plugins/src/data_source_library.cpp +++ b/pj_plugins/src/data_source_library.cpp @@ -11,12 +11,13 @@ namespace PJ { DataSourceLibrary::DataSourceLibrary( - std::shared_ptr handle, const PJ_data_source_vtable_t* vtable, std::string path, + std::shared_ptr handle, const PJ_data_source_vtable_t* vtable, std::string path, std::string resolved_path, const PJ_dialog_vtable_t* static_dialog_vtable) : handle_(std::move(handle)), vtable_(vtable), static_dialog_vtable_(static_dialog_vtable), - path_(std::move(path)) {} + path_(std::move(path)), + resolved_path_(std::move(resolved_path)) {} DataSourceLibrary::~DataSourceLibrary() { reset(); @@ -26,7 +27,8 @@ DataSourceLibrary::DataSourceLibrary(DataSourceLibrary&& other) noexcept : handle_(std::move(other.handle_)), vtable_(other.vtable_), static_dialog_vtable_(other.static_dialog_vtable_), - path_(std::move(other.path_)) { + path_(std::move(other.path_)), + resolved_path_(std::move(other.resolved_path_)) { other.vtable_ = nullptr; other.static_dialog_vtable_ = nullptr; } @@ -38,6 +40,7 @@ DataSourceLibrary& DataSourceLibrary::operator=(DataSourceLibrary&& other) noexc vtable_ = other.vtable_; static_dialog_vtable_ = other.static_dialog_vtable_; path_ = std::move(other.path_); + resolved_path_ = std::move(other.resolved_path_); other.vtable_ = nullptr; other.static_dialog_vtable_ = nullptr; } @@ -49,28 +52,33 @@ Expected DataSourceLibrary::load(std::string_view path) { } Expected DataSourceLibrary::load(const std::filesystem::path& path) { - std::filesystem::path loaded_path; - auto raw_handle = detail::loadLibraryHandle(path, &loaded_path); + detail::LibraryPathIdentity recorded_path; + auto raw_handle = detail::loadLibraryHandle(path, &recorded_path); if (!raw_handle) { return unexpected(raw_handle.error()); } - return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), loaded_path); + return loadFromHandleWithIdentity(detail::adoptLibraryHandle(*raw_handle), recorded_path); } Expected DataSourceLibrary::loadFromHandle( std::shared_ptr handle, const std::filesystem::path& origin) { + auto recorded_path = detail::recordLibraryPathIdentity(origin); + if (!recorded_path) { + return unexpected(recorded_path.error()); + } + return loadFromHandleWithIdentity(std::move(handle), *recorded_path); +} + +Expected DataSourceLibrary::loadFromHandleWithIdentity( + std::shared_ptr handle, const detail::LibraryPathIdentity& recorded_path) { if (handle == nullptr) { return unexpected("library not loaded"); } - auto loaded_path = detail::normalizedAbsoluteLibraryPath(origin); - if (!loaded_path) { - return unexpected(loaded_path.error()); - } - if (auto abi = detail::checkPluginAbiVersion(handle.get(), *loaded_path); !abi) { + if (auto abi = detail::checkPluginAbiVersion(handle.get(), recorded_path); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_data_source_vtable", *loaded_path); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_data_source_vtable", recorded_path); if (!sym) { return unexpected(sym.error()); } @@ -92,7 +100,9 @@ Expected DataSourceLibrary::loadFromHandle( return unexpected(status.error()); } - return DataSourceLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(*loaded_path)); + return DataSourceLibrary( + std::move(handle), vtable, detail::pathForLegacyAccessor(recorded_path.load_path), + detail::pathForLegacyAccessor(recorded_path.resolved_path)); } Expected DataSourceLibrary::loadStatic( @@ -124,7 +134,7 @@ Expected DataSourceLibrary::loadStatic( // non-null owner. Use a sentinel shared_ptr with a no-op deleter. static char anchor = 0; std::shared_ptr handle(&anchor, [](void*) {}); - return DataSourceLibrary(std::move(handle), vtable, "static://", dialog_vtable); + return DataSourceLibrary(std::move(handle), vtable, "static://", "", dialog_vtable); } Expected DataSourceLibrary::resolveDialogVtable() const { @@ -137,7 +147,8 @@ Expected DataSourceLibrary::resolveDialogVtable() con #if defined(_WIN32) auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", {}); #else - auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", std::filesystem::path(path_)); + auto sym = detail::resolveSymbol( + handle_.get(), "PJ_get_dialog_vtable", {std::filesystem::path(path_), std::filesystem::path(resolved_path_)}); #endif if (!sym) { return unexpected(sym.error()); @@ -165,6 +176,7 @@ void DataSourceLibrary::reset() { vtable_ = nullptr; static_dialog_vtable_ = nullptr; path_.clear(); + resolved_path_.clear(); } } diff --git a/pj_plugins/src/detail/library_loader.hpp b/pj_plugins/src/detail/library_loader.hpp index bfce9b16..b458cb4f 100644 --- a/pj_plugins/src/detail/library_loader.hpp +++ b/pj_plugins/src/detail/library_loader.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #if defined(_WIN32) #ifndef NOMINMAX @@ -35,6 +36,14 @@ inline std::string pathForLegacyAccessor(const std::filesystem::path& path) { #endif } +/// The two immutable path spellings captured while a candidate still exists. +/// `load_path` is the exact normalized absolute argument passed to the native +/// loader. `resolved_path` is its best-effort weakly-canonical spelling. +struct LibraryPathIdentity { + std::filesystem::path load_path; + std::filesystem::path resolved_path; +}; + /// Produce the normalized absolute spelling used for the native loader call. /// This is deliberately lexical: loading does not require the candidate to /// remain stat-able after the native module handle has been acquired. @@ -51,20 +60,34 @@ inline Expected normalizedAbsoluteLibraryPath(const std:: return absolute_path.lexically_normal(); } +inline Expected recordLibraryPathIdentity(const std::filesystem::path& path) { + auto load_path = normalizedAbsoluteLibraryPath(path); + if (!load_path) { + return unexpected(load_path.error()); + } + + std::error_code canonical_error; + std::filesystem::path resolved_path = std::filesystem::weakly_canonical(*load_path, canonical_error); + if (canonical_error) { + resolved_path.clear(); + } + return LibraryPathIdentity{std::move(*load_path), std::move(resolved_path)}; +} + inline Expected loadLibraryHandle( - const std::filesystem::path& path, std::filesystem::path* loaded_path = nullptr) { - auto absolute_path = normalizedAbsoluteLibraryPath(path); - if (!absolute_path) { - return unexpected(absolute_path.error()); + const std::filesystem::path& path, LibraryPathIdentity* recorded_path = nullptr) { + auto identity = recordLibraryPathIdentity(path); + if (!identity) { + return unexpected(identity.error()); } #if defined(_WIN32) HMODULE module = LoadLibraryExW( - absolute_path->c_str(), nullptr, LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); + identity->load_path.c_str(), nullptr, LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS); if (module == nullptr) { return unexpected("LoadLibraryExW failed (error " + std::to_string(GetLastError()) + ")"); } - if (loaded_path != nullptr) { - *loaded_path = *absolute_path; + if (recorded_path != nullptr) { + *recorded_path = *identity; } return reinterpret_cast(module); #else @@ -95,13 +118,13 @@ inline Expected loadLibraryHandle( // admission behavior is intentional and provenance diagnostics stay explicit. flags |= RTLD_FIRST; #endif - void* handle = dlopen(absolute_path->c_str(), flags); + void* handle = dlopen(identity->load_path.c_str(), flags); if (handle == nullptr) { const char* error = dlerror(); return unexpected(error == nullptr ? "" : error); } - if (loaded_path != nullptr) { - *loaded_path = *absolute_path; + if (recorded_path != nullptr) { + *recorded_path = *identity; } return handle; #endif @@ -125,7 +148,7 @@ inline Expected symbolOwner(void* symbol) { /// dependency. POSIX accepts an exact recorded loader-path match without any /// filesystem access, then uses equivalent() only for different spellings. inline Expected verifySymbolProvenance( - void* candidate_handle, void* symbol, const char* symbol_name, const std::filesystem::path& candidate_path) { + void* candidate_handle, void* symbol, const char* symbol_name, const LibraryPathIdentity& candidate_path) { #if defined(_WIN32) HMODULE owner = nullptr; if (symbol == nullptr || GetModuleHandleExW( @@ -147,7 +170,7 @@ inline Expected verifySymbolProvenance( return pathForLegacyAccessor(std::filesystem::path(buffer)); }; const std::string candidate_name = - candidate_path.empty() ? module_path(candidate) : pathForLegacyAccessor(candidate_path); + candidate_path.load_path.empty() ? module_path(candidate) : pathForLegacyAccessor(candidate_path.load_path); return unexpected( "symbol '" + std::string(symbol_name) + "' resolved from dependency '" + module_path(owner) + "', not candidate '" + candidate_name + "'"); @@ -159,32 +182,32 @@ inline Expected verifySymbolProvenance( if (!owner) { return unexpected( "cannot prove provenance for symbol '" + std::string(symbol_name) + "' in candidate '" + - candidate_path.string() + "': " + owner.error()); + candidate_path.load_path.string() + "': " + owner.error()); } - if (owner->native() == candidate_path.native()) { + if (owner->native() == candidate_path.load_path.native() || + (!candidate_path.resolved_path.empty() && owner->native() == candidate_path.resolved_path.native())) { return {}; } std::error_code equivalent_error; - const bool equivalent = std::filesystem::equivalent(*owner, candidate_path, equivalent_error); + const bool equivalent = std::filesystem::equivalent(*owner, candidate_path.load_path, equivalent_error); if (equivalent_error) { return unexpected( "cannot prove provenance for symbol '" + std::string(symbol_name) + "': defining object '" + owner->string() + - "', candidate '" + candidate_path.string() + "': " + equivalent_error.message()); + "', candidate '" + candidate_path.load_path.string() + "': " + equivalent_error.message()); } if (!equivalent) { return unexpected( "symbol '" + std::string(symbol_name) + "' resolved from dependency '" + owner->string() + - "', not candidate '" + candidate_path.string() + "'"); + "', not candidate '" + candidate_path.load_path.string() + "'"); } return {}; #endif } /// Resolve a named symbol and prove that it is defined by @p candidate_path. -inline Expected resolveSymbol( - void* handle, const char* symbol_name, const std::filesystem::path& candidate_path) { +inline Expected resolveSymbol(void* handle, const char* symbol_name, const LibraryPathIdentity& candidate_path) { if (handle == nullptr) { return unexpected("library not loaded"); } @@ -203,7 +226,7 @@ inline Expected resolveSymbol( #if defined(__APPLE__) return unexpected( "cannot prove provenance for symbol '" + std::string(symbol_name) + "' in candidate '" + - candidate_path.string() + "': RTLD_FIRST lookup failed: " + err); + candidate_path.load_path.string() + "': RTLD_FIRST lookup failed: " + err); #else return unexpected(err); #endif @@ -219,7 +242,7 @@ inline Expected resolveSymbol( /// Verify the plugin exports `pj_plugin_abi_version` and its value equals /// PJ_ABI_VERSION. Must be called BEFORE the family vtable is fetched — the /// vtable layout is only meaningful once the boot-level ABI matches. -inline Expected checkPluginAbiVersion(void* handle, const std::filesystem::path& candidate_path) { +inline Expected checkPluginAbiVersion(void* handle, const LibraryPathIdentity& candidate_path) { auto sym = resolveSymbol(handle, "pj_plugin_abi_version", candidate_path); if (!sym) { return unexpected("plugin missing pj_plugin_abi_version symbol: " + sym.error()); diff --git a/pj_plugins/src/detail/native_parser_module_loader.hpp b/pj_plugins/src/detail/native_parser_module_loader.hpp index d9441387..e941b125 100644 --- a/pj_plugins/src/detail/native_parser_module_loader.hpp +++ b/pj_plugins/src/detail/native_parser_module_loader.hpp @@ -19,7 +19,7 @@ using NativeModuleHandle = void*; /// UTF-8 by contract, including on Windows where filesystem::path(char*) would /// otherwise interpret them using the active ANSI code page. inline Expected openNativeParserModule( - std::string_view path, std::filesystem::path* loaded_path = nullptr) { + std::string_view path, LibraryPathIdentity* recorded_path = nullptr) { #if defined(_WIN32) if (path.size() > static_cast(INT_MAX)) { return unexpected("native parser-module path is too long"); @@ -34,16 +34,16 @@ inline Expected openNativeParserModule( CP_UTF8, MB_ERR_INVALID_CHARS, path.data(), static_cast(path.size()), wide_path.data(), required) == 0) { return unexpected("native parser-module path conversion failed"); } - return loadLibraryHandle(std::filesystem::path(wide_path), loaded_path); + return loadLibraryHandle(std::filesystem::path(wide_path), recorded_path); #else - return loadLibraryHandle(std::filesystem::path(std::string(path)), loaded_path); + return loadLibraryHandle(std::filesystem::path(std::string(path)), recorded_path); #endif } /// Resolve a required parser-module export and apply the same defining-module /// provenance policy as the family plugin loaders. inline Expected resolveNativeParserModuleSymbol( - NativeModuleHandle handle, const char* name, const std::filesystem::path& candidate_path) { + NativeModuleHandle handle, const char* name, const LibraryPathIdentity& candidate_path) { if (handle == nullptr) { return unexpected("native parser module is not loaded"); } diff --git a/pj_plugins/src/message_parser_library.cpp b/pj_plugins/src/message_parser_library.cpp index c6ecfd48..b76d48e9 100644 --- a/pj_plugins/src/message_parser_library.cpp +++ b/pj_plugins/src/message_parser_library.cpp @@ -11,12 +11,13 @@ namespace PJ { MessageParserLibrary::MessageParserLibrary( - std::shared_ptr handle, const PJ_message_parser_vtable_t* vtable, std::string path, + std::shared_ptr handle, const PJ_message_parser_vtable_t* vtable, std::string path, std::string resolved_path, const PJ_dialog_vtable_t* static_dialog_vtable) : handle_(std::move(handle)), vtable_(vtable), static_dialog_vtable_(static_dialog_vtable), - path_(std::move(path)) {} + path_(std::move(path)), + resolved_path_(std::move(resolved_path)) {} MessageParserLibrary::~MessageParserLibrary() { reset(); @@ -26,7 +27,8 @@ MessageParserLibrary::MessageParserLibrary(MessageParserLibrary&& other) noexcep : handle_(std::move(other.handle_)), vtable_(other.vtable_), static_dialog_vtable_(other.static_dialog_vtable_), - path_(std::move(other.path_)) { + path_(std::move(other.path_)), + resolved_path_(std::move(other.resolved_path_)) { other.vtable_ = nullptr; other.static_dialog_vtable_ = nullptr; } @@ -38,6 +40,7 @@ MessageParserLibrary& MessageParserLibrary::operator=(MessageParserLibrary&& oth vtable_ = other.vtable_; static_dialog_vtable_ = other.static_dialog_vtable_; path_ = std::move(other.path_); + resolved_path_ = std::move(other.resolved_path_); other.vtable_ = nullptr; other.static_dialog_vtable_ = nullptr; } @@ -49,28 +52,33 @@ Expected MessageParserLibrary::load(std::string_view path) } Expected MessageParserLibrary::load(const std::filesystem::path& path) { - std::filesystem::path loaded_path; - auto raw_handle = detail::loadLibraryHandle(path, &loaded_path); + detail::LibraryPathIdentity recorded_path; + auto raw_handle = detail::loadLibraryHandle(path, &recorded_path); if (!raw_handle) { return unexpected(raw_handle.error()); } - return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), loaded_path); + return loadFromHandleWithIdentity(detail::adoptLibraryHandle(*raw_handle), recorded_path); } Expected MessageParserLibrary::loadFromHandle( std::shared_ptr handle, const std::filesystem::path& origin) { + auto recorded_path = detail::recordLibraryPathIdentity(origin); + if (!recorded_path) { + return unexpected(recorded_path.error()); + } + return loadFromHandleWithIdentity(std::move(handle), *recorded_path); +} + +Expected MessageParserLibrary::loadFromHandleWithIdentity( + std::shared_ptr handle, const detail::LibraryPathIdentity& recorded_path) { if (handle == nullptr) { return unexpected("library not loaded"); } - auto loaded_path = detail::normalizedAbsoluteLibraryPath(origin); - if (!loaded_path) { - return unexpected(loaded_path.error()); - } - if (auto abi = detail::checkPluginAbiVersion(handle.get(), *loaded_path); !abi) { + if (auto abi = detail::checkPluginAbiVersion(handle.get(), recorded_path); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_message_parser_vtable", *loaded_path); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_message_parser_vtable", recorded_path); if (!sym) { return unexpected(sym.error()); } @@ -90,7 +98,9 @@ Expected MessageParserLibrary::loadFromHandle( return unexpected(status.error()); } - return MessageParserLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(*loaded_path)); + return MessageParserLibrary( + std::move(handle), vtable, detail::pathForLegacyAccessor(recorded_path.load_path), + detail::pathForLegacyAccessor(recorded_path.resolved_path)); } Expected MessageParserLibrary::loadStatic( @@ -120,7 +130,7 @@ Expected MessageParserLibrary::loadStatic( } static char anchor = 0; std::shared_ptr handle(&anchor, [](void*) {}); - return MessageParserLibrary(std::move(handle), vtable, "static://", dialog_vtable); + return MessageParserLibrary(std::move(handle), vtable, "static://", "", dialog_vtable); } Expected MessageParserLibrary::resolveDialogVtable() const { @@ -133,7 +143,8 @@ Expected MessageParserLibrary::resolveDialogVtable() #if defined(_WIN32) auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", {}); #else - auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", std::filesystem::path(path_)); + auto sym = detail::resolveSymbol( + handle_.get(), "PJ_get_dialog_vtable", {std::filesystem::path(path_), std::filesystem::path(resolved_path_)}); #endif if (!sym) { return unexpected(sym.error()); @@ -161,6 +172,7 @@ void MessageParserLibrary::reset() { vtable_ = nullptr; static_dialog_vtable_ = nullptr; path_.clear(); + resolved_path_.clear(); } } diff --git a/pj_plugins/src/native_parser_module.cpp b/pj_plugins/src/native_parser_module.cpp index 2bcba3f3..0a9a181f 100644 --- a/pj_plugins/src/native_parser_module.cpp +++ b/pj_plugins/src/native_parser_module.cpp @@ -48,7 +48,7 @@ Expected rejectLoad( template Expected resolve( - detail::NativeModuleHandle handle, const char* name, const std::filesystem::path& candidate_path) { + detail::NativeModuleHandle handle, const char* name, const detail::LibraryPathIdentity& candidate_path) { auto symbol = detail::resolveNativeParserModuleSymbol(handle, name, candidate_path); if (!symbol) { return unexpected(symbol.error()); @@ -63,8 +63,8 @@ NativeParserModule::NativeParserModule(std::shared_ptr NativeParserModule::load( std::string_view path, DiagnosticSink sink, std::string diagnostic_source) { - std::filesystem::path loaded_path; - auto handle_result = detail::openNativeParserModule(path, &loaded_path); + detail::LibraryPathIdentity recorded_path; + auto handle_result = detail::openNativeParserModule(path, &recorded_path); if (!handle_result) { return rejectLoad(path, sink, diagnostic_source, "failed to open native parser module: " + handle_result.error()); } @@ -77,7 +77,7 @@ Expected NativeParserModule::load( #define PJ_RESOLVE_MODULE_EXPORT(member, type, name) \ do { \ - auto resolved = resolve(handle, name, loaded_path); \ + auto resolved = resolve(handle, name, recorded_path); \ if (!resolved) { \ return rejectLoad(path, sink, diagnostic_source, resolved.error()); \ } \ diff --git a/pj_plugins/src/plugin_catalog.cpp b/pj_plugins/src/plugin_catalog.cpp index 81a7e394..6beb978a 100644 --- a/pj_plugins/src/plugin_catalog.cpp +++ b/pj_plugins/src/plugin_catalog.cpp @@ -48,7 +48,7 @@ bool hasDsoSuffix(const std::filesystem::path& path) { // Only the family-specific types and constants vary. template Expected probeDirectVtable( - void* handle, const std::filesystem::path& origin, const char* symbol, const char* family_name, + void* handle, const detail::LibraryPathIdentity& origin, const char* symbol, const char* family_name, uint32_t expected_protocol, size_t min_vtable_size, PluginFamily family) { auto sym = detail::resolveSymbol(handle, symbol, origin); if (!sym) { @@ -70,25 +70,25 @@ Expected probeDirectVtable( return ManifestCandidate{family, vt->manifest_json == nullptr ? "" : vt->manifest_json}; } -Expected tryDataSource(void* handle, const std::filesystem::path& origin) { +Expected tryDataSource(void* handle, const detail::LibraryPathIdentity& origin) { return probeDirectVtable( handle, origin, "PJ_get_data_source_vtable", "DataSource", PJ_DATA_SOURCE_PROTOCOL_VERSION, PJ_DATA_SOURCE_MIN_VTABLE_SIZE, PluginFamily::kDataSource); } -Expected tryMessageParser(void* handle, const std::filesystem::path& origin) { +Expected tryMessageParser(void* handle, const detail::LibraryPathIdentity& origin) { return probeDirectVtable( handle, origin, "PJ_get_message_parser_vtable", "MessageParser", PJ_MESSAGE_PARSER_PROTOCOL_VERSION, PJ_MESSAGE_PARSER_MIN_VTABLE_SIZE, PluginFamily::kMessageParser); } -Expected tryToolbox(void* handle, const std::filesystem::path& origin) { +Expected tryToolbox(void* handle, const detail::LibraryPathIdentity& origin) { return probeDirectVtable( handle, origin, "PJ_get_toolbox_vtable", "Toolbox", PJ_TOOLBOX_PLUGIN_PROTOCOL_VERSION, PJ_TOOLBOX_MIN_VTABLE_SIZE, PluginFamily::kToolbox); } -Expected tryDialog(void* handle, const std::filesystem::path& origin) { +Expected tryDialog(void* handle, const detail::LibraryPathIdentity& origin) { auto sym = detail::resolveSymbol(handle, "PJ_get_dialog_vtable", origin); if (!sym) { return unexpected(sym.error()); @@ -121,7 +121,7 @@ Expected tryDialog(void* handle, const std::filesystem::path& return ManifestCandidate{PluginFamily::kDialog, std::move(manifest_json)}; } -Expected findEmbeddedManifest(void* handle, const std::filesystem::path& origin) { +Expected findEmbeddedManifest(void* handle, const detail::LibraryPathIdentity& origin) { std::vector errors; if (auto candidate = tryDataSource(handle, origin)) { @@ -307,12 +307,12 @@ Expected inspectPluginDso(const std::filesystem::path& dso_pat return unexpected(fmt::format("not a platform plugin DSO: {}", dso_path.string())); } - std::filesystem::path loaded_path; - auto raw_handle = detail::loadLibraryHandle(dso_path, &loaded_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 inspectPluginDso(detail::adoptLibraryHandle(*raw_handle), loaded_path); + return inspectPluginDso(detail::adoptLibraryHandle(*raw_handle), recorded_path.load_path); } Expected inspectPluginDso( @@ -325,16 +325,16 @@ Expected inspectPluginDso( return unexpected(with_path("library not loaded")); } - auto loaded_path = detail::normalizedAbsoluteLibraryPath(dso_path); - if (!loaded_path) { - return unexpected(with_path(loaded_path.error())); + auto recorded_path = detail::recordLibraryPathIdentity(dso_path); + if (!recorded_path) { + return unexpected(with_path(recorded_path.error())); } - if (auto abi = detail::checkPluginAbiVersion(handle.get(), *loaded_path); !abi) { + if (auto abi = detail::checkPluginAbiVersion(handle.get(), *recorded_path); !abi) { return unexpected(with_path(abi.error())); } - auto candidate = findEmbeddedManifest(handle.get(), *loaded_path); + auto candidate = findEmbeddedManifest(handle.get(), *recorded_path); if (!candidate) { return unexpected(with_path(candidate.error())); } @@ -354,13 +354,13 @@ std::vector exportedPluginFamilies( if (handle == nullptr) { return families; } - auto loaded_path = normalizedAbsoluteLibraryPath(dso_path); - if (!loaded_path) { + auto recorded_path = recordLibraryPathIdentity(dso_path); + if (!recorded_path) { return families; } auto append_if_owned = [&](const char* symbol, PluginFamily family) { - if (resolveSymbol(handle.get(), symbol, *loaded_path)) { + if (resolveSymbol(handle.get(), symbol, *recorded_path)) { families.push_back(family); } }; diff --git a/pj_plugins/src/toolbox_library.cpp b/pj_plugins/src/toolbox_library.cpp index 4ef4edcd..9842ec5a 100644 --- a/pj_plugins/src/toolbox_library.cpp +++ b/pj_plugins/src/toolbox_library.cpp @@ -11,12 +11,13 @@ namespace PJ { ToolboxLibrary::ToolboxLibrary( - std::shared_ptr handle, const PJ_toolbox_vtable_t* vtable, std::string path, + std::shared_ptr handle, const PJ_toolbox_vtable_t* vtable, std::string path, std::string resolved_path, const PJ_dialog_vtable_t* static_dialog_vtable) : handle_(std::move(handle)), vtable_(vtable), static_dialog_vtable_(static_dialog_vtable), - path_(std::move(path)) {} + path_(std::move(path)), + resolved_path_(std::move(resolved_path)) {} ToolboxLibrary::~ToolboxLibrary() { reset(); @@ -26,7 +27,8 @@ ToolboxLibrary::ToolboxLibrary(ToolboxLibrary&& other) noexcept : handle_(std::move(other.handle_)), vtable_(other.vtable_), static_dialog_vtable_(other.static_dialog_vtable_), - path_(std::move(other.path_)) { + path_(std::move(other.path_)), + resolved_path_(std::move(other.resolved_path_)) { other.vtable_ = nullptr; other.static_dialog_vtable_ = nullptr; } @@ -38,6 +40,7 @@ ToolboxLibrary& ToolboxLibrary::operator=(ToolboxLibrary&& other) noexcept { vtable_ = other.vtable_; static_dialog_vtable_ = other.static_dialog_vtable_; path_ = std::move(other.path_); + resolved_path_ = std::move(other.resolved_path_); other.vtable_ = nullptr; other.static_dialog_vtable_ = nullptr; } @@ -49,28 +52,33 @@ Expected ToolboxLibrary::load(std::string_view path) { } Expected ToolboxLibrary::load(const std::filesystem::path& path) { - std::filesystem::path loaded_path; - auto raw_handle = detail::loadLibraryHandle(path, &loaded_path); + detail::LibraryPathIdentity recorded_path; + auto raw_handle = detail::loadLibraryHandle(path, &recorded_path); if (!raw_handle) { return unexpected(raw_handle.error()); } - return loadFromHandle(detail::adoptLibraryHandle(*raw_handle), loaded_path); + return loadFromHandleWithIdentity(detail::adoptLibraryHandle(*raw_handle), recorded_path); } Expected ToolboxLibrary::loadFromHandle( std::shared_ptr handle, const std::filesystem::path& origin) { + auto recorded_path = detail::recordLibraryPathIdentity(origin); + if (!recorded_path) { + return unexpected(recorded_path.error()); + } + return loadFromHandleWithIdentity(std::move(handle), *recorded_path); +} + +Expected ToolboxLibrary::loadFromHandleWithIdentity( + std::shared_ptr handle, const detail::LibraryPathIdentity& recorded_path) { if (handle == nullptr) { return unexpected("library not loaded"); } - auto loaded_path = detail::normalizedAbsoluteLibraryPath(origin); - if (!loaded_path) { - return unexpected(loaded_path.error()); - } - if (auto abi = detail::checkPluginAbiVersion(handle.get(), *loaded_path); !abi) { + if (auto abi = detail::checkPluginAbiVersion(handle.get(), recorded_path); !abi) { return unexpected(abi.error()); } - auto sym = detail::resolveSymbol(handle.get(), "PJ_get_toolbox_vtable", *loaded_path); + auto sym = detail::resolveSymbol(handle.get(), "PJ_get_toolbox_vtable", recorded_path); if (!sym) { return unexpected(sym.error()); } @@ -90,7 +98,9 @@ Expected ToolboxLibrary::loadFromHandle( return unexpected(status.error()); } - return ToolboxLibrary(std::move(handle), vtable, detail::pathForLegacyAccessor(*loaded_path)); + return ToolboxLibrary( + std::move(handle), vtable, detail::pathForLegacyAccessor(recorded_path.load_path), + detail::pathForLegacyAccessor(recorded_path.resolved_path)); } Expected ToolboxLibrary::loadStatic( @@ -120,7 +130,7 @@ Expected ToolboxLibrary::loadStatic( } static char anchor = 0; std::shared_ptr handle(&anchor, [](void*) {}); - return ToolboxLibrary(std::move(handle), vtable, "static://", dialog_vtable); + return ToolboxLibrary(std::move(handle), vtable, "static://", "", dialog_vtable); } Expected ToolboxLibrary::resolveDialogVtable() const { @@ -133,7 +143,8 @@ Expected ToolboxLibrary::resolveDialogVtable() const #if defined(_WIN32) auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", {}); #else - auto sym = detail::resolveSymbol(handle_.get(), "PJ_get_dialog_vtable", std::filesystem::path(path_)); + auto sym = detail::resolveSymbol( + handle_.get(), "PJ_get_dialog_vtable", {std::filesystem::path(path_), std::filesystem::path(resolved_path_)}); #endif if (!sym) { return unexpected(sym.error()); @@ -161,6 +172,7 @@ void ToolboxLibrary::reset() { vtable_ = nullptr; static_dialog_vtable_ = nullptr; path_.clear(); + resolved_path_.clear(); } } diff --git a/pj_plugins/tests/entry_point_forwarder.def b/pj_plugins/tests/entry_point_forwarder.def index 52f49a04..ef0b96fc 100644 --- a/pj_plugins/tests/entry_point_forwarder.def +++ b/pj_plugins/tests/entry_point_forwarder.def @@ -1,3 +1,3 @@ LIBRARY entry_point_forwarder_plugin EXPORTS - PJ_get_data_source_vtable=entry_point_donor.PJ_get_data_source_vtable + PJ_get_data_source_vtable = entry_point_donor.PJ_get_data_source_vtable diff --git a/pj_plugins/tests/source_dialog_integration_test.cpp b/pj_plugins/tests/source_dialog_integration_test.cpp index 048b893b..9cf96bc2 100644 --- a/pj_plugins/tests/source_dialog_integration_test.cpp +++ b/pj_plugins/tests/source_dialog_integration_test.cpp @@ -10,6 +10,7 @@ #include #include +#include "detail/library_loader.hpp" #include "pj_plugins/host/config_envelope.hpp" #include "pj_plugins/host/data_source_library.hpp" #include "pj_plugins/host/dialog_handle.hpp" @@ -96,17 +97,45 @@ TEST(SourceDialogIntegration, ResolveDialogVtable) { TEST(SourceDialogIntegration, DialogVtableSurvivesCandidateFileDeletion) { TemporaryDirectory temporary; - const std::filesystem::path candidate = - temporary.path() / std::filesystem::path(PJ_MOCK_SOURCE_WITH_DIALOG_PLUGIN_PATH).filename(); + const std::filesystem::path plugin_filename = + std::filesystem::path(PJ_MOCK_SOURCE_WITH_DIALOG_PLUGIN_PATH).filename(); + const std::filesystem::path candidate = temporary.path() / plugin_filename; std::filesystem::copy_file(PJ_MOCK_SOURCE_WITH_DIALOG_PLUGIN_PATH, candidate); - auto lib = PJ::DataSourceLibrary::load(candidate); - ASSERT_TRUE(lib) << lib.error(); - ASSERT_TRUE(std::filesystem::remove(candidate)); + { + auto lib = PJ::DataSourceLibrary::load(candidate); + ASSERT_TRUE(lib) << lib.error(); + ASSERT_TRUE(std::filesystem::remove(candidate)); - auto dialog_vtable = lib->resolveDialogVtable(); - ASSERT_TRUE(dialog_vtable) << dialog_vtable.error(); - EXPECT_EQ((*dialog_vtable)->protocol_version, PJ_DIALOG_PROTOCOL_VERSION); + auto dialog_vtable = lib->resolveDialogVtable(); + ASSERT_TRUE(dialog_vtable) << dialog_vtable.error(); + EXPECT_EQ((*dialog_vtable)->protocol_version, PJ_DIALOG_PROTOCOL_VERSION); + } + +#if !defined(_WIN32) + const std::filesystem::path real_directory = temporary.path() / "real"; + const std::filesystem::path symlink_directory = temporary.path() / "symlink"; + std::filesystem::create_directories(real_directory); + std::filesystem::create_directory_symlink(real_directory, symlink_directory); + const std::filesystem::path real_candidate = real_directory / plugin_filename; + const std::filesystem::path symlink_candidate = symlink_directory / plugin_filename; + std::filesystem::copy_file(PJ_MOCK_SOURCE_WITH_DIALOG_PLUGIN_PATH, real_candidate); + + // Preload the real spelling so glibc reuses a link-map whose dli_fname is the + // canonical path when the library API subsequently loads through the symlink. + // This reproduces dyld's realpath reporting on Linux. + auto preloaded_handle = PJ::detail::loadLibraryHandle(real_candidate); + ASSERT_TRUE(preloaded_handle) << preloaded_handle.error(); + auto preloaded_owner = PJ::detail::adoptLibraryHandle(*preloaded_handle); + + auto symlink_lib = PJ::DataSourceLibrary::load(symlink_candidate); + ASSERT_TRUE(symlink_lib) << symlink_lib.error(); + ASSERT_TRUE(std::filesystem::remove(real_candidate)); + + auto symlink_dialog_vtable = symlink_lib->resolveDialogVtable(); + ASSERT_TRUE(symlink_dialog_vtable) << symlink_dialog_vtable.error(); + EXPECT_EQ((*symlink_dialog_vtable)->protocol_version, PJ_DIALOG_PROTOCOL_VERSION); +#endif } TEST(SourceDialogIntegration, DialogVtableSurvivesCwdChangeAfterRelativeLoad) { From 1c99bd86d5d1be8cec8a693900ec2b54edf72730 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Tue, 11 Aug 2026 09:44:17 +0200 Subject: [PATCH 4/5] fix(tests): emit the PE forwarder via /export pragma instead of a .def entry MSVC LINK parses the .def EXPORTS forwarder entry as an internal-name alias and demands a local definition (LNK2001 in two CI rounds); the /export linker-directive form emits a true forwarder. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0165qLfnJYSYb51NQMdPidZr --- pj_plugins/CMakeLists.txt | 13 ++++++------- pj_plugins/tests/entry_point_forwarder.cpp | 8 ++++++++ pj_plugins/tests/entry_point_forwarder.def | 3 --- 3 files changed, 14 insertions(+), 10 deletions(-) delete mode 100644 pj_plugins/tests/entry_point_forwarder.def diff --git a/pj_plugins/CMakeLists.txt b/pj_plugins/CMakeLists.txt index 1510855f..f1094880 100644 --- a/pj_plugins/CMakeLists.txt +++ b/pj_plugins/CMakeLists.txt @@ -350,8 +350,8 @@ add_library(entry_point_donor SHARED tests/entry_point_donor.cpp) target_compile_features(entry_point_donor PRIVATE cxx_std_20) target_compile_options(entry_point_donor PRIVATE ${PJ_WARNING_FLAGS}) target_link_libraries(entry_point_donor PRIVATE pj_base) -# The PE forwarder string below names entry_point_donor.dll at runtime. Pin the -# output basename so it cannot drift from the module token in the .def file. +# The PE forwarder pragma in entry_point_forwarder.cpp names entry_point_donor.dll +# at runtime. Pin the output basename so it cannot drift from that module token. set_target_properties(entry_point_donor PROPERTIES OUTPUT_NAME entry_point_donor) add_library(entry_point_via_dependency_plugin SHARED tests/entry_point_via_dependency.cpp) @@ -367,14 +367,13 @@ target_link_libraries(entry_point_with_own_exports_plugin PRIVATE entry_point_do if(WIN32) add_library(entry_point_forwarder_plugin SHARED tests/entry_point_forwarder.cpp - tests/entry_point_forwarder.def ) target_compile_features(entry_point_forwarder_plugin PRIVATE cxx_std_20) target_compile_options(entry_point_forwarder_plugin PRIVATE ${PJ_WARNING_FLAGS}) - # Deliberately do not link entry_point_donor: the .def entry is a PE forwarder, - # not an import that the linker should resolve. Both targets retain CMake's - # common runtime output directory, so entry_point_donor.dll is available when - # GetProcAddress follows the forwarder. + # Deliberately do not link entry_point_donor: the /export pragma in the source + # emits a PE forwarder, not an import the linker should resolve. Both targets + # share one runtime output directory, so entry_point_donor.dll is available + # when GetProcAddress follows the forwarder. target_link_libraries(entry_point_forwarder_plugin PRIVATE pj_base) add_dependencies(entry_point_forwarder_plugin entry_point_donor) set_target_properties( diff --git a/pj_plugins/tests/entry_point_forwarder.cpp b/pj_plugins/tests/entry_point_forwarder.cpp index a19c1084..2a0d8973 100644 --- a/pj_plugins/tests/entry_point_forwarder.cpp +++ b/pj_plugins/tests/entry_point_forwarder.cpp @@ -2,3 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 #include "pj_base/plugin_abi_export.hpp" + +#if defined(_MSC_VER) +// PE forwarder for the family getter: the export resolves at GetProcAddress +// time from entry_point_donor.dll. The /export directive form is required +// because MSVC LINK parses the equivalent .def EXPORTS entry as an +// internal-name alias and demands a local definition (LNK2001). +#pragma comment(linker, "/export:PJ_get_data_source_vtable=entry_point_donor.PJ_get_data_source_vtable") +#endif diff --git a/pj_plugins/tests/entry_point_forwarder.def b/pj_plugins/tests/entry_point_forwarder.def deleted file mode 100644 index ef0b96fc..00000000 --- a/pj_plugins/tests/entry_point_forwarder.def +++ /dev/null @@ -1,3 +0,0 @@ -LIBRARY entry_point_forwarder_plugin -EXPORTS - PJ_get_data_source_vtable = entry_point_donor.PJ_get_data_source_vtable From 47f0ef23102873825d8142a426d12a619ac04be0 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Tue, 11 Aug 2026 10:04:10 +0200 Subject: [PATCH 5/5] fix(tests): make the two Windows loader tests honest about platform semantics Rename instead of delete the mapped candidate on Windows (deletion of a mapped image is impossible there; deferred resolution must survive the path vanishing either way), and emit the real dependency fixture into its own directory so LOAD_LIBRARY_SEARCH_APPLICATION_DIR cannot resolve it from beside the test executable and defeat the decoy-only case. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0165qLfnJYSYb51NQMdPidZr --- pj_plugins/CMakeLists.txt | 10 +++++++++- pj_plugins/tests/source_dialog_integration_test.cpp | 6 ++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/pj_plugins/CMakeLists.txt b/pj_plugins/CMakeLists.txt index f1094880..b2bb9758 100644 --- a/pj_plugins/CMakeLists.txt +++ b/pj_plugins/CMakeLists.txt @@ -390,7 +390,15 @@ endif() add_library(dependency_search_real SHARED tests/dependency_search_dependency.cpp) target_compile_features(dependency_search_real PRIVATE cxx_std_20) target_compile_options(dependency_search_real PRIVATE ${PJ_WARNING_FLAGS}) -set_target_properties(dependency_search_real PROPERTIES OUTPUT_NAME pj_dependency_search_fixture) +set_target_properties(dependency_search_real PROPERTIES + OUTPUT_NAME pj_dependency_search_fixture + # Keep this DLL out of the test executable's directory: on Windows the + # loader's LOAD_LIBRARY_SEARCH_APPLICATION_DIR leg would otherwise resolve + # the dependency from there and defeat the decoy-only case. + ARCHIVE_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/dependency_search_real" + LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/dependency_search_real" + RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/dependency_search_real" +) add_library(dependency_search_decoy SHARED tests/dependency_search_dependency.cpp) target_compile_features(dependency_search_decoy PRIVATE cxx_std_20) diff --git a/pj_plugins/tests/source_dialog_integration_test.cpp b/pj_plugins/tests/source_dialog_integration_test.cpp index 9cf96bc2..7a56e5b6 100644 --- a/pj_plugins/tests/source_dialog_integration_test.cpp +++ b/pj_plugins/tests/source_dialog_integration_test.cpp @@ -105,7 +105,13 @@ TEST(SourceDialogIntegration, DialogVtableSurvivesCandidateFileDeletion) { { auto lib = PJ::DataSourceLibrary::load(candidate); ASSERT_TRUE(lib) << lib.error(); +#if defined(_WIN32) + // A mapped image cannot be deleted on Windows, but it can be renamed away; + // deferred resolution must not depend on the original path either way. + std::filesystem::rename(candidate, candidate.parent_path() / "moved-away.dll"); +#else ASSERT_TRUE(std::filesystem::remove(candidate)); +#endif auto dialog_vtable = lib->resolveDialogVtable(); ASSERT_TRUE(dialog_vtable) << dialog_vtable.error();