Skip to content

Commit 214743e

Browse files
elinor-fungelinor-fung
committed
Merged PR 49479: [internal/release/9.0] Fix issue where libhost scenarios (COM, C++/CLI, custom component host) could try to load coreclr from CWD
There is a fallback for apps with no .deps.json where the host will consider the app directory for loading coreclr. In component hosting scenarios, we do not have an app path / directory. We were incorrectly going down the path of looking for coreclr next to the empty app directory, which resulted in looking in the current directory. This change skips that path for libhost scenarios. It also adds checks that the paths we determine for loading coreclr, hostpolicy, and hostfxr are absolute.
1 parent e36e4d1 commit 214743e

File tree

9 files changed

+133
-6
lines changed

9 files changed

+133
-6
lines changed

‎src/installer/tests/HostActivation.Tests/NativeHosting/Comhost.cs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,35 @@ public void ActivateClass_IgnoreAppLocalHostFxr()
111111
}
112112
}
113113

114+
[Fact]
115+
public void ActivateClass_IgnoreWorkingDirectory()
116+
{
117+
using (TestArtifact cwd = TestArtifact.Create("cwd"))
118+
{
119+
// Validate that hosting components in the working directory will not be used
120+
File.Copy(Binaries.CoreClr.MockPath, Path.Combine(cwd.Location, Binaries.CoreClr.FileName));
121+
File.Copy(Binaries.HostFxr.MockPath_5_0, Path.Combine(cwd.Location, Binaries.HostFxr.FileName));
122+
File.Copy(Binaries.HostPolicy.MockPath, Path.Combine(cwd.Location, Binaries.HostPolicy.FileName));
123+
124+
string[] args = {
125+
"comhost",
126+
"synchronous",
127+
"1",
128+
sharedState.ComHostPath,
129+
sharedState.ClsidString
130+
};
131+
sharedState.CreateNativeHostCommand(args, TestContext.BuiltDotNet.BinPath)
132+
.WorkingDirectory(cwd.Location)
133+
.Execute()
134+
.Should().Pass()
135+
.And.HaveStdOutContaining("New instance of Server created")
136+
.And.HaveStdOutContaining($"Activation of {sharedState.ClsidString} succeeded.")
137+
.And.ResolveHostFxr(TestContext.BuiltDotNet)
138+
.And.ResolveHostPolicy(TestContext.BuiltDotNet)
139+
.And.ResolveCoreClr(TestContext.BuiltDotNet);
140+
}
141+
}
142+
114143
[Fact]
115144
public void ActivateClass_ValidateIErrorInfoResult()
116145
{

‎src/installer/tests/HostActivation.Tests/NativeHosting/Ijwhost.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,32 @@ public void LoadLibrary_ContextConfig(bool load_isolated)
8989
}
9090
}
9191

92+
[Fact]
93+
public void LoadLibrary_IgnoreWorkingDirectory()
94+
{
95+
using (TestArtifact cwd = TestArtifact.Create("cwd"))
96+
{
97+
// Validate that hosting components in the working directory will not be used
98+
File.Copy(Binaries.CoreClr.MockPath, Path.Combine(cwd.Location, Binaries.CoreClr.FileName));
99+
File.Copy(Binaries.HostFxr.MockPath_5_0, Path.Combine(cwd.Location, Binaries.HostFxr.FileName));
100+
File.Copy(Binaries.HostPolicy.MockPath, Path.Combine(cwd.Location, Binaries.HostPolicy.FileName));
101+
102+
string [] args = {
103+
"ijwhost",
104+
sharedState.IjwApp.AppDll,
105+
"NativeEntryPoint"
106+
};
107+
sharedState.CreateNativeHostCommand(args, TestContext.BuiltDotNet.BinPath)
108+
.WorkingDirectory(cwd.Location)
109+
.Execute()
110+
.Should().Pass()
111+
.And.HaveStdOutContaining("[C++/CLI] NativeEntryPoint: calling managed class")
112+
.And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"Default\" System.Runtime.Loader.DefaultAssemblyLoadContext")
113+
.And.ResolveHostFxr(TestContext.BuiltDotNet)
114+
.And.ResolveHostPolicy(TestContext.BuiltDotNet)
115+
.And.ResolveCoreClr(TestContext.BuiltDotNet);
116+
}
117+
}
92118

93119
[Fact]
94120
public void LoadLibraryWithoutRuntimeConfigButActiveRuntime()

‎src/installer/tests/HostActivation.Tests/NativeHosting/LoadAssemblyAndGetFunctionPointer.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Generic;
5+
using System.IO;
56
using System.Linq;
67

78
using Microsoft.DotNet.Cli.Build.Framework;
@@ -231,6 +232,37 @@ public void CallDelegateOnComponentContext_UnhandledException()
231232
.And.ExecuteFunctionPointerWithException(entryPoint, 1);
232233
}
233234

235+
[Fact]
236+
public void CallDelegateOnComponentContext_IgnoreWorkingDirectory()
237+
{
238+
using (TestArtifact cwd = TestArtifact.Create("cwd"))
239+
{
240+
// Validate that hosting components in the working directory will not be used
241+
File.Copy(Binaries.CoreClr.MockPath, Path.Combine(cwd.Location, Binaries.CoreClr.FileName));
242+
File.Copy(Binaries.HostPolicy.MockPath, Path.Combine(cwd.Location, Binaries.HostPolicy.FileName));
243+
244+
var component = sharedState.Component;
245+
string[] args =
246+
{
247+
ComponentLoadAssemblyAndGetFunctionPointerArg,
248+
sharedState.HostFxrPath,
249+
component.RuntimeConfigJson,
250+
component.AppDll,
251+
sharedState.ComponentTypeName,
252+
sharedState.ComponentEntryPoint1,
253+
};
254+
255+
sharedState.CreateNativeHostCommand(args, sharedState.DotNetRoot)
256+
.WorkingDirectory(cwd.Location)
257+
.Execute()
258+
.Should().Pass()
259+
.And.InitializeContextForConfig(component.RuntimeConfigJson)
260+
.And.ExecuteFunctionPointer(sharedState.ComponentEntryPoint1, 1, 1)
261+
.And.ResolveHostPolicy(TestContext.BuiltDotNet)
262+
.And.ResolveCoreClr(TestContext.BuiltDotNet);
263+
}
264+
}
265+
234266
public class SharedTestState : SharedTestStateBase
235267
{
236268
public string HostFxrPath { get; }

‎src/installer/tests/HostActivation.Tests/NativeHosting/FunctionPointerResultExtensions.cs renamed to ‎src/installer/tests/HostActivation.Tests/NativeHosting/NativeHostingResultExtensions.cs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
5+
using System.IO;
56

67
namespace Microsoft.DotNet.CoreSetup.Test.HostActivation.NativeHosting
78
{
8-
internal static class FunctionPointerResultExtensions
9+
internal static class NativeHostingResultExtensions
910
{
1011
public static FluentAssertions.AndConstraint<CommandResultAssertions> ExecuteFunctionPointer(this CommandResultAssertions assertion, string methodName, int callCount, int returnValue)
1112
{
@@ -47,5 +48,21 @@ public static FluentAssertions.AndConstraint<CommandResultAssertions> ExecuteWit
4748
{
4849
return assertion.HaveStdOutContaining($"{assemblyName}: Location = '{location}'");
4950
}
51+
52+
public static FluentAssertions.AndConstraint<CommandResultAssertions> ResolveHostFxr(this CommandResultAssertions assertion, Microsoft.DotNet.Cli.Build.DotNetCli dotnet)
53+
{
54+
return assertion.HaveStdErrContaining($"Resolved fxr [{dotnet.GreatestVersionHostFxrFilePath}]");
55+
}
56+
57+
public static FluentAssertions.AndConstraint<CommandResultAssertions> ResolveHostPolicy(this CommandResultAssertions assertion, Microsoft.DotNet.Cli.Build.DotNetCli dotnet)
58+
{
59+
return assertion.HaveStdErrContaining($"{Binaries.HostPolicy.FileName} directory is [{dotnet.GreatestVersionSharedFxPath}]");
60+
}
61+
62+
public static FluentAssertions.AndConstraint<CommandResultAssertions> ResolveCoreClr(this CommandResultAssertions assertion, Microsoft.DotNet.Cli.Build.DotNetCli dotnet)
63+
{
64+
return assertion.HaveStdErrContaining($"CoreCLR path = '{Path.Combine(dotnet.GreatestVersionSharedFxPath, Binaries.CoreClr.FileName)}'")
65+
.And.HaveStdErrContaining($"CoreCLR dir = '{dotnet.GreatestVersionSharedFxPath}{Path.DirectorySeparatorChar}'");
66+
}
5067
}
5168
}

‎src/native/corehost/apphost/standalone/hostfxr_resolver.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,11 @@ hostfxr_resolver_t::hostfxr_resolver_t(const pal::string_t& app_root)
118118
{
119119
m_status_code = StatusCode::CoreHostLibMissingFailure;
120120
}
121+
else if (!pal::is_path_rooted(m_fxr_path))
122+
{
123+
// We should always be loading hostfxr from an absolute path
124+
m_status_code = StatusCode::CoreHostLibMissingFailure;
125+
}
121126
else if (pal::load_library(&m_fxr_path, &m_hostfxr_dll))
122127
{
123128
m_status_code = StatusCode::Success;

‎src/native/corehost/fxr/standalone/hostpolicy_resolver.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,10 @@ int hostpolicy_resolver::load(
180180
return StatusCode::CoreHostLibMissingFailure;
181181
}
182182

183+
// We should always be loading hostpolicy from an absolute path
184+
if (!pal::is_path_rooted(host_path))
185+
return StatusCode::CoreHostLibMissingFailure;
186+
183187
// Load library
184188
// We expect to leak hostpolicy - just as we do not unload coreclr, we do not unload hostpolicy
185189
if (!pal::load_library(&host_path, &g_hostpolicy))

‎src/native/corehost/fxr_resolver.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ int load_fxr_and_get_delegate(hostfxr_delegate_type type, THostPathToConfigCallb
5555
return StatusCode::CoreHostLibMissingFailure;
5656
}
5757

58+
// We should always be loading hostfxr from an absolute path
59+
if (!pal::is_path_rooted(fxr_path))
60+
return StatusCode::CoreHostLibMissingFailure;
61+
5862
// Load library
5963
if (!pal::load_library(&fxr_path, &fxr))
6064
{

‎src/native/corehost/hostpolicy/deps_resolver.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -830,15 +830,21 @@ bool deps_resolver_t::resolve_probe_dirs(
830830
}
831831
}
832832

833-
// If the deps file is missing add known locations.
834-
if (!get_app_deps().exists())
833+
// If the deps file is missing for the app, add known locations.
834+
// For libhost scenarios, there is no app or app path
835+
if (m_host_mode != host_mode_t::libhost && !get_app_deps().exists())
835836
{
837+
assert(!m_app_dir.empty());
838+
836839
// App local path
837840
add_unique_path(asset_type, m_app_dir, &items, output, &non_serviced, core_servicing);
838841

839-
// deps_resolver treats being able to get the coreclr path as optional, so we ignore the return value here.
840-
// The caller is responsible for checking whether coreclr path is set and handling as appropriate.
841-
(void) file_exists_in_dir(m_app_dir, LIBCORECLR_NAME, &m_coreclr_path);
842+
if (m_coreclr_path.empty())
843+
{
844+
// deps_resolver treats being able to get the coreclr path as optional, so we ignore the return value here.
845+
// The caller is responsible for checking whether coreclr path is set and handling as appropriate.
846+
(void) file_exists_in_dir(m_app_dir, LIBCORECLR_NAME, &m_coreclr_path);
847+
}
842848
}
843849

844850
// Handle any additional deps.json that were specified.

‎src/native/corehost/hostpolicy/standalone/coreclr_resolver.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ bool coreclr_resolver_t::resolve_coreclr(const pal::string_t& libcoreclr_path, c
1313
pal::string_t coreclr_dll_path(libcoreclr_path);
1414
append_path(&coreclr_dll_path, LIBCORECLR_NAME);
1515

16+
// We should always be loading coreclr from an absolute path
17+
if (!pal::is_path_rooted(coreclr_dll_path))
18+
return false;
19+
1620
if (!pal::load_library(&coreclr_dll_path, &coreclr_resolver_contract.coreclr))
1721
{
1822
return false;

0 commit comments

Comments
 (0)