From 2edc18369f12543bd1a3fcf6a870c1d3ecdc3cec Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Thu, 22 Jul 2021 14:55:51 -0700 Subject: [PATCH 1/6] Update runtimeconfig paths if we exceed MAX_PATH --- src/native/corehost/deps_format.cpp | 4 ++-- src/native/corehost/runtime_config.cpp | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/native/corehost/deps_format.cpp b/src/native/corehost/deps_format.cpp index 8ab0f22caff2c..ba02cbbd1e6bc 100644 --- a/src/native/corehost/deps_format.cpp +++ b/src/native/corehost/deps_format.cpp @@ -439,7 +439,7 @@ bool deps_json_t::has_package(const pal::string_t& name, const pal::string_t& ve bool deps_json_t::load(bool is_framework_dependent, const pal::string_t& deps_path, const rid_fallback_graph_t& rid_fallback_graph) { m_deps_file = deps_path; - m_file_exists = bundle::info_t::config_t::probe(deps_path) || pal::file_exists(deps_path); + m_file_exists = pal::realpath(&m_deps_file, true) || bundle::info_t::config_t::probe(m_deps_file); json_parser_t json; if (!m_file_exists) @@ -449,7 +449,7 @@ bool deps_json_t::load(bool is_framework_dependent, const pal::string_t& deps_pa return true; } - if (!json.parse_file(deps_path)) + if (!json.parse_file(m_deps_file)) { return false; } diff --git a/src/native/corehost/runtime_config.cpp b/src/native/corehost/runtime_config.cpp index 986c44c9bad36..f758ebc0e787e 100644 --- a/src/native/corehost/runtime_config.cpp +++ b/src/native/corehost/runtime_config.cpp @@ -334,9 +334,9 @@ 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::file_exists(m_dev_path)) + if (!pal::realpath(&m_dev_path, true)) { - // Not existing is valid. + // 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("Did not successfully parse the runtimeconfig.dev.json")); } - if (!bundle::info_t::config_t::probe(m_path) && !pal::file_exists(m_path)) + if (!pal::realpath(&m_path, true) && !bundle::info_t::config_t::probe(m_path)) { // Not existing is not an error. return true; From bc795aad1254180bf00643fcfaa0a35bc1981f14 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Fri, 23 Jul 2021 08:38:06 -0700 Subject: [PATCH 2/6] Add test --- .../PortableAppActivation.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index a2a0d963e1f09..579af205ff509 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -332,6 +332,36 @@ public void AppHost_FrameworkDependent_GlobalLocation_Succeeds(bool useRegistere } } + [Fact] + public void RuntimeConfig_FilePath_Breaks_MAX_PATH_Threshold() + { + var project = sharedTestState.PortableAppFixture_Published + .Copy(); + + var appExeName = Path.GetFileName(project.TestProject.AppExe); + var outputDir = project.TestProject.OutputDirectory; + + // Move the portable app to a path such that the length of the executable's fullpath + // is just 1 char behind MAX_PATH (260) so that the runtimeconfig(.dev).json files + // break this threshold. This will cause hostfxr to normalize these paths -- here we + // are checking that the updated paths are used. + var dirName = new string('a', 259 - outputDir.Length - appExeName.Length - 2); + var newDir = Path.Combine(outputDir, dirName); + var appExe = Path.Combine(newDir, appExeName); + Debug.Assert(appExe.Length == 259); + Directory.CreateDirectory(newDir); + foreach (var file in Directory.GetFiles(outputDir, "*.*", SearchOption.TopDirectoryOnly)) + File.Copy(file, Path.Combine(newDir, Path.GetFileName(file))); + + Command.Create(appExe) + .DotNetRoot(project.BuiltDotnet.BinPath) + .EnableTracingAndCaptureOutputs() + .MultilevelLookup(false) + .Execute() + .Should().Pass() + .And.HaveStdOutContaining("Hello World"); + } + [Fact] public void ComputedTPADoesntEndWithPathSeparator() { From e4468c956f2974d8353a5cdf8a0e26e66866a422 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Mon, 26 Jul 2021 08:59:55 -0700 Subject: [PATCH 3/6] PR feedback --- .../tests/HostActivation.Tests/PortableAppActivation.cs | 7 ++++--- src/native/corehost/deps_format.cpp | 2 +- src/native/corehost/runtime_config.cpp | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index 579af205ff509..46eda41113d03 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -345,13 +345,14 @@ public void RuntimeConfig_FilePath_Breaks_MAX_PATH_Threshold() // is just 1 char behind MAX_PATH (260) so that the runtimeconfig(.dev).json files // break this threshold. This will cause hostfxr to normalize these paths -- here we // are checking that the updated paths are used. - var dirName = new string('a', 259 - outputDir.Length - appExeName.Length - 2); - var newDir = Path.Combine(outputDir, dirName); + var tmp = Path.GetTempPath(); + var dirName = new string('a', 259 - tmp.Length - appExeName.Length - 1); + var newDir = Path.Combine(tmp, dirName); var appExe = Path.Combine(newDir, appExeName); Debug.Assert(appExe.Length == 259); Directory.CreateDirectory(newDir); foreach (var file in Directory.GetFiles(outputDir, "*.*", SearchOption.TopDirectoryOnly)) - File.Copy(file, Path.Combine(newDir, Path.GetFileName(file))); + File.Copy(file, Path.Combine(newDir, Path.GetFileName(file)), true); Command.Create(appExe) .DotNetRoot(project.BuiltDotnet.BinPath) diff --git a/src/native/corehost/deps_format.cpp b/src/native/corehost/deps_format.cpp index ba02cbbd1e6bc..f720f9df57b8a 100644 --- a/src/native/corehost/deps_format.cpp +++ b/src/native/corehost/deps_format.cpp @@ -439,7 +439,7 @@ bool deps_json_t::has_package(const pal::string_t& name, const pal::string_t& ve bool deps_json_t::load(bool is_framework_dependent, const pal::string_t& deps_path, const rid_fallback_graph_t& rid_fallback_graph) { m_deps_file = deps_path; - m_file_exists = pal::realpath(&m_deps_file, true) || bundle::info_t::config_t::probe(m_deps_file); + m_file_exists = !m_deps_file.empty() && (pal::realpath(&m_deps_file, true) || bundle::info_t::config_t::probe(deps_path)); json_parser_t json; if (!m_file_exists) diff --git a/src/native/corehost/runtime_config.cpp b/src/native/corehost/runtime_config.cpp index f758ebc0e787e..4b0b0d5eba558 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 (m_dev_path.empty() || !pal::realpath(&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("Did not successfully parse the runtimeconfig.dev.json")); } - if (!pal::realpath(&m_path, true) && !bundle::info_t::config_t::probe(m_path)) + if (m_path.empty() || (!pal::realpath(&m_path, true) && !bundle::info_t::config_t::probe(m_path))) { // Not existing is not an error. return true; From 072492da7112260a1dc6fb7ba511955500407088 Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Mon, 26 Jul 2021 12:48:41 -0700 Subject: [PATCH 4/6] Get apphost for all platforms --- .../tests/HostActivation.Tests/PortableAppActivation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs index 46eda41113d03..14580eec0f795 100644 --- a/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs +++ b/src/installer/tests/HostActivation.Tests/PortableAppActivation.cs @@ -730,7 +730,7 @@ public SharedTestState() PortableAppFixture_Published = new TestProjectFixture("PortableApp", RepoDirectories) .EnsureRestored() - .PublishProject(); + .PublishProject(extraArgs: "/p:UseAppHost=true"); MockApp = new TestApp(SharedFramework.CalculateUniqueTestDirectory(Path.Combine(TestArtifact.TestArtifactsPath, "portableAppActivation")), "App"); Directory.CreateDirectory(MockApp.Location); From 3c01e0d1bdff189e9dc8cd2568efe6796e91902c Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Mon, 26 Jul 2021 15:59:47 -0700 Subject: [PATCH 5/6] Probe bundle first Fix return value of realpath --- src/native/corehost/deps_format.cpp | 2 +- src/native/corehost/hostmisc/pal.windows.cpp | 4 ++-- src/native/corehost/runtime_config.cpp | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/native/corehost/deps_format.cpp b/src/native/corehost/deps_format.cpp index f720f9df57b8a..fdfc1869ca8b0 100644 --- a/src/native/corehost/deps_format.cpp +++ b/src/native/corehost/deps_format.cpp @@ -439,7 +439,7 @@ bool deps_json_t::has_package(const pal::string_t& name, const pal::string_t& ve bool deps_json_t::load(bool is_framework_dependent, const pal::string_t& deps_path, const rid_fallback_graph_t& rid_fallback_graph) { m_deps_file = deps_path; - m_file_exists = !m_deps_file.empty() && (pal::realpath(&m_deps_file, true) || bundle::info_t::config_t::probe(deps_path)); + m_file_exists = bundle::info_t::config_t::probe(deps_path) || pal::realpath(&m_deps_file, true); json_parser_t json; if (!m_file_exists) diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index c992f92da8294..ea1934007fae3 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -648,8 +648,8 @@ bool pal::realpath(string_t* path, bool skip_error_logging) if (LongFile::IsNormalized(*path)) { WIN32_FILE_ATTRIBUTE_DATA data; - if (path->empty() // An empty path doesn't exist - || GetFileAttributesExW(path->c_str(), GetFileExInfoStandard, &data) != 0) + if (!path->empty() // An empty path doesn't exist + && GetFileAttributesExW(path->c_str(), GetFileExInfoStandard, &data) != 0) { return true; } diff --git a/src/native/corehost/runtime_config.cpp b/src/native/corehost/runtime_config.cpp index 4b0b0d5eba558..fe6a0546cff92 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 (m_dev_path.empty() || !pal::realpath(&m_dev_path, true)) + if (!pal::realpath(&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("Did not successfully parse the runtimeconfig.dev.json")); } - if (m_path.empty() || (!pal::realpath(&m_path, true) && !bundle::info_t::config_t::probe(m_path))) + if (!bundle::info_t::config_t::probe(m_path) && !pal::realpath(&m_path, true)) { // Not existing is not an error. return true; From 3c2f128bf39b4b594891de181a7ca8f38a42ecaf Mon Sep 17 00:00:00 2001 From: Mateo Torres Ruiz Date: Tue, 27 Jul 2021 09:40:50 -0700 Subject: [PATCH 6/6] Check for empty path upfront --- src/native/corehost/hostmisc/pal.windows.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/native/corehost/hostmisc/pal.windows.cpp b/src/native/corehost/hostmisc/pal.windows.cpp index ea1934007fae3..87a5508badbf5 100644 --- a/src/native/corehost/hostmisc/pal.windows.cpp +++ b/src/native/corehost/hostmisc/pal.windows.cpp @@ -645,11 +645,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) { + if (path->empty()) + { + return false; + } + if (LongFile::IsNormalized(*path)) { WIN32_FILE_ATTRIBUTE_DATA data; - if (!path->empty() // An empty path doesn't exist - && GetFileAttributesExW(path->c_str(), GetFileExInfoStandard, &data) != 0) + if (GetFileAttributesExW(path->c_str(), GetFileExInfoStandard, &data) != 0) { return true; } @@ -713,11 +717,6 @@ bool pal::realpath(string_t* path, bool skip_error_logging) bool pal::file_exists(const string_t& path) { - if (path.empty()) - { - return false; - } - string_t tmp(path); return pal::realpath(&tmp, true); }