From 734be3f21928beb67895fe97b034508e54e628d3 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Fri, 7 Jul 2023 15:11:19 -0700 Subject: [PATCH 01/17] Add test and copy implementation to realpath2 --- .../PortableAppActivation.cs | 25 ++++++ src/native/corehost/hostmisc/pal.h | 1 + src/native/corehost/hostmisc/pal.unix.cpp | 5 ++ src/native/corehost/hostmisc/pal.windows.cpp | 82 +++++++++++++++++++ 4 files changed, 113 insertions(+) diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index fa19efd53121ce..d873f4cd4fd2cc 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -19,6 +19,31 @@ public PortableAppActivation(SharedTestState fixture) sharedTestState = fixture; } + [Fact] + public void Muxer_behind_symlink() + { + var fixture = sharedTestState.PortableAppFixture_Built + .Copy(); + + var dotnetLoc = fixture.BuiltDotnet.DotnetExecutablePath; + var appDll = fixture.TestProject.AppDll; + var testDir = Directory.GetParent(fixture.TestProject.Location).ToString(); + + var exeSuffix = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".exe" : ""; + + using var symlink = new SymLink(Path.Combine(testDir, "dn" + exeSuffix), dotnetLoc); + + var cmd = Command.Create(symlink.SrcPath, new[] { appDll }) + .CaptureStdOut() + .CaptureStdErr() + .EnvironmentVariable("DOTNET_SKIP_FIRST_TIME_EXPERIENCE", "1") + .MultilevelLookup(false); // Avoid looking at machine state by default + + cmd.Execute() + .Should().Pass() + .And.HaveStdOutContaining("Hello World"); + } + [Fact] public void Muxer_Default() { diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index f482b6df63456b..eeb90f79cfbbbd 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -280,6 +280,7 @@ namespace pal bool touch_file(const string_t& path); bool realpath(string_t* path, bool skip_error_logging = false); + bool fullpath(string_t* path, bool skip_error_logging = false); bool file_exists(const string_t& path); inline bool directory_exists(const string_t& path) { return file_exists(path); } void readdir(const string_t& path, const string_t& pattern, std::vector* list); diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 34520aefd7365a..ce72b0feaa6fc0 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -944,6 +944,11 @@ bool pal::getenv(const pal::char_t* name, pal::string_t* recv) return (recv->length() > 0); } +bool pal::fullpath(pal::string_t* path, bool skip_error_logging) +{ + return realpath(path, skip_error_logging); +} + bool pal::realpath(pal::string_t* path, bool skip_error_logging) { auto resolved = ::realpath(path->c_str(), nullptr); diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index b11610492d3214..f9aa087f26336a 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -729,6 +729,88 @@ bool pal::clr_palstring(const char* cstr, pal::string_t* out) // Return if path is valid and file exists, return true and adjust path as appropriate. bool pal::realpath(string_t* path, bool skip_error_logging) +{ + return fullpath(path, skip_error_logging); +} + +typedef std::unique_ptr::type, decltype(&::CloseHandle)> SmartHandle; + +// Like realpath, but resolves symlinks. +bool pal::realpath2(string_t* path, bool skip_error_logging) +{ + if (path->empty()) + { + return false; + } + + // Use CreateFileW + GetFinalPathNameByHandleW to resolve symlinks + + SmartHandle file( + ::CreateFileW( + path->c_str(), + 0, // Querying only + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + nullptr, // default security + OPEN_EXISTING, // existing file + FILE_ATTRIBUTE_NORMAL, // normal file + nullptr), // No attribute template + &::CloseHandle); + + char_t buf[MAX_PATH]; + size_t size; + + if (file.get() == INVALID_HANDLE_VALUE) + { + // If we get "access denied" that may mean the path represents a directory. + // Even if not, we can fall back to GetFullPathNameW, which doesn't require a HANDLE + + auto error = ::GetLastError(); + file.release(); + if (ERROR_ACCESS_DENIED != error) + { + goto invalidPath; + } + } + else + { + size = ::GetFinalPathNameByHandleW(file.get(), buf, MAX_PATH, FILE_NAME_NORMALIZED); + // If size is 0, this call failed. Fall back to GetFullPathNameW, below + if (size != 0) + { + string_t str; + if (size < MAX_PATH) + { + str.assign(buf); + } + else + { + str.resize(size, 0); + size = ::GetFinalPathNameByHandleW(file.get(), (LPWSTR)str.data(), static_cast(size), FILE_NAME_NORMALIZED); + assert(size <= str.size()); + + if (size == 0) + { + goto invalidPath; + } + } + + // Remove the \\?\ prefix, unless it is necessary or was already there + if (LongFile::IsExtended(str) && !LongFile::IsExtended(*path) && + !LongFile::ShouldNormalize(str.substr(LongFile::ExtendedPrefix.size()))) + { + str.erase(0, LongFile::ExtendedPrefix.size()); + } + + *path = str; + return true; + } + } + + // If the above fails, fall back to fullpath + return fullpath(path, skip_error_logging); +} + +bool pal::fullpath(string_t* path, bool skip_error_logging) { if (path->empty()) { From 43092b33a5a9c1ce4ee4e0db3001ace19c38ba6c Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Fri, 7 Jul 2023 15:34:13 -0700 Subject: [PATCH 02/17] Remove realpath2 from pal --- src/native/corehost/hostmisc/pal.windows.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index f9aa087f26336a..af356cd2530496 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -736,7 +736,7 @@ bool pal::realpath(string_t* path, bool skip_error_logging) typedef std::unique_ptr::type, decltype(&::CloseHandle)> SmartHandle; // Like realpath, but resolves symlinks. -bool pal::realpath2(string_t* path, bool skip_error_logging) +bool realpath2(string_t* path, bool skip_error_logging) { if (path->empty()) { From d31e6e560a417a9d8e49a59328b09b4b77448ca5 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Fri, 7 Jul 2023 16:22:10 -0700 Subject: [PATCH 03/17] pal prefix --- src/native/corehost/hostmisc/pal.windows.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index af356cd2530496..97af54c1d1cc25 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -736,7 +736,7 @@ bool pal::realpath(string_t* path, bool skip_error_logging) typedef std::unique_ptr::type, decltype(&::CloseHandle)> SmartHandle; // Like realpath, but resolves symlinks. -bool realpath2(string_t* path, bool skip_error_logging) +static bool realpath2(pal::string_t* path, bool skip_error_logging) { if (path->empty()) { @@ -756,7 +756,7 @@ bool realpath2(string_t* path, bool skip_error_logging) nullptr), // No attribute template &::CloseHandle); - char_t buf[MAX_PATH]; + pal::char_t buf[MAX_PATH]; size_t size; if (file.get() == INVALID_HANDLE_VALUE) @@ -777,7 +777,7 @@ bool realpath2(string_t* path, bool skip_error_logging) // If size is 0, this call failed. Fall back to GetFullPathNameW, below if (size != 0) { - string_t str; + pal::string_t str; if (size < MAX_PATH) { str.assign(buf); @@ -807,7 +807,14 @@ bool realpath2(string_t* path, bool skip_error_logging) } // If the above fails, fall back to fullpath - return fullpath(path, skip_error_logging); + return pal::fullpath(path, skip_error_logging); + +invalidPath: + if (!skip_error_logging) + { + trace::error(_X("Error resolving full path [%s]"), path->c_str()); + } + return false; } bool pal::fullpath(string_t* path, bool skip_error_logging) From 099f101dd3222bc0bd21ef30dd850a4cb2e0146f Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Fri, 7 Jul 2023 23:03:37 -0700 Subject: [PATCH 04/17] Migrate from realpath to fullpath realpath is guaranteed to resolve symlinks. fullpath may resolve symlinks. Moves all existing code to call fullpath, which is the existing contract. On Unix, fullpath calls realpath, meaning it resolves symlinks. On Windows it doesn't. Code which requires symlinks to be resolved should be moved to call realpath. realpath is temporarily renamed realpath2 in this commit to find dangling references. --- src/native/corehost/corehost.cpp | 4 ++-- src/native/corehost/fxr/command_line.cpp | 4 ++-- src/native/corehost/fxr/fx_muxer.cpp | 8 +++---- src/native/corehost/fxr/hostfxr.cpp | 4 ++-- src/native/corehost/fxr_resolver.h | 2 +- src/native/corehost/host_startup_info.cpp | 4 ++-- src/native/corehost/hostmisc/pal.h | 2 +- src/native/corehost/hostmisc/pal.unix.cpp | 10 ++++----- src/native/corehost/hostmisc/pal.windows.cpp | 22 +++++++++---------- src/native/corehost/hostmisc/utils.cpp | 2 +- src/native/corehost/hostpolicy/args.cpp | 4 ++-- .../corehost/hostpolicy/deps_format.cpp | 2 +- .../corehost/hostpolicy/deps_resolver.cpp | 2 +- .../hostpolicy/hostpolicy_context.cpp | 2 +- .../corehost/hostpolicy/shared_store.cpp | 2 +- src/native/corehost/runtime_config.cpp | 4 ++-- .../test/nativehost/host_context_test.cpp | 2 +- .../corehost/test/nativehost/nativehost.cpp | 2 +- 18 files changed, 41 insertions(+), 41 deletions(-) diff --git a/src/native/corehost/corehost.cpp b/src/native/corehost/corehost.cpp index 5edc2fbf5d5219..b13e1d766537c7 100644 --- a/src/native/corehost/corehost.cpp +++ b/src/native/corehost/corehost.cpp @@ -102,7 +102,7 @@ int exe_start(const int argc, const pal::char_t* argv[]) pal::initialize_createdump(); pal::string_t host_path; - if (!pal::get_own_executable_path(&host_path) || !pal::realpath(&host_path)) + if (!pal::get_own_executable_path(&host_path) || !pal::fullpath(&host_path)) { trace::error(_X("Failed to resolve full path of the current executable [%s]"), host_path.c_str()); return StatusCode::CoreHostCurHostFindFailure; @@ -137,7 +137,7 @@ int exe_start(const int argc, const pal::char_t* argv[]) { trace::info(_X("Detected Single-File app bundle")); } - else if (!pal::realpath(&app_path)) + else if (!pal::fullpath(&app_path)) { trace::error(_X("The application to execute does not exist: '%s'."), app_path.c_str()); return StatusCode::LibHostAppRootFindFailure; diff --git a/src/native/corehost/fxr/command_line.cpp b/src/native/corehost/fxr/command_line.cpp index 19d204f348898e..6b589292a7ec1d 100644 --- a/src/native/corehost/fxr/command_line.cpp +++ b/src/native/corehost/fxr/command_line.cpp @@ -145,7 +145,7 @@ namespace if (mode == host_mode_t::apphost) { app_candidate = host_info.app_path; - doesAppExist = bundle::info_t::is_single_file_bundle() || pal::realpath(&app_candidate); + doesAppExist = bundle::info_t::is_single_file_bundle() || pal::fullpath(&app_candidate); } else { @@ -169,7 +169,7 @@ namespace } } - doesAppExist = pal::realpath(&app_candidate); + doesAppExist = pal::fullpath(&app_candidate); if (!doesAppExist) { trace::verbose(_X("Application '%s' does not exist."), app_candidate.c_str()); diff --git a/src/native/corehost/fxr/fx_muxer.cpp b/src/native/corehost/fxr/fx_muxer.cpp index cbaf90aa69cba2..a85cc807d5bb9b 100644 --- a/src/native/corehost/fxr/fx_muxer.cpp +++ b/src/native/corehost/fxr/fx_muxer.cpp @@ -226,7 +226,7 @@ void append_probe_realpath(const pal::string_t& path, std::vector { pal::string_t probe_path = path; - if (pal::realpath(&probe_path, true)) + if (pal::fullpath(&probe_path, true)) { realpaths->push_back(probe_path); } @@ -249,7 +249,7 @@ void append_probe_realpath(const pal::string_t& path, std::vector segment.append(tfm); probe_path.replace(pos_placeholder, placeholder.length(), segment); - if (pal::realpath(&probe_path, true)) + if (pal::fullpath(&probe_path, true)) { realpaths->push_back(probe_path); } @@ -274,7 +274,7 @@ namespace const runtime_config_t::settings_t& override_settings) { // Check for the runtimeconfig.json file specified at the command line - if (!runtime_config.empty() && !pal::realpath(&runtime_config)) + if (!runtime_config.empty() && !pal::fullpath(&runtime_config)) { trace::error(_X("The specified runtimeconfig.json [%s] does not exist"), runtime_config.c_str()); return StatusCode::InvalidConfigFile; @@ -377,7 +377,7 @@ namespace // This check is for --depsfile option, which must be an actual file. pal::string_t deps_file = command_line::get_option_value(opts, known_options::deps_file, _X("")); - if (!deps_file.empty() && !pal::realpath(&deps_file)) + if (!deps_file.empty() && !pal::fullpath(&deps_file)) { trace::error(_X("The specified deps.json [%s] does not exist"), deps_file.c_str()); return StatusCode::InvalidArgFailure; diff --git a/src/native/corehost/fxr/hostfxr.cpp b/src/native/corehost/fxr/hostfxr.cpp index a386cbe3762163..400d29b4d9f13f 100644 --- a/src/native/corehost/fxr/hostfxr.cpp +++ b/src/native/corehost/fxr/hostfxr.cpp @@ -546,7 +546,7 @@ namespace if (startup_info.host_path.empty()) { - if (!pal::get_own_executable_path(&startup_info.host_path) || !pal::realpath(&startup_info.host_path)) + if (!pal::get_own_executable_path(&startup_info.host_path) || !pal::fullpath(&startup_info.host_path)) { trace::error(_X("Failed to resolve full path of the current host [%s]"), startup_info.host_path.c_str()); return StatusCode::CoreHostCurHostFindFailure; @@ -560,7 +560,7 @@ namespace return StatusCode::CoreHostCurHostFindFailure; startup_info.dotnet_root = get_dotnet_root_from_fxr_path(mod_path); - if (!pal::realpath(&startup_info.dotnet_root)) + if (!pal::fullpath(&startup_info.dotnet_root)) { trace::error(_X("Failed to resolve full path of dotnet root [%s]"), startup_info.dotnet_root.c_str()); return StatusCode::CoreHostCurHostFindFailure; diff --git a/src/native/corehost/fxr_resolver.h b/src/native/corehost/fxr_resolver.h index 8d6301f0befe2b..36ff4d88e68e3d 100644 --- a/src/native/corehost/fxr_resolver.h +++ b/src/native/corehost/fxr_resolver.h @@ -23,7 +23,7 @@ int load_fxr_and_get_delegate(hostfxr_delegate_type type, THostPathToConfigCallb pal::dll_t fxr; pal::string_t host_path; - if (!pal::get_own_module_path(&host_path) || !pal::realpath(&host_path)) + if (!pal::get_own_module_path(&host_path) || !pal::fullpath(&host_path)) { trace::error(_X("Failed to resolve full path of the current host module [%s]"), host_path.c_str()); return StatusCode::CoreHostCurHostFindFailure; diff --git a/src/native/corehost/host_startup_info.cpp b/src/native/corehost/host_startup_info.cpp index 20bee5859ad794..e07e5785b77826 100644 --- a/src/native/corehost/host_startup_info.cpp +++ b/src/native/corehost/host_startup_info.cpp @@ -23,7 +23,7 @@ bool get_path_from_argv(pal::string_t *path) // the wrong location when filename ends up being found in %PATH% and not the current directory. if (path->find(DIR_SEPARATOR) != pal::string_t::npos) { - return pal::realpath(path); + return pal::fullpath(path); } return false; @@ -86,7 +86,7 @@ const pal::string_t host_startup_info_t::get_app_name() const } // If argv[0] did not work, get the executable name - if (host_path->empty() && (!pal::get_own_executable_path(host_path) || !pal::realpath(host_path))) + if (host_path->empty() && (!pal::get_own_executable_path(host_path) || !pal::fullpath(host_path))) { trace::error(_X("Failed to resolve full path of the current executable [%s]"), host_path->c_str()); return StatusCode::LibHostCurExeFindFailure; diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index eeb90f79cfbbbd..0baa08c05f3150 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -279,7 +279,7 @@ namespace pal void* mmap_copy_on_write(const string_t& path, size_t* length = nullptr); bool touch_file(const string_t& path); - bool realpath(string_t* path, bool skip_error_logging = false); + bool realpath2(string_t* path, bool skip_error_logging = false); bool fullpath(string_t* path, bool skip_error_logging = false); bool file_exists(const string_t& path); inline bool directory_exists(const string_t& path) { return file_exists(path); } diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index ce72b0feaa6fc0..527a5dabd07987 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -269,7 +269,7 @@ bool pal::get_default_breadcrumb_store(string_t* recv) { recv->clear(); pal::string_t ext; - if (pal::getenv(_X("CORE_BREADCRUMBS"), &ext) && pal::realpath(&ext)) + if (pal::getenv(_X("CORE_BREADCRUMBS"), &ext) && pal::fullpath(&ext)) { // We should have the path in ext. trace::info(_X("Realpath CORE_BREADCRUMBS [%s]"), ext.c_str()); @@ -301,7 +301,7 @@ bool pal::get_default_servicing_directory(string_t* recv) { recv->clear(); pal::string_t ext; - if (pal::getenv(_X("CORE_SERVICING"), &ext) && pal::realpath(&ext)) + if (pal::getenv(_X("CORE_SERVICING"), &ext) && pal::fullpath(&ext)) { // We should have the path in ext. trace::info(_X("Realpath CORE_SERVICING [%s]"), ext.c_str()); @@ -332,7 +332,7 @@ bool pal::get_default_servicing_directory(string_t* recv) bool is_read_write_able_directory(pal::string_t& dir) { - return pal::realpath(&dir) && + return pal::fullpath(&dir) && (access(dir.c_str(), R_OK | W_OK | X_OK) == 0); } @@ -946,10 +946,10 @@ bool pal::getenv(const pal::char_t* name, pal::string_t* recv) bool pal::fullpath(pal::string_t* path, bool skip_error_logging) { - return realpath(path, skip_error_logging); + return realpath2(path, skip_error_logging); } -bool pal::realpath(pal::string_t* path, bool skip_error_logging) +bool pal::realpath2(pal::string_t* path, bool skip_error_logging) { auto resolved = ::realpath(path->c_str(), nullptr); if (resolved == nullptr) diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index 97af54c1d1cc25..399b2c394e4e61 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -173,7 +173,7 @@ bool pal::load_library(const string_t* in_path, dll_t* dll) if (LongFile::IsPathNotFullyQualified(path)) { - if (!pal::realpath(&path)) + if (!pal::fullpath(&path)) { trace::error(_X("Failed to load the dll from [%s], HRESULT: 0x%X"), path.c_str(), HRESULT_FROM_WIN32(GetLastError())); return false; @@ -648,7 +648,7 @@ bool get_extraction_base_parent_directory(pal::string_t& directory) assert(len < max_len); directory.assign(temp_path); - return pal::realpath(&directory); + return pal::fullpath(&directory); } bool pal::get_default_bundle_extraction_base_dir(pal::string_t& extraction_dir) @@ -662,7 +662,7 @@ bool pal::get_default_bundle_extraction_base_dir(pal::string_t& extraction_dir) append_path(&extraction_dir, _X(".net")); // Windows Temp-Path is already user-private. - if (realpath(&extraction_dir)) + if (fullpath(&extraction_dir)) { return true; } @@ -675,7 +675,7 @@ bool pal::get_default_bundle_extraction_base_dir(pal::string_t& extraction_dir) return false; } - return realpath(&extraction_dir); + return fullpath(&extraction_dir); } static bool wchar_convert_helper(DWORD code_page, const char* cstr, size_t len, pal::string_t* out) @@ -728,15 +728,15 @@ bool pal::clr_palstring(const char* cstr, pal::string_t* out) } // Return if path is valid and file exists, return true and adjust path as appropriate. -bool pal::realpath(string_t* path, bool skip_error_logging) -{ - return fullpath(path, skip_error_logging); -} +//bool pal::realpath(string_t* path, bool skip_error_logging) +//{ +// return fullpath(path, skip_error_logging); +//} typedef std::unique_ptr::type, decltype(&::CloseHandle)> SmartHandle; // Like realpath, but resolves symlinks. -static bool realpath2(pal::string_t* path, bool skip_error_logging) +bool pal::realpath2(pal::string_t* path, bool skip_error_logging) { if (path->empty()) { @@ -892,7 +892,7 @@ bool pal::fullpath(string_t* path, bool skip_error_logging) bool pal::file_exists(const string_t& path) { string_t tmp(path); - return pal::realpath(&tmp, true); + return pal::fullpath(&tmp, true); } static void readdir(const pal::string_t& path, const pal::string_t& pattern, bool onlydirectories, std::vector* list) @@ -904,7 +904,7 @@ static void readdir(const pal::string_t& path, const pal::string_t& pattern, boo if (LongFile::ShouldNormalize(normalized_path)) { - if (!pal::realpath(&normalized_path)) + if (!pal::fullpath(&normalized_path)) { return; } diff --git a/src/native/corehost/hostmisc/utils.cpp b/src/native/corehost/hostmisc/utils.cpp index 521574fe2a5fd4..7326f3b8b8ae17 100644 --- a/src/native/corehost/hostmisc/utils.cpp +++ b/src/native/corehost/hostmisc/utils.cpp @@ -320,7 +320,7 @@ bool get_file_path_from_env(const pal::char_t* env_key, pal::string_t* recv) pal::string_t file_path; if (pal::getenv(env_key, &file_path)) { - if (pal::realpath(&file_path)) + if (pal::fullpath(&file_path)) { recv->assign(file_path); return true; diff --git a/src/native/corehost/hostpolicy/args.cpp b/src/native/corehost/hostpolicy/args.cpp index a5182719ce0eaa..4c64ebde2a1d37 100644 --- a/src/native/corehost/hostpolicy/args.cpp +++ b/src/native/corehost/hostpolicy/args.cpp @@ -98,10 +98,10 @@ bool set_root_from_app(const pal::string_t& managed_application_path, // for very unlikely case where the main app.dll was itself excluded from the app bundle. // Note that unlike non-single-file we don't want to set the app_root to the location of the app.dll // it needs to stay the location of the single-file bundle. - return pal::realpath(&args.managed_application); + return pal::fullpath(&args.managed_application); } - if (pal::realpath(&args.managed_application)) + if (pal::fullpath(&args.managed_application)) { args.app_root = get_directory(args.managed_application); return true; diff --git a/src/native/corehost/hostpolicy/deps_format.cpp b/src/native/corehost/hostpolicy/deps_format.cpp index 89f0ded1599269..7a58f12bb067f2 100644 --- a/src/native/corehost/hostpolicy/deps_format.cpp +++ b/src/native/corehost/hostpolicy/deps_format.cpp @@ -76,7 +76,7 @@ namespace bool deps_file_exists(pal::string_t& deps_path) { - if (bundle::info_t::config_t::probe(deps_path) || pal::realpath(&deps_path, /*skip_error_logging*/ true)) + if (bundle::info_t::config_t::probe(deps_path) || pal::fullpath(&deps_path, /*skip_error_logging*/ true)) return true; trace::verbose(_X("Dependencies manifest does not exist at [%s]"), deps_path.c_str()); diff --git a/src/native/corehost/hostpolicy/deps_resolver.cpp b/src/native/corehost/hostpolicy/deps_resolver.cpp index 8bab2fa1d79f90..55d9bd8f948868 100644 --- a/src/native/corehost/hostpolicy/deps_resolver.cpp +++ b/src/native/corehost/hostpolicy/deps_resolver.cpp @@ -767,7 +767,7 @@ bool deps_resolver_t::resolve_probe_dirs( std::unordered_set items; pal::string_t core_servicing = m_core_servicing; - pal::realpath(&core_servicing, true); + pal::fullpath(&core_servicing, true); // Filter out non-serviced assets so the paths can be added after servicing paths. pal::string_t non_serviced; diff --git a/src/native/corehost/hostpolicy/hostpolicy_context.cpp b/src/native/corehost/hostpolicy/hostpolicy_context.cpp index 1a1f63dae073c4..63ef9cdbc0d32f 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_context.cpp +++ b/src/native/corehost/hostpolicy/hostpolicy_context.cpp @@ -204,7 +204,7 @@ int hostpolicy_context_t::initialize(const hostpolicy_init_t &hostpolicy_init, c } clr_path = probe_paths.coreclr; - if (clr_path.empty() || !pal::realpath(&clr_path)) + if (clr_path.empty() || !pal::fullpath(&clr_path)) { // in a single-file case we may not need coreclr, // otherwise fail early. diff --git a/src/native/corehost/hostpolicy/shared_store.cpp b/src/native/corehost/hostpolicy/shared_store.cpp index e8ea0d89f0be15..0279bde549e248 100644 --- a/src/native/corehost/hostpolicy/shared_store.cpp +++ b/src/native/corehost/hostpolicy/shared_store.cpp @@ -20,7 +20,7 @@ namespace pal::stringstream_t ss(path); while (std::getline(ss, tok, PATH_SEPARATOR)) { - if (pal::realpath(&tok)) + if (pal::fullpath(&tok)) { append_path(&tok, arch); append_path(&tok, tfm.c_str()); diff --git a/src/native/corehost/runtime_config.cpp b/src/native/corehost/runtime_config.cpp index 08e75fd61e016d..6effd54bc99ace 100644 --- a/src/native/corehost/runtime_config.cpp +++ b/src/native/corehost/runtime_config.cpp @@ -334,7 +334,7 @@ bool runtime_config_t::ensure_dev_config_parsed() trace::verbose(_X("Attempting to read dev runtime config: %s"), m_dev_path.c_str()); pal::string_t retval; - if (!pal::realpath(&m_dev_path, true)) + if (!pal::fullpath(&m_dev_path, true)) { // It is valid for the runtimeconfig.dev.json to not exist. return true; @@ -402,7 +402,7 @@ bool runtime_config_t::ensure_parsed() } trace::verbose(_X("Attempting to read runtime config: %s"), m_path.c_str()); - if (!bundle::info_t::config_t::probe(m_path) && !pal::realpath(&m_path, true)) + if (!bundle::info_t::config_t::probe(m_path) && !pal::fullpath(&m_path, true)) { // Not existing is not an error. trace::verbose(_X("Runtime config does not exist at [%s]"), m_path.c_str()); diff --git a/src/native/corehost/test/nativehost/host_context_test.cpp b/src/native/corehost/test/nativehost/host_context_test.cpp index b7c8c983d3a471..cf05ff2cc5d9de 100644 --- a/src/native/corehost/test/nativehost/host_context_test.cpp +++ b/src/native/corehost/test/nativehost/host_context_test.cpp @@ -917,7 +917,7 @@ bool host_context_test::non_context_mixed( hostfxr_exports hostfxr { hostfxr_path }; pal::string_t host_path; - if (!pal::get_own_executable_path(&host_path) || !pal::realpath(&host_path)) + if (!pal::get_own_executable_path(&host_path) || !pal::fullpath(&host_path)) { trace::error(_X("Failed to resolve full path of the current executable [%s]"), host_path.c_str()); return false; diff --git a/src/native/corehost/test/nativehost/nativehost.cpp b/src/native/corehost/test/nativehost/nativehost.cpp index 09ec47b259b7bf..f7497caf04e58f 100644 --- a/src/native/corehost/test/nativehost/nativehost.cpp +++ b/src/native/corehost/test/nativehost/nativehost.cpp @@ -65,7 +65,7 @@ int main(const int argc, const pal::char_t *argv[]) if (explicit_load) { pal::string_t nethost_path; - if (!pal::get_own_executable_path(&nethost_path) || !pal::realpath(&nethost_path)) + if (!pal::get_own_executable_path(&nethost_path) || !pal::fullpath(&nethost_path)) { std::cout << "Failed to get path to current executable" << std::endl; return EXIT_FAILURE; From 570906dfb4845a091e01e7bcd280c1e3c341148a Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Mon, 11 Mar 2024 17:33:19 -0700 Subject: [PATCH 05/17] Enable muxer behind symlink --- .../HostActivation.Tests/PortableAppActivation.cs | 12 ++++++------ src/installer/tests/TestUtils/SymbolicLinking.cs | 2 +- src/native/corehost/corehost.cpp | 4 +++- src/native/corehost/hostmisc/pal.h | 2 +- src/native/corehost/hostmisc/pal.windows.cpp | 10 ++-------- 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index d873f4cd4fd2cc..fbc1bd63af6c36 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using System.IO; +using System.Runtime.InteropServices; using Microsoft.DotNet.Cli.Build.Framework; using Xunit; @@ -22,16 +23,15 @@ public PortableAppActivation(SharedTestState fixture) [Fact] public void Muxer_behind_symlink() { - var fixture = sharedTestState.PortableAppFixture_Built - .Copy(); + var app = sharedTestState.App.Copy(); - var dotnetLoc = fixture.BuiltDotnet.DotnetExecutablePath; - var appDll = fixture.TestProject.AppDll; - var testDir = Directory.GetParent(fixture.TestProject.Location).ToString(); + var dotnetLoc = TestContext.BuiltDotNet.DotnetExecutablePath; + var appDll = app.AppDll; + var testDir = Directory.GetParent(app.Location).ToString(); var exeSuffix = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".exe" : ""; - using var symlink = new SymLink(Path.Combine(testDir, "dn" + exeSuffix), dotnetLoc); + using var symlink = new SymLink(Path.Combine(testDir, "dotnet" + exeSuffix), dotnetLoc); var cmd = Command.Create(symlink.SrcPath, new[] { appDll }) .CaptureStdOut() diff --git a/src/installer/tests/TestUtils/SymbolicLinking.cs b/src/installer/tests/TestUtils/SymbolicLinking.cs index c41146a5ff6504..e96e0f6b0fb994 100644 --- a/src/installer/tests/TestUtils/SymbolicLinking.cs +++ b/src/installer/tests/TestUtils/SymbolicLinking.cs @@ -35,7 +35,7 @@ private static bool MakeSymbolicLink(string symbolicLinkName, string targetFileN errorMessage = string.Empty; if (OperatingSystem.IsWindows()) { - if (!CreateSymbolicLink(symbolicLinkName, targetFileName, SymbolicLinkFlag.IsFile)) + if (!CreateSymbolicLink(symbolicLinkName, targetFileName, SymbolicLinkFlag.IsFile | SymbolicLinkFlag.AllowUnprivilegedCreate)) { int errno = Marshal.GetLastWin32Error(); errorMessage = $"CreateSymbolicLink failed with error number {errno}"; diff --git a/src/native/corehost/corehost.cpp b/src/native/corehost/corehost.cpp index b13e1d766537c7..190fcc0118d915 100644 --- a/src/native/corehost/corehost.cpp +++ b/src/native/corehost/corehost.cpp @@ -102,7 +102,9 @@ int exe_start(const int argc, const pal::char_t* argv[]) pal::initialize_createdump(); pal::string_t host_path; - if (!pal::get_own_executable_path(&host_path) || !pal::fullpath(&host_path)) + // Use realpath to find the path of the host through symlinks, since hostfxr will be + // next to the target + if (!pal::get_own_executable_path(&host_path) || !pal::realpath(&host_path)) { trace::error(_X("Failed to resolve full path of the current executable [%s]"), host_path.c_str()); return StatusCode::CoreHostCurHostFindFailure; diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index 0baa08c05f3150..eeb90f79cfbbbd 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -279,7 +279,7 @@ namespace pal void* mmap_copy_on_write(const string_t& path, size_t* length = nullptr); bool touch_file(const string_t& path); - bool realpath2(string_t* path, bool skip_error_logging = false); + bool realpath(string_t* path, bool skip_error_logging = false); bool fullpath(string_t* path, bool skip_error_logging = false); bool file_exists(const string_t& path); inline bool directory_exists(const string_t& path) { return file_exists(path); } diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index 399b2c394e4e61..65c6ac4a8116b1 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -727,16 +727,10 @@ bool pal::clr_palstring(const char* cstr, pal::string_t* out) return wchar_convert_helper(CP_UTF8, cstr, ::strlen(cstr), out); } -// Return if path is valid and file exists, return true and adjust path as appropriate. -//bool pal::realpath(string_t* path, bool skip_error_logging) -//{ -// return fullpath(path, skip_error_logging); -//} - typedef std::unique_ptr::type, decltype(&::CloseHandle)> SmartHandle; -// Like realpath, but resolves symlinks. -bool pal::realpath2(pal::string_t* path, bool skip_error_logging) +// Like fullpath, but resolves symlinks. +bool pal::realpath(pal::string_t* path, bool skip_error_logging) { if (path->empty()) { From 1b6c97574b2c8b3b29df0cc5ad10a604051240ba Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Mon, 11 Mar 2024 21:20:16 -0700 Subject: [PATCH 06/17] Enable symlink testing on Windows --- .../AppHost.Bundle.Tests/BundleExtractToSpecificPath.cs | 2 +- src/installer/tests/HostActivation.Tests/InvalidHost.cs | 6 +++--- .../NativeHosting/ComhostSideBySide.cs | 2 +- .../NativeHosting/SharedTestStateBase.cs | 2 +- .../tests/HostActivation.Tests/NativeUnitTests.cs | 2 +- .../tests/HostActivation.Tests/PortableAppActivation.cs | 4 +--- .../HostActivation.Tests/StandaloneAppActivation.cs | 2 +- .../tests/HostActivation.Tests/SymbolicLinks.cs | 9 +++------ .../Bundle/BundlerConsistencyTests.cs | 2 +- src/installer/tests/TestUtils/Binaries.cs | 8 ++++---- src/installer/tests/TestUtils/SingleFileTestApp.cs | 2 +- .../tests/TestUtils/{SymbolicLinking.cs => SymLink.cs} | 0 src/installer/tests/TestUtils/TestApp.cs | 2 +- 13 files changed, 19 insertions(+), 24 deletions(-) rename src/installer/tests/TestUtils/{SymbolicLinking.cs => SymLink.cs} (100%) diff --git a/src/installer/tests/AppHost.Bundle.Tests/BundleExtractToSpecificPath.cs b/src/installer/tests/AppHost.Bundle.Tests/BundleExtractToSpecificPath.cs index d3b2f3a876186b..b04653ddfd21bf 100644 --- a/src/installer/tests/AppHost.Bundle.Tests/BundleExtractToSpecificPath.cs +++ b/src/installer/tests/AppHost.Bundle.Tests/BundleExtractToSpecificPath.cs @@ -31,7 +31,7 @@ private void AbsolutePath() var bundleDir = Directory.GetParent(bundledApp.Path); bundleDir.Should().OnlyHaveFiles(new[] { - Binaries.GetExeFileNameForCurrentPlatform(app.Name), + Binaries.GetExeName(app.Name), $"{app.Name}.pdb" }); diff --git a/src/installer/tests/HostActivation.Tests/InvalidHost.cs b/src/installer/tests/HostActivation.Tests/InvalidHost.cs index 39c8c711a04c74..059650fc3f2763 100644 --- a/src/installer/tests/HostActivation.Tests/InvalidHost.cs +++ b/src/installer/tests/HostActivation.Tests/InvalidHost.cs @@ -112,16 +112,16 @@ public SharedTestState() BaseDirectory = TestArtifact.Create(nameof(InvalidHost)); Directory.CreateDirectory(BaseDirectory.Location); - RenamedDotNet = Path.Combine(BaseDirectory.Location, Binaries.GetExeFileNameForCurrentPlatform("renamed")); + RenamedDotNet = Path.Combine(BaseDirectory.Location, Binaries.GetExeName("renamed")); File.Copy(Binaries.DotNet.FilePath, RenamedDotNet); - UnboundAppHost = Path.Combine(BaseDirectory.Location, Binaries.GetExeFileNameForCurrentPlatform("unbound")); + UnboundAppHost = Path.Combine(BaseDirectory.Location, Binaries.GetExeName("unbound")); File.Copy(Binaries.AppHost.FilePath, UnboundAppHost); if (OperatingSystem.IsWindows()) { // Mark the apphost as GUI, but don't bind it to anything - this will cause it to fail - UnboundAppHostGUI = Path.Combine(BaseDirectory.Location, Binaries.GetExeFileNameForCurrentPlatform("unboundgui")); + UnboundAppHostGUI = Path.Combine(BaseDirectory.Location, Binaries.GetExeName("unboundgui")); File.Copy(Binaries.AppHost.FilePath, UnboundAppHostGUI); PEUtils.SetWindowsGraphicalUserInterfaceBit(UnboundAppHostGUI); } diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs index 8ad38fbdafb1e5..e6c70d5bbff0cf 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/ComhostSideBySide.cs @@ -116,7 +116,7 @@ public SharedTestState() } } - string comsxsName = Binaries.GetExeFileNameForCurrentPlatform("comsxs"); + string comsxsName = Binaries.GetExeName("comsxs"); ComSxsPath = Path.Combine(comsxsDirectory, comsxsName); File.Copy( Path.Combine(RepoDirectoriesProvider.Default.HostTestArtifacts, comsxsName), diff --git a/src/installer/tests/HostActivation.Tests/NativeHosting/SharedTestStateBase.cs b/src/installer/tests/HostActivation.Tests/NativeHosting/SharedTestStateBase.cs index ae855079be71a8..196b114e81d464 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHosting/SharedTestStateBase.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHosting/SharedTestStateBase.cs @@ -21,7 +21,7 @@ public SharedTestStateBase() _baseDirArtifact = TestArtifact.Create("nativeHosting"); BaseDirectory = _baseDirArtifact.Location; - string nativeHostName = Binaries.GetExeFileNameForCurrentPlatform("nativehost"); + string nativeHostName = Binaries.GetExeName("nativehost"); NativeHostPath = Path.Combine(BaseDirectory, nativeHostName); // Copy over native host diff --git a/src/installer/tests/HostActivation.Tests/NativeUnitTests.cs b/src/installer/tests/HostActivation.Tests/NativeUnitTests.cs index a98e323d6ef1ea..0c4114d3e4d98b 100644 --- a/src/installer/tests/HostActivation.Tests/NativeUnitTests.cs +++ b/src/installer/tests/HostActivation.Tests/NativeUnitTests.cs @@ -17,7 +17,7 @@ public void Native_Test_Fx_Ver() { RepoDirectoriesProvider repoDirectoriesProvider = new RepoDirectoriesProvider(); - string testPath = Path.Combine(repoDirectoriesProvider.HostTestArtifacts, Binaries.GetExeFileNameForCurrentPlatform("test_fx_ver")); + string testPath = Path.Combine(repoDirectoriesProvider.HostTestArtifacts, Binaries.GetExeName("test_fx_ver")); Command testCommand = Command.Create(testPath); testCommand diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index fbc1bd63af6c36..e7b517b78ab490 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -29,9 +29,7 @@ public void Muxer_behind_symlink() var appDll = app.AppDll; var testDir = Directory.GetParent(app.Location).ToString(); - var exeSuffix = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".exe" : ""; - - using var symlink = new SymLink(Path.Combine(testDir, "dotnet" + exeSuffix), dotnetLoc); + using var symlink = new SymLink(Path.Combine(testDir, Binaries.GetExeName("dotnet")), dotnetLoc); var cmd = Command.Create(symlink.SrcPath, new[] { appDll }) .CaptureStdOut() diff --git a/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs b/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs index b4ea95307c01d6..93940370a45397 100644 --- a/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/StandaloneAppActivation.cs @@ -76,7 +76,7 @@ public void RenameApphost() { var app = sharedTestState.App.Copy(); - var renamedAppExe = app.AppExe + Binaries.GetExeFileNameForCurrentPlatform("renamed"); + var renamedAppExe = app.AppExe + Binaries.GetExeName("renamed"); File.Move(app.AppExe, renamedAppExe, true); Command.Create(renamedAppExe) diff --git a/src/installer/tests/HostActivation.Tests/SymbolicLinks.cs b/src/installer/tests/HostActivation.Tests/SymbolicLinks.cs index 45ad4f9961af07..2d753a75b1a30c 100644 --- a/src/installer/tests/HostActivation.Tests/SymbolicLinks.cs +++ b/src/installer/tests/HostActivation.Tests/SymbolicLinks.cs @@ -22,11 +22,11 @@ public SymbolicLinks(SymbolicLinks.SharedTestState fixture) } [Theory] - [SkipOnPlatform(TestPlatforms.Windows, "Creating symbolic links requires administrative privilege on Windows, so skip test.")] [InlineData ("a/b/SymlinkToApphost")] [InlineData ("a/SymlinkToApphost")] public void Run_apphost_behind_symlink(string symlinkRelativePath) { + symlinkRelativePath = Binaries.GetExeName(symlinkRelativePath); using (var testDir = TestArtifact.Create("symlink")) { Directory.CreateDirectory(Path.Combine(testDir.Location, Path.GetDirectoryName(symlinkRelativePath))); @@ -43,13 +43,14 @@ public void Run_apphost_behind_symlink(string symlinkRelativePath) } [Theory] - [SkipOnPlatform(TestPlatforms.Windows, "Creating symbolic links requires administrative privilege on Windows, so skip test.")] [InlineData ("a/b/FirstSymlink", "c/d/SecondSymlink")] [InlineData ("a/b/FirstSymlink", "c/SecondSymlink")] [InlineData ("a/FirstSymlink", "c/d/SecondSymlink")] [InlineData ("a/FirstSymlink", "c/SecondSymlink")] public void Run_apphost_behind_transitive_symlinks(string firstSymlinkRelativePath, string secondSymlinkRelativePath) { + firstSymlinkRelativePath = Binaries.GetExeName(firstSymlinkRelativePath); + secondSymlinkRelativePath = Binaries.GetExeName(secondSymlinkRelativePath); using (var testDir = TestArtifact.Create("symlink")) { // second symlink -> apphost @@ -117,7 +118,6 @@ public void Run_framework_dependent_app_with_runtime_behind_symlink() } [Fact] - [SkipOnPlatform(TestPlatforms.Windows, "Creating symbolic links requires administrative privilege on Windows, so skip test.")] public void Put_app_directory_behind_symlink() { var app = sharedTestState.SelfContainedApp.Copy(); @@ -138,7 +138,6 @@ public void Put_app_directory_behind_symlink() } [Fact] - [SkipOnPlatform(TestPlatforms.Windows, "Creating symbolic links requires administrative privilege on Windows, so skip test.")] public void Put_dotnet_behind_symlink() { using (var testDir = TestArtifact.Create("symlink")) @@ -156,7 +155,6 @@ public void Put_dotnet_behind_symlink() } [Fact] - [SkipOnPlatform(TestPlatforms.Windows, "Creating symbolic links requires administrative privilege on Windows, so skip test.")] public void Put_app_directory_behind_symlink_and_use_dotnet() { var app = sharedTestState.SelfContainedApp.Copy(); @@ -177,7 +175,6 @@ public void Put_app_directory_behind_symlink_and_use_dotnet() } [Fact] - [SkipOnPlatform(TestPlatforms.Windows, "Creating symbolic links requires administrative privilege on Windows, so skip test.")] public void Put_satellite_assembly_behind_symlink() { var app = sharedTestState.LocalizedApp.Copy(); diff --git a/src/installer/tests/Microsoft.NET.HostModel.Tests/Bundle/BundlerConsistencyTests.cs b/src/installer/tests/Microsoft.NET.HostModel.Tests/Bundle/BundlerConsistencyTests.cs index b7a2d6ab08cfcc..0087effaa8cb23 100644 --- a/src/installer/tests/Microsoft.NET.HostModel.Tests/Bundle/BundlerConsistencyTests.cs +++ b/src/installer/tests/Microsoft.NET.HostModel.Tests/Bundle/BundlerConsistencyTests.cs @@ -23,7 +23,7 @@ public BundlerConsistencyTests(SharedTestState fixture) sharedTestState = fixture; } - private static string BundlerHostName = Binaries.GetExeFileNameForCurrentPlatform(SharedTestState.AppName); + private static string BundlerHostName = Binaries.GetExeName(SharedTestState.AppName); private Bundler CreateBundlerInstance(BundleOptions bundleOptions = BundleOptions.None, Version version = null, bool macosCodesign = true) => new Bundler(BundlerHostName, sharedTestState.App.GetUniqueSubdirectory("bundle"), bundleOptions, targetFrameworkVersion: version, macosCodesign: macosCodesign); diff --git a/src/installer/tests/TestUtils/Binaries.cs b/src/installer/tests/TestUtils/Binaries.cs index de2538c939c29c..e715068f70b341 100644 --- a/src/installer/tests/TestUtils/Binaries.cs +++ b/src/installer/tests/TestUtils/Binaries.cs @@ -13,7 +13,7 @@ namespace Microsoft.DotNet.CoreSetup.Test { public static class Binaries { - public static string GetExeFileNameForCurrentPlatform(string exeName) => + public static string GetExeName(string exeName) => exeName + (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".exe" : string.Empty); public static (string, string) GetSharedLibraryPrefixSuffix() @@ -35,7 +35,7 @@ public static string GetSharedLibraryFileNameForCurrentPlatform(string libraryNa public static class AppHost { - public static string FileName = GetExeFileNameForCurrentPlatform("apphost"); + public static string FileName = GetExeName("apphost"); public static string FilePath = Path.Combine(RepoDirectoriesProvider.Default.HostArtifacts, FileName); } @@ -50,7 +50,7 @@ public static class CoreClr public static class DotNet { - public static string FileName = GetExeFileNameForCurrentPlatform("dotnet"); + public static string FileName = GetExeName("dotnet"); public static string FilePath = Path.Combine(RepoDirectoriesProvider.Default.HostArtifacts, FileName); } @@ -82,7 +82,7 @@ public static class NetHost public static class SingleFileHost { - public static string FileName = GetExeFileNameForCurrentPlatform("singlefilehost"); + public static string FileName = GetExeName("singlefilehost"); public static string FilePath = Path.Combine(RepoDirectoriesProvider.Default.HostArtifacts, FileName); } diff --git a/src/installer/tests/TestUtils/SingleFileTestApp.cs b/src/installer/tests/TestUtils/SingleFileTestApp.cs index 69d61dd7f41926..2eadec40c2af44 100644 --- a/src/installer/tests/TestUtils/SingleFileTestApp.cs +++ b/src/installer/tests/TestUtils/SingleFileTestApp.cs @@ -82,7 +82,7 @@ public string Bundle(BundleOptions options, out Manifest manifest, Version? bund { string bundleDirectory = GetUniqueSubdirectory("bundle"); var bundler = new Bundler( - Binaries.GetExeFileNameForCurrentPlatform(AppName), + Binaries.GetExeName(AppName), bundleDirectory, options, targetFrameworkVersion: bundleVersion, diff --git a/src/installer/tests/TestUtils/SymbolicLinking.cs b/src/installer/tests/TestUtils/SymLink.cs similarity index 100% rename from src/installer/tests/TestUtils/SymbolicLinking.cs rename to src/installer/tests/TestUtils/SymLink.cs diff --git a/src/installer/tests/TestUtils/TestApp.cs b/src/installer/tests/TestUtils/TestApp.cs index ab5df6be6c700e..4d775f6a99ff7c 100644 --- a/src/installer/tests/TestUtils/TestApp.cs +++ b/src/installer/tests/TestUtils/TestApp.cs @@ -190,7 +190,7 @@ private void LoadAssets() { Directory.CreateDirectory(Location); AppDll = Path.Combine(Location, $"{AssemblyName}.dll"); - AppExe = Path.Combine(Location, Binaries.GetExeFileNameForCurrentPlatform(AssemblyName)); + AppExe = Path.Combine(Location, Binaries.GetExeName(AssemblyName)); DepsJson = Path.Combine(Location, $"{AssemblyName}.deps.json"); RuntimeConfigJson = Path.Combine(Location, $"{AssemblyName}.runtimeconfig.json"); RuntimeDevConfigJson = Path.Combine(Location, $"{AssemblyName}.runtimeconfig.dev.json"); From a8a293795422eda6eb5a7163bcf6bb66b88c39d3 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Sun, 17 Mar 2024 14:49:45 -0700 Subject: [PATCH 07/17] Fix up unix side as well --- src/native/corehost/hostmisc/pal.unix.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/native/corehost/hostmisc/pal.unix.cpp b/src/native/corehost/hostmisc/pal.unix.cpp index 527a5dabd07987..25de3a8585523a 100644 --- a/src/native/corehost/hostmisc/pal.unix.cpp +++ b/src/native/corehost/hostmisc/pal.unix.cpp @@ -946,10 +946,10 @@ bool pal::getenv(const pal::char_t* name, pal::string_t* recv) bool pal::fullpath(pal::string_t* path, bool skip_error_logging) { - return realpath2(path, skip_error_logging); + return realpath(path, skip_error_logging); } -bool pal::realpath2(pal::string_t* path, bool skip_error_logging) +bool pal::realpath(pal::string_t* path, bool skip_error_logging) { auto resolved = ::realpath(path->c_str(), nullptr); if (resolved == nullptr) From b0834e4bc8006d49104ef50cef48144a607bfc07 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Tue, 2 Apr 2024 13:28:58 -0700 Subject: [PATCH 08/17] Respond to PR comments --- .../PortableAppActivation.cs | 22 ------------------- .../HostActivation.Tests/SymbolicLinks.cs | 21 +++++++++--------- src/native/corehost/hostmisc/pal.h | 2 ++ 3 files changed, 12 insertions(+), 33 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index e7b517b78ab490..a254a4bea1466b 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -20,28 +20,6 @@ public PortableAppActivation(SharedTestState fixture) sharedTestState = fixture; } - [Fact] - public void Muxer_behind_symlink() - { - var app = sharedTestState.App.Copy(); - - var dotnetLoc = TestContext.BuiltDotNet.DotnetExecutablePath; - var appDll = app.AppDll; - var testDir = Directory.GetParent(app.Location).ToString(); - - using var symlink = new SymLink(Path.Combine(testDir, Binaries.GetExeName("dotnet")), dotnetLoc); - - var cmd = Command.Create(symlink.SrcPath, new[] { appDll }) - .CaptureStdOut() - .CaptureStdErr() - .EnvironmentVariable("DOTNET_SKIP_FIRST_TIME_EXPERIENCE", "1") - .MultilevelLookup(false); // Avoid looking at machine state by default - - cmd.Execute() - .Should().Pass() - .And.HaveStdOutContaining("Hello World"); - } - [Fact] public void Muxer_Default() { diff --git a/src/installer/tests/HostActivation.Tests/SymbolicLinks.cs b/src/installer/tests/HostActivation.Tests/SymbolicLinks.cs index 2d753a75b1a30c..d440d3eae2caa2 100644 --- a/src/installer/tests/HostActivation.Tests/SymbolicLinks.cs +++ b/src/installer/tests/HostActivation.Tests/SymbolicLinks.cs @@ -72,15 +72,14 @@ public void Run_apphost_behind_transitive_symlinks(string firstSymlinkRelativePa } } - //[Theory] - //[InlineData("a/b/SymlinkToFrameworkDependentApp")] - //[InlineData("a/SymlinkToFrameworkDependentApp")] - [Fact(Skip = "Currently failing in OSX with \"No such file or directory\" when running Command.Create. " + + [Theory] + [InlineData("a/b/SymlinkToFrameworkDependentApp")] + [InlineData("a/SymlinkToFrameworkDependentApp")] + [SkipOnPlatform(TestPlatforms.OSX, "Currently failing in OSX with \"No such file or directory\" when running Command.Create. " + "CI failing to use stat on symbolic links on Linux (permission denied).")] - [SkipOnPlatform(TestPlatforms.Windows, "Creating symbolic links requires administrative privilege on Windows, so skip test.")] - public void Run_framework_dependent_app_behind_symlink(/*string symlinkRelativePath*/) + public void Run_framework_dependent_app_behind_symlink(string symlinkRelativePath) { - var symlinkRelativePath = string.Empty; + symlinkRelativePath = Binaries.GetExeName(symlinkRelativePath); using (var testDir = TestArtifact.Create("symlink")) { @@ -97,14 +96,14 @@ public void Run_framework_dependent_app_behind_symlink(/*string symlinkRelativeP } } - [Fact(Skip = "Currently failing in OSX with \"No such file or directory\" when running Command.Create. " + - "CI failing to use stat on symbolic links on Linux (permission denied).")] - [SkipOnPlatform(TestPlatforms.Windows, "Creating symbolic links requires administrative privilege on Windows, so skip test.")] + [Fact] + [SkipOnPlatform(TestPlatforms.OSX, "Currently failing in OSX with \"No such file or directory\" when running Command.Create. " + + "CI failing to use stat on symbolic links on Linux (permission denied).")] public void Run_framework_dependent_app_with_runtime_behind_symlink() { using (var testDir = TestArtifact.Create("symlink")) { - var dotnetSymlink = Path.Combine(testDir.Location, "dotnet"); + var dotnetSymlink = Path.Combine(testDir.Location, Binaries.GetExeName("dotnet")); using var symlink = new SymLink(dotnetSymlink, TestContext.BuiltDotNet.BinPath); Command.Create(sharedTestState.FrameworkDependentApp.AppExe) diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index eeb90f79cfbbbd..2382f4082dd538 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -279,7 +279,9 @@ namespace pal void* mmap_copy_on_write(const string_t& path, size_t* length = nullptr); bool touch_file(const string_t& path); + // Realpath resolves a fully-qualified path to the target. It always resolves through symlinks. bool realpath(string_t* path, bool skip_error_logging = false); + // Fullpath resolves a fully-qualified path to the target. It may resolve through symlinks, depending on platform. bool fullpath(string_t* path, bool skip_error_logging = false); bool file_exists(const string_t& path); inline bool directory_exists(const string_t& path) { return file_exists(path); } From 02b1e3ee022d12e24cb6f73a3affce344d0b4d41 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Tue, 2 Apr 2024 13:54:06 -0700 Subject: [PATCH 09/17] Apply suggestions from code review Co-authored-by: Elinor Fung --- src/native/corehost/corehost.cpp | 4 ++-- src/native/corehost/hostmisc/pal.windows.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/native/corehost/corehost.cpp b/src/native/corehost/corehost.cpp index 190fcc0118d915..7a47ff0bc77ef4 100644 --- a/src/native/corehost/corehost.cpp +++ b/src/native/corehost/corehost.cpp @@ -101,9 +101,9 @@ int exe_start(const int argc, const pal::char_t* argv[]) { pal::initialize_createdump(); + // Use realpath to find the path of the host, resolving any symlinks. + // hostfxr (for dotnet) and the app dll (for apphost) are found relative to the host. pal::string_t host_path; - // Use realpath to find the path of the host through symlinks, since hostfxr will be - // next to the target if (!pal::get_own_executable_path(&host_path) || !pal::realpath(&host_path)) { trace::error(_X("Failed to resolve full path of the current executable [%s]"), host_path.c_str()); diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index 65c6ac4a8116b1..85cd297b123239 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -738,7 +738,7 @@ bool pal::realpath(pal::string_t* path, bool skip_error_logging) } // Use CreateFileW + GetFinalPathNameByHandleW to resolve symlinks - +https://learn.microsoft.com/windows/win32/fileio/symbolic-link-effects-on-file-systems-functions#createfile-and-createfiletransacted SmartHandle file( ::CreateFileW( path->c_str(), From db526905e5972b77c28d110beaeaedea8ce1b5ae Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Tue, 2 Apr 2024 13:59:44 -0700 Subject: [PATCH 10/17] Respond to PR comments --- src/native/corehost/fxr/fx_muxer.cpp | 12 ++++++------ src/native/corehost/host_startup_info.cpp | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/native/corehost/fxr/fx_muxer.cpp b/src/native/corehost/fxr/fx_muxer.cpp index a85cc807d5bb9b..11374a4c70a3e8 100644 --- a/src/native/corehost/fxr/fx_muxer.cpp +++ b/src/native/corehost/fxr/fx_muxer.cpp @@ -221,14 +221,14 @@ void get_runtime_config_paths_from_app(const pal::string_t& app, pal::string_t* get_runtime_config_paths(path, name, cfg, dev_cfg); } -// Convert "path" to realpath (merging working dir if needed) and append to "realpaths" out param. -void append_probe_realpath(const pal::string_t& path, std::vector* realpaths, const pal::string_t& tfm) +// Convert "path" to fullpath (merging working dir if needed) and append to "fullpaths" out param. +void append_probe_fullpath(const pal::string_t& path, std::vector* fullpaths, const pal::string_t& tfm) { pal::string_t probe_path = path; if (pal::fullpath(&probe_path, true)) { - realpaths->push_back(probe_path); + fullpaths->push_back(probe_path); } else { @@ -251,7 +251,7 @@ void append_probe_realpath(const pal::string_t& path, std::vector if (pal::fullpath(&probe_path, true)) { - realpaths->push_back(probe_path); + fullpaths->push_back(probe_path); } else { @@ -348,7 +348,7 @@ namespace std::vector probe_realpaths; for (const auto& path : specified_probing_paths) { - append_probe_realpath(path, &probe_realpaths, tfm); + append_probe_fullpath(path, &probe_realpaths, tfm); } // Each framework can add probe paths @@ -356,7 +356,7 @@ namespace { for (const auto& path : fx->get_runtime_config().get_probe_paths()) { - append_probe_realpath(path, &probe_realpaths, tfm); + append_probe_fullpath(path, &probe_realpaths, tfm); } } diff --git a/src/native/corehost/host_startup_info.cpp b/src/native/corehost/host_startup_info.cpp index e07e5785b77826..8ad4dadc96ff29 100644 --- a/src/native/corehost/host_startup_info.cpp +++ b/src/native/corehost/host_startup_info.cpp @@ -15,7 +15,7 @@ host_startup_info_t::host_startup_info_t( , dotnet_root(dotnet_root_value) , app_path(app_path_value) {} -// Determine if string is a valid path, and if so then fix up by using realpath() +// Determine if string is a valid path, and if so then fix up by using fullpath() bool get_path_from_argv(pal::string_t *path) { // Assume all paths will have at least one separator. We want to detect path vs. file before calling realpath From 229302b9c52b364c6306d073edde7a376d4e390b Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Tue, 2 Apr 2024 14:26:28 -0700 Subject: [PATCH 11/17] Move up error handling --- src/native/corehost/hostmisc/pal.windows.cpp | 21 +++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index 85cd297b123239..ca1edb051e29dd 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -762,7 +762,12 @@ bool pal::realpath(pal::string_t* path, bool skip_error_logging) file.release(); if (ERROR_ACCESS_DENIED != error) { - goto invalidPath; + if (!skip_error_logging) + { + trace::error(_X("Error resolving full path [%s]"), path->c_str()); + trace::error(_X("GetLastError(): [%s]"), ::GetLastError()); + return false; + } } } else @@ -784,7 +789,12 @@ bool pal::realpath(pal::string_t* path, bool skip_error_logging) if (size == 0) { - goto invalidPath; + if (!skip_error_logging) + { + trace::error(_X("Error resolving full path [%s]"), path->c_str()); + trace::error(_X("GetLastError(): [%s]"), ::GetLastError()); + return false; + } } } @@ -802,13 +812,6 @@ bool pal::realpath(pal::string_t* path, bool skip_error_logging) // If the above fails, fall back to fullpath return pal::fullpath(path, skip_error_logging); - -invalidPath: - if (!skip_error_logging) - { - trace::error(_X("Error resolving full path [%s]"), path->c_str()); - } - return false; } bool pal::fullpath(string_t* path, bool skip_error_logging) From 11ec1a8b11c275ba48bc9a829b5559050ebd94ec Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Wed, 17 Apr 2024 16:11:07 -0700 Subject: [PATCH 12/17] Fix up more names --- src/native/corehost/fxr/fx_muxer.cpp | 24 +++++++++---------- .../fxr/standalone/hostpolicy_resolver.cpp | 14 +++++------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/native/corehost/fxr/fx_muxer.cpp b/src/native/corehost/fxr/fx_muxer.cpp index 11374a4c70a3e8..fc06d960eaef4a 100644 --- a/src/native/corehost/fxr/fx_muxer.cpp +++ b/src/native/corehost/fxr/fx_muxer.cpp @@ -337,18 +337,18 @@ namespace return host_mode_t::muxer; } - std::vector get_probe_realpaths( + std::vector get_probe_fullpaths( const fx_definition_vector_t &fx_definitions, const std::vector &specified_probing_paths) { // The tfm is taken from the app. pal::string_t tfm = get_app(fx_definitions).get_runtime_config().get_tfm(); - // Append specified probe paths first and then config file probe paths into realpaths. - std::vector probe_realpaths; + // Append specified probe paths first and then config file probe paths into fullpaths. + std::vector probe_fullpaths; for (const auto& path : specified_probing_paths) { - append_probe_fullpath(path, &probe_realpaths, tfm); + append_probe_fullpath(path, &probe_fullpaths, tfm); } // Each framework can add probe paths @@ -356,11 +356,11 @@ namespace { for (const auto& path : fx->get_runtime_config().get_probe_paths()) { - append_probe_fullpath(path, &probe_realpaths, tfm); + append_probe_fullpath(path, &probe_fullpaths, tfm); } } - return probe_realpaths; + return probe_fullpaths; } int get_init_info_for_app( @@ -485,17 +485,17 @@ namespace const known_options opts_probe_path = known_options::additional_probing_path; std::vector spec_probe_paths = opts.count(opts_probe_path) ? opts.find(opts_probe_path)->second : std::vector(); - std::vector probe_realpaths = get_probe_realpaths(fx_definitions, spec_probe_paths); + std::vector probe_fullpaths = get_probe_fullpaths(fx_definitions, spec_probe_paths); trace::verbose(_X("Executing as a %s app as per config file [%s]"), (is_framework_dependent ? _X("framework-dependent") : _X("self-contained")), app_config.get_path().c_str()); - if (!hostpolicy_resolver::try_get_dir(mode, host_info.dotnet_root, fx_definitions, app_candidate, deps_file, probe_realpaths, &hostpolicy_dir)) + if (!hostpolicy_resolver::try_get_dir(mode, host_info.dotnet_root, fx_definitions, app_candidate, deps_file, probe_fullpaths, &hostpolicy_dir)) { return StatusCode::CoreHostLibMissingFailure; } - init.reset(new corehost_init_t(host_command, host_info, deps_file, additional_deps_serialized, probe_realpaths, mode, fx_definitions, additional_properties)); + init.reset(new corehost_init_t(host_command, host_info, deps_file, additional_deps_serialized, probe_fullpaths, mode, fx_definitions, additional_properties)); return StatusCode::Success; } @@ -623,19 +623,19 @@ namespace if (rc != StatusCode::Success) return rc; - const std::vector probe_realpaths = get_probe_realpaths(fx_definitions, std::vector() /* specified_probing_paths */); + const std::vector probe_fullpaths = get_probe_fullpaths(fx_definitions, std::vector() /* specified_probing_paths */); trace::verbose(_X("Libhost loading occurring for a framework-dependent component per config file [%s]"), app_config.get_path().c_str()); const pal::string_t deps_file; - if (!hostpolicy_resolver::try_get_dir(mode, host_info.dotnet_root, fx_definitions, host_info.app_path, deps_file, probe_realpaths, &hostpolicy_dir)) + if (!hostpolicy_resolver::try_get_dir(mode, host_info.dotnet_root, fx_definitions, host_info.app_path, deps_file, probe_fullpaths, &hostpolicy_dir)) { return StatusCode::CoreHostLibMissingFailure; } const pal::string_t additional_deps_serialized; const std::vector> additional_properties; - init.reset(new corehost_init_t(pal::string_t{}, host_info, deps_file, additional_deps_serialized, probe_realpaths, mode, fx_definitions, additional_properties)); + init.reset(new corehost_init_t(pal::string_t{}, host_info, deps_file, additional_deps_serialized, probe_fullpaths, mode, fx_definitions, additional_properties)); return StatusCode::Success; } diff --git a/src/native/corehost/fxr/standalone/hostpolicy_resolver.cpp b/src/native/corehost/fxr/standalone/hostpolicy_resolver.cpp index 1a32e5a6a1b12f..4e75400a04a81b 100644 --- a/src/native/corehost/fxr/standalone/hostpolicy_resolver.cpp +++ b/src/native/corehost/fxr/standalone/hostpolicy_resolver.cpp @@ -110,15 +110,15 @@ namespace * Given a version and probing paths, find if package layout * directory containing hostpolicy exists. */ - bool resolve_hostpolicy_dir_from_probe_paths(const pal::string_t& version, const std::vector& probe_realpaths, pal::string_t* candidate) + bool resolve_hostpolicy_dir_from_probe_paths(const pal::string_t& version, const std::vector& probe_fullpaths, pal::string_t* candidate) { - if (probe_realpaths.empty() || version.empty()) + if (probe_fullpaths.empty() || version.empty()) { return false; } // Check if the package relative directory containing hostpolicy exists. - for (const auto& probe_path : probe_realpaths) + for (const auto& probe_path : probe_fullpaths) { trace::verbose(_X("Considering %s to probe for %s"), probe_path.c_str(), LIBHOSTPOLICY_NAME); if (to_hostpolicy_package_dir(probe_path, version, candidate)) @@ -129,8 +129,8 @@ namespace // Print detailed message about the file not found in the probe paths. trace::error(_X("Could not find required library %s in %d probing paths:"), - LIBHOSTPOLICY_NAME, probe_realpaths.size()); - for (const auto& path : probe_realpaths) + LIBHOSTPOLICY_NAME, probe_fullpaths.size()); + for (const auto& path : probe_fullpaths) { trace::error(_X(" %s"), path.c_str()); } @@ -233,7 +233,7 @@ bool hostpolicy_resolver::try_get_dir( const fx_definition_vector_t& fx_definitions, const pal::string_t& app_candidate, const pal::string_t& specified_deps_file, - const std::vector& probe_realpaths, + const std::vector& probe_fullpaths, pal::string_t* impl_dir) { bool is_framework_dependent = get_app(fx_definitions).get_runtime_config().get_is_framework_dependent(); @@ -299,7 +299,7 @@ bool hostpolicy_resolver::try_get_dir( // Start probing for hostpolicy in the specified probe paths. pal::string_t candidate; - if (resolve_hostpolicy_dir_from_probe_paths(version, probe_realpaths, &candidate)) + if (resolve_hostpolicy_dir_from_probe_paths(version, probe_fullpaths, &candidate)) { impl_dir->assign(candidate); return true; From ba83fc187ff4c555d9c1f0abd28fa575fba10c62 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Thu, 18 Apr 2024 10:45:56 -0700 Subject: [PATCH 13/17] Missed a comment --- src/native/corehost/host_startup_info.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/native/corehost/host_startup_info.cpp b/src/native/corehost/host_startup_info.cpp index 8ad4dadc96ff29..d2fdcbe3585501 100644 --- a/src/native/corehost/host_startup_info.cpp +++ b/src/native/corehost/host_startup_info.cpp @@ -18,8 +18,8 @@ host_startup_info_t::host_startup_info_t( // Determine if string is a valid path, and if so then fix up by using fullpath() bool get_path_from_argv(pal::string_t *path) { - // Assume all paths will have at least one separator. We want to detect path vs. file before calling realpath - // because realpath will expand a filename into a full path containing the current directory which may be + // Assume all paths will have at least one separator. We want to detect path vs. file before calling fullpath + // because fullpath will expand a filename into a full path containing the current directory which may be // the wrong location when filename ends up being found in %PATH% and not the current directory. if (path->find(DIR_SEPARATOR) != pal::string_t::npos) { From 96508d554f254d3250dab6e4118162afb8fb4dcb Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Thu, 18 Apr 2024 11:28:35 -0700 Subject: [PATCH 14/17] Typo --- src/native/corehost/hostmisc/pal.windows.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index ca1edb051e29dd..4ae83ea9cc84a8 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -738,7 +738,7 @@ bool pal::realpath(pal::string_t* path, bool skip_error_logging) } // Use CreateFileW + GetFinalPathNameByHandleW to resolve symlinks -https://learn.microsoft.com/windows/win32/fileio/symbolic-link-effects-on-file-systems-functions#createfile-and-createfiletransacted + // https://learn.microsoft.com/windows/win32/fileio/symbolic-link-effects-on-file-systems-functions#createfile-and-createfiletransacted SmartHandle file( ::CreateFileW( path->c_str(), From 3697bc88e1a123293511d05cf17816ff39871196 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Thu, 6 Jun 2024 14:24:19 -0700 Subject: [PATCH 15/17] Remove unused using --- .../tests/HostActivation.Tests/PortableAppActivation.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index a254a4bea1466b..fa19efd53121ce 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -4,7 +4,6 @@ using System; using System.Diagnostics; using System.IO; -using System.Runtime.InteropServices; using Microsoft.DotNet.Cli.Build.Framework; using Xunit; From 13ddd84147dc859563aaa3f848fc934ed8c14a9b Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Mon, 29 Jul 2024 14:17:44 -0700 Subject: [PATCH 16/17] Apply suggestions from code review Co-authored-by: Elinor Fung --- src/native/corehost/hostmisc/pal.windows.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index 0ef08e2d0ebd8a..cc07426e8f60a0 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -820,8 +820,7 @@ bool pal::realpath(pal::string_t* path, bool skip_error_logging) { if (!skip_error_logging) { - trace::error(_X("Error resolving full path [%s]"), path->c_str()); - trace::error(_X("GetLastError(): [%s]"), ::GetLastError()); + trace::error(_X("Error resolving full path [%s]. Error code: %d"), path->c_str(), error); return false; } } @@ -847,8 +846,7 @@ bool pal::realpath(pal::string_t* path, bool skip_error_logging) { if (!skip_error_logging) { - trace::error(_X("Error resolving full path [%s]"), path->c_str()); - trace::error(_X("GetLastError(): [%s]"), ::GetLastError()); + trace::error(_X("Error resolving full path [%s]. Error code: %d"), path->c_str(), ::GetLastError()); return false; } } From fcc27e0217f5b5a7b1a34745f86f2bb4e68a4e27 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Mon, 29 Jul 2024 14:19:54 -0700 Subject: [PATCH 17/17] Respond to PR comments --- src/native/corehost/hostmisc/pal.h | 2 +- src/native/corehost/hostmisc/pal.windows.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/native/corehost/hostmisc/pal.h b/src/native/corehost/hostmisc/pal.h index 7aae9f35763d93..bde8446cc22cdf 100644 --- a/src/native/corehost/hostmisc/pal.h +++ b/src/native/corehost/hostmisc/pal.h @@ -281,7 +281,7 @@ namespace pal void* mmap_copy_on_write(const string_t& path, size_t* length = nullptr); bool touch_file(const string_t& path); - // Realpath resolves a fully-qualified path to the target. It always resolves through symlinks. + // Realpath resolves a fully-qualified path to the target. It always resolves through file symlinks (not necessarily directory symlinks). bool realpath(string_t* path, bool skip_error_logging = false); // Fullpath resolves a fully-qualified path to the target. It may resolve through symlinks, depending on platform. bool fullpath(string_t* path, bool skip_error_logging = false); diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index e53d91e05fd403..3a3db968b5bde4 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -817,7 +817,7 @@ bool pal::clr_palstring(const char* cstr, pal::string_t* out) typedef std::unique_ptr::type, decltype(&::CloseHandle)> SmartHandle; -// Like fullpath, but resolves symlinks. +// Like fullpath, but resolves file symlinks (note: not necessarily directory symlinks). bool pal::realpath(pal::string_t* path, bool skip_error_logging) { if (path->empty()) @@ -853,8 +853,8 @@ bool pal::realpath(pal::string_t* path, bool skip_error_logging) if (!skip_error_logging) { trace::error(_X("Error resolving full path [%s]. Error code: %d"), path->c_str(), error); - return false; } + return false; } } else @@ -879,8 +879,8 @@ bool pal::realpath(pal::string_t* path, bool skip_error_logging) if (!skip_error_logging) { trace::error(_X("Error resolving full path [%s]. Error code: %d"), path->c_str(), ::GetLastError()); - return false; } + return false; } }