Skip to content

Commit 5b9ae97

Browse files
vseanreesermsftelinor-fungMirroring
authored
Merged PR 49480: [internal/release/8.0] Fix issue where libhost scenarios (COM, C++/CLI, custom component host) could try to load coreclr from CWD (#116498)
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. Co-authored-by: Elinor Fung <[email protected]> Co-authored-by: Mirroring <[email protected]>
1 parent 56fecbb commit 5b9ae97

File tree

10 files changed

+149
-4
lines changed

10 files changed

+149
-4
lines changed

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,35 @@ public void ActivateClass_IgnoreAppLocalHostFxr()
116116
}
117117
}
118118

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

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,34 @@ public void LoadLibrary_ContextConfig(bool load_isolated)
7373
}
7474
}
7575

76+
[Fact]
77+
public void LoadLibrary_IgnoreWorkingDirectory()
78+
{
79+
using (TestArtifact cwd = TestArtifact.Create("cwd"))
80+
{
81+
// Validate that hosting components in the working directory will not be used
82+
File.Copy(Binaries.CoreClr.MockPath, Path.Combine(cwd.Location, Binaries.CoreClr.FileName));
83+
File.Copy(Binaries.HostFxr.MockPath_5_0, Path.Combine(cwd.Location, Binaries.HostFxr.FileName));
84+
File.Copy(Binaries.HostPolicy.MockPath, Path.Combine(cwd.Location, Binaries.HostPolicy.FileName));
85+
86+
string [] args = {
87+
"ijwhost",
88+
sharedState.IjwApp.AppDll,
89+
"NativeEntryPoint"
90+
};
91+
var dotnet = new Microsoft.DotNet.Cli.Build.DotNetCli(sharedState.RepoDirectories.BuiltDotnet);
92+
sharedState.CreateNativeHostCommand(args, sharedState.RepoDirectories.BuiltDotnet)
93+
.WorkingDirectory(cwd.Location)
94+
.Execute()
95+
.Should().Pass()
96+
.And.HaveStdOutContaining("[C++/CLI] NativeEntryPoint: calling managed class")
97+
.And.HaveStdOutContaining("[C++/CLI] ManagedClass: AssemblyLoadContext = \"Default\" System.Runtime.Loader.DefaultAssemblyLoadContext")
98+
.And.ResolveHostFxr(dotnet)
99+
.And.ResolveHostPolicy(dotnet)
100+
.And.ResolveCoreClr(dotnet);
101+
}
102+
}
103+
76104
[Theory]
77105
[InlineData(true)]
78106
[InlineData(false)]

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,38 @@ public void CallDelegateOnComponentContext_UnhandledException()
234234
.And.ExecuteFunctionPointerWithException(entryPoint, 1);
235235
}
236236

237+
[Fact]
238+
public void CallDelegateOnComponentContext_IgnoreWorkingDirectory()
239+
{
240+
using (TestArtifact cwd = TestArtifact.Create("cwd"))
241+
{
242+
// Validate that hosting components in the working directory will not be used
243+
File.Copy(Binaries.CoreClr.MockPath, Path.Combine(cwd.Location, Binaries.CoreClr.FileName));
244+
File.Copy(Binaries.HostPolicy.MockPath, Path.Combine(cwd.Location, Binaries.HostPolicy.FileName));
245+
246+
var component = sharedState.ComponentWithNoDependenciesFixture.TestProject;
247+
string[] args =
248+
{
249+
ComponentLoadAssemblyAndGetFunctionPointerArg,
250+
sharedState.HostFxrPath,
251+
component.RuntimeConfigJson,
252+
component.AppDll,
253+
sharedState.ComponentTypeName,
254+
sharedState.ComponentEntryPoint1,
255+
};
256+
257+
var dotnet = new Microsoft.DotNet.Cli.Build.DotNetCli(sharedState.DotNetRoot);
258+
sharedState.CreateNativeHostCommand(args, sharedState.DotNetRoot)
259+
.WorkingDirectory(cwd.Location)
260+
.Execute()
261+
.Should().Pass()
262+
.And.InitializeContextForConfig(component.RuntimeConfigJson)
263+
.And.ExecuteFunctionPointer(sharedState.ComponentEntryPoint1, 1, 1)
264+
.And.ResolveHostPolicy(dotnet)
265+
.And.ResolveCoreClr(dotnet);
266+
}
267+
}
268+
237269
public class SharedTestState : SharedTestStateBase
238270
{
239271
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/installer/tests/TestUtils/TestArtifact.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,20 @@ protected TestArtifact(TestArtifact source)
5252
source._copies.Add(this);
5353
}
5454

55+
/// <summary>
56+
/// Create a new test artifact.
57+
/// </summary>
58+
/// <param name="name">Name of the test artifact</param>
59+
/// <returns>Test artifact containing no files</returns>
60+
public static TestArtifact Create(string name)
61+
{
62+
var (location, parentPath) = GetNewTestArtifactPath(name);
63+
return new TestArtifact(location)
64+
{
65+
DirectoryToDelete = parentPath
66+
};
67+
}
68+
5569
protected void RegisterCopy(TestArtifact artifact)
5670
{
5771
_copies.Add(artifact);

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ hostfxr_resolver_t::hostfxr_resolver_t(const pal::string_t& app_root)
3838
{
3939
m_status_code = StatusCode::CoreHostLibMissingFailure;
4040
}
41+
else if (!pal::is_path_rooted(m_fxr_path))
42+
{
43+
// We should always be loading hostfxr from an absolute path
44+
m_status_code = StatusCode::CoreHostLibMissingFailure;
45+
}
4146
else if (pal::load_library(&m_fxr_path, &m_hostfxr_dll))
4247
{
4348
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
@@ -44,6 +44,10 @@ int load_fxr_and_get_delegate(hostfxr_delegate_type type, THostPathToConfigCallb
4444
return StatusCode::CoreHostLibMissingFailure;
4545
}
4646

47+
// We should always be loading hostfxr from an absolute path
48+
if (!pal::is_path_rooted(fxr_path))
49+
return StatusCode::CoreHostLibMissingFailure;
50+
4751
// Load library
4852
if (!pal::load_library(&fxr_path, &fxr))
4953
{

src/native/corehost/hostpolicy/deps_resolver.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -830,13 +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-
(void) library_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) library_exists_in_dir(m_app_dir, LIBCORECLR_NAME, &m_coreclr_path);
847+
}
840848
}
841849

842850
// 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)