From ad12cda0d88a7c55bad9766c49cd694d56fbc33a Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Mon, 21 Jul 2025 20:12:36 -0700 Subject: [PATCH 1/2] Print specific error when framework in runtimeconfig.json is missing a version --- .../FrameworkDependentAppLaunch.cs | 41 +++++++++++++- .../IncludedFrameworksSettings.cs | 21 ++++++-- .../HostActivation.Tests/NativeHostApis.cs | 32 +++++++++++ .../Assertions/CommandResultAssertions.cs | 4 +- .../tests/TestUtils/RuntimeConfig.cs | 2 +- src/native/corehost/fxr/hostfxr.cpp | 7 ++- src/native/corehost/runtime_config.cpp | 53 +++++++++---------- 7 files changed, 125 insertions(+), 35 deletions(-) diff --git a/src/installer/tests/HostActivation.Tests/FrameworkDependentAppLaunch.cs b/src/installer/tests/HostActivation.Tests/FrameworkDependentAppLaunch.cs index 70d3bacfd76c78..523bc564207e03 100644 --- a/src/installer/tests/HostActivation.Tests/FrameworkDependentAppLaunch.cs +++ b/src/installer/tests/HostActivation.Tests/FrameworkDependentAppLaunch.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.IO; using System.Linq; +using System.Text.Json; using Microsoft.DotNet.Cli.Build.Framework; using Microsoft.Extensions.DependencyModel; @@ -325,7 +326,6 @@ public void MissingFrameworkInRuntimeConfig_Fails(bool useAppHost) } command.EnableTracingAndCaptureOutputs() - .MultilevelLookup(false) .Execute() .Should().Fail() .And.HaveStdErrContaining($"The library '{Binaries.HostPolicy.FileName}' required to execute the application was not found") @@ -333,6 +333,45 @@ public void MissingFrameworkInRuntimeConfig_Fails(bool useAppHost) .And.HaveStdErrContaining($"'{app.RuntimeConfigJson}' did not specify a framework"); } + [Fact] + public void MissingFrameworkName() + { + TestApp app = sharedTestState.MockApp.Copy(); + + // Create a runtimeconfig.json with a framework that has no name property + var framework = new RuntimeConfig.Framework(null, TestContext.MicrosoftNETCoreAppVersion); + new RuntimeConfig(app.RuntimeConfigJson) + .WithFramework(framework) + .Save(); + + Command.Create(app.AppExe) + .DotNetRoot(TestContext.BuiltDotNet.BinPath) + .EnableTracingAndCaptureOutputs() + .Execute() + .Should().Fail() + .And.HaveStdErrContaining($"No framework name specified: {framework.ToJson().ToJsonString(new JsonSerializerOptions { WriteIndented = false })}") + .And.HaveStdErrContaining($"Invalid runtimeconfig.json [{app.RuntimeConfigJson}]"); + } + + [Fact] + public void MissingFrameworkVersion() + { + TestApp app = sharedTestState.MockApp.Copy(); + + // Create a runtimeconfig.json with a framework that has no version property + new RuntimeConfig(app.RuntimeConfigJson) + .WithFramework(Constants.MicrosoftNETCoreApp, null) + .Save(); + + Command.Create(app.AppExe) + .DotNetRoot(TestContext.BuiltDotNet.BinPath) + .EnableTracingAndCaptureOutputs() + .Execute() + .Should().Fail() + .And.HaveStdErrContaining($"Framework '{Constants.MicrosoftNETCoreApp}' is missing a version") + .And.HaveStdErrContaining($"Invalid runtimeconfig.json [{app.RuntimeConfigJson}]"); + } + [Theory] [InlineData(true)] [InlineData(false)] diff --git a/src/installer/tests/HostActivation.Tests/FrameworkResolution/IncludedFrameworksSettings.cs b/src/installer/tests/HostActivation.Tests/FrameworkResolution/IncludedFrameworksSettings.cs index cd3efe4ddcc862..a46105cb933c10 100644 --- a/src/installer/tests/HostActivation.Tests/FrameworkResolution/IncludedFrameworksSettings.cs +++ b/src/installer/tests/HostActivation.Tests/FrameworkResolution/IncludedFrameworksSettings.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Runtime.CompilerServices; +using System.Text.Json; using Microsoft.DotNet.Cli.Build; using Microsoft.DotNet.Cli.Build.Framework; using Xunit; @@ -45,14 +46,28 @@ public void SelfContainedCanHaveIncludedFrameworks() } [Fact] - public void IncludedFrameworkMustSpecifyName() + public void MissingName() { + var framework = new RuntimeConfig.Framework(null, TestContext.MicrosoftNETCoreAppVersion); RunSelfContainedTest( new TestSettings() .WithRuntimeConfigCustomizer(runtimeConfig => runtimeConfig - .WithIncludedFramework(null, "5.1.2"))) + .WithIncludedFramework(framework))) .Should().Fail() - .And.HaveStdErrContaining("No framework name specified."); + .And.HaveStdErrContaining($"No framework name specified: {framework.ToJson().ToJsonString(new JsonSerializerOptions { WriteIndented = false })}") + .And.HaveStdErrContaining($"Invalid runtimeconfig.json [{SharedState.SelfContainedApp.RuntimeConfigJson}]"); + } + + [Fact] + public void MissingVersion() + { + RunSelfContainedTest( + new TestSettings() + .WithRuntimeConfigCustomizer(runtimeConfig => runtimeConfig + .WithIncludedFramework(Constants.MicrosoftNETCoreApp, null))) + .Should().Fail() + .And.HaveStdErrContaining($"Framework '{Constants.MicrosoftNETCoreApp}' is missing a version") + .And.HaveStdErrContaining($"Invalid runtimeconfig.json [{SharedState.SelfContainedApp.RuntimeConfigJson}]"); } [Fact] diff --git a/src/installer/tests/HostActivation.Tests/NativeHostApis.cs b/src/installer/tests/HostActivation.Tests/NativeHostApis.cs index e8b071b8fc7e83..93d3b1f6f9a219 100644 --- a/src/installer/tests/HostActivation.Tests/NativeHostApis.cs +++ b/src/installer/tests/HostActivation.Tests/NativeHostApis.cs @@ -749,6 +749,38 @@ public void Hostfxr_resolve_frameworks_for_runtime_config_InvalidConfig() } } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Hostfxr_resolve_frameworks_for_runtime_config_MissingVersion(bool selfContained) + { + string api = ApiNames.hostfxr_resolve_frameworks_for_runtime_config; + using (TestArtifact artifact = TestArtifact.Create(api)) + { + // Create a runtimeconfig.json with a framework reference that has no version property + string configPath = Path.Combine(artifact.Location, "test.runtimeconfig.json"); + RuntimeConfig config = RuntimeConfig.FromFile(configPath); + if (selfContained) + { + config.WithIncludedFramework(Constants.MicrosoftNETCoreApp, null); + } + else + { + config.WithFramework(Constants.MicrosoftNETCoreApp, null); + } + + config.Save(); + + TestContext.BuiltDotNet.Exec(sharedTestState.HostApiInvokerApp.AppDll, api, configPath, TestContext.BuiltDotNet.BinPath) + .CaptureStdOut() + .CaptureStdErr() + .Execute() + .Should().Pass() + .And.HaveStdErrContaining($"Framework '{Constants.MicrosoftNETCoreApp}' is missing a version") + .And.ReturnStatusCode(api, Constants.ErrorCode.InvalidConfigFile); + } + } + [Fact] public void Hostpolicy_corehost_set_error_writer_test() { diff --git a/src/installer/tests/TestUtils/Assertions/CommandResultAssertions.cs b/src/installer/tests/TestUtils/Assertions/CommandResultAssertions.cs index 25acf3d3123e32..b2195ce1973772 100644 --- a/src/installer/tests/TestUtils/Assertions/CommandResultAssertions.cs +++ b/src/installer/tests/TestUtils/Assertions/CommandResultAssertions.cs @@ -63,7 +63,7 @@ public AndConstraint HaveStdOut(string expectedOutput) public AndConstraint HaveStdOutContaining(string pattern) { - CurrentAssertionChain.ForCondition(Result.StdOut.Contains(pattern)) + CurrentAssertionChain.ForCondition(!string.IsNullOrEmpty(Result.StdOut) && Result.StdOut.Contains(pattern)) .FailWith($"The command output did not contain expected result: '{pattern}'{GetDiagnosticsInfo()}"); return new AndConstraint(this); } @@ -98,7 +98,7 @@ public AndConstraint HaveStdErr() public AndConstraint HaveStdErrContaining(string pattern) { - CurrentAssertionChain.ForCondition(Result.StdErr.Contains(pattern)) + CurrentAssertionChain.ForCondition(!string.IsNullOrEmpty(Result.StdErr) && Result.StdErr.Contains(pattern)) .FailWith($"The command error output did not contain expected result: '{pattern}'{GetDiagnosticsInfo()}"); return new AndConstraint(this); } diff --git a/src/installer/tests/TestUtils/RuntimeConfig.cs b/src/installer/tests/TestUtils/RuntimeConfig.cs index 7f13e3dab88538..3ff9ab746b99f1 100644 --- a/src/installer/tests/TestUtils/RuntimeConfig.cs +++ b/src/installer/tests/TestUtils/RuntimeConfig.cs @@ -46,7 +46,7 @@ public Framework WithApplyPatches(bool? value) return this; } - internal JsonObject ToJson() + public JsonObject ToJson() { JsonObject frameworkReference = new(); diff --git a/src/native/corehost/fxr/hostfxr.cpp b/src/native/corehost/fxr/hostfxr.cpp index 98c2ae26a14f92..bb3d8345568a9c 100644 --- a/src/native/corehost/fxr/hostfxr.cpp +++ b/src/native/corehost/fxr/hostfxr.cpp @@ -623,7 +623,12 @@ SHARED_API int32_t HOSTFXR_CALLTYPE hostfxr_resolve_frameworks_for_runtime_confi const runtime_config_t::settings_t override_settings; app->parse_runtime_config(runtime_config, _X(""), override_settings); - const runtime_config_t app_config = app->get_runtime_config(); + const runtime_config_t& app_config = app->get_runtime_config(); + if (!app_config.is_valid()) + { + trace::error(_X("Invalid runtimeconfig.json [%s]"), app_config.get_path().c_str()); + return StatusCode::InvalidConfigFile; + } // Resolve frameworks for framework-dependent apps. // Self-contained apps assume the framework is next to the app, so we just treat it as success. diff --git a/src/native/corehost/runtime_config.cpp b/src/native/corehost/runtime_config.cpp index 04081ddbbea482..2a9864dd038059 100644 --- a/src/native/corehost/runtime_config.cpp +++ b/src/native/corehost/runtime_config.cpp @@ -249,22 +249,34 @@ bool runtime_config_t::parse_framework(const json_parser_t::value_t& fx_obj, fx_ } const auto& fx_name = fx_obj.FindMember(_X("name")); - if (fx_name != fx_obj.MemberEnd()) + if (fx_name == fx_obj.MemberEnd()) { - fx_out.set_fx_name(fx_name->value.GetString()); + using string_buffer_t = rapidjson::GenericStringBuffer; + string_buffer_t sb; + rapidjson::Writer writer{sb}; + fx_obj.Accept(writer); + + trace::error(_X("No framework name specified: %s"), sb.GetString()); + return false; } + fx_out.set_fx_name(fx_name->value.GetString()); + const auto& fx_ver = fx_obj.FindMember(_X("version")); - if (fx_ver != fx_obj.MemberEnd()) + if (fx_ver == fx_obj.MemberEnd()) { - fx_out.set_fx_version(fx_ver->value.GetString()); + trace::error(_X("Framework '%s' is missing a version."), fx_out.get_fx_name().c_str()); + return false; + } - // Release version should prefer release versions, unless the rollForwardToPrerelease is set - // in which case no preference should be applied. - if (!name_and_version_only && !fx_out.get_fx_version_number().is_prerelease() && !m_roll_forward_to_prerelease) - { - fx_out.set_prefer_release(true); - } + fx_out.set_fx_version(fx_ver->value.GetString()); + + // Release version should prefer release versions, unless the rollForwardToPrerelease is set + // in which case no preference should be applied. + if (!name_and_version_only && !fx_out.get_fx_version_number().is_prerelease() && !m_roll_forward_to_prerelease) + { + fx_out.set_prefer_release(true); } if (name_and_version_only) @@ -359,23 +371,11 @@ bool runtime_config_t::ensure_dev_config_parsed() bool runtime_config_t::read_framework_array(const json_parser_t::value_t& frameworks_json, fx_reference_vector_t& frameworks_out, bool name_and_version_only) { - bool rc = true; - for (const auto& fx_json : frameworks_json.GetArray()) { fx_reference_t fx_out; - rc = parse_framework(fx_json, fx_out, name_and_version_only); - if (!rc) - { - break; - } - - if (fx_out.get_fx_name().length() == 0) - { - trace::verbose(_X("No framework name specified.")); - rc = false; - break; - } + if (!parse_framework(fx_json, fx_out, name_and_version_only)) + return false; if (std::find_if( frameworks_out.begin(), @@ -384,14 +384,13 @@ bool runtime_config_t::read_framework_array(const json_parser_t::value_t& framew != frameworks_out.end()) { trace::verbose(_X("Framework %s already specified."), fx_out.get_fx_name().c_str()); - rc = false; - break; + return false; } frameworks_out.push_back(fx_out); } - return rc; + return true; } bool runtime_config_t::ensure_parsed() From 24b095a74df5687cd8dca3b9d44901abb66fbf6e Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 24 Jul 2025 16:39:15 -0700 Subject: [PATCH 2/2] Make name_and_version_only argument non-optional --- src/native/corehost/runtime_config.cpp | 22 ++++++++++------------ src/native/corehost/runtime_config.h | 4 ++-- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/native/corehost/runtime_config.cpp b/src/native/corehost/runtime_config.cpp index 2a9864dd038059..be14888d1c28ba 100644 --- a/src/native/corehost/runtime_config.cpp +++ b/src/native/corehost/runtime_config.cpp @@ -188,7 +188,7 @@ bool runtime_config_t::parse_opts(const json_parser_t::value_t& opts) m_is_framework_dependent = true; fx_reference_t fx_out; - if (!parse_framework(framework->value, fx_out)) + if (!parse_framework(framework->value, /*name_and_version_only*/ false, fx_out)) { return false; } @@ -201,7 +201,7 @@ bool runtime_config_t::parse_opts(const json_parser_t::value_t& opts) { m_is_framework_dependent = true; - if (!read_framework_array(iter->value, m_frameworks)) + if (!read_framework_array(iter->value, /*name_and_version_only*/ false, m_frameworks)) { return false; } @@ -216,7 +216,7 @@ bool runtime_config_t::parse_opts(const json_parser_t::value_t& opts) return false; } - if (!read_framework_array(includedFrameworks->value, m_included_frameworks, /*name_and_version_only*/ true)) + if (!read_framework_array(includedFrameworks->value, /*name_and_version_only*/ true, m_included_frameworks)) { return false; } @@ -241,7 +241,7 @@ namespace } } -bool runtime_config_t::parse_framework(const json_parser_t::value_t& fx_obj, fx_reference_t& fx_out, bool name_and_version_only) +bool runtime_config_t::parse_framework(const json_parser_t::value_t& fx_obj, bool name_and_version_only, fx_reference_t& fx_out) { if (!name_and_version_only) { @@ -272,18 +272,16 @@ bool runtime_config_t::parse_framework(const json_parser_t::value_t& fx_obj, fx_ fx_out.set_fx_version(fx_ver->value.GetString()); + if (name_and_version_only) + return true; + // Release version should prefer release versions, unless the rollForwardToPrerelease is set // in which case no preference should be applied. - if (!name_and_version_only && !fx_out.get_fx_version_number().is_prerelease() && !m_roll_forward_to_prerelease) + if (!fx_out.get_fx_version_number().is_prerelease() && !m_roll_forward_to_prerelease) { fx_out.set_prefer_release(true); } - if (name_and_version_only) - { - return true; - } - const auto& roll_forward = fx_obj.FindMember(_X("rollForward")); if (roll_forward != fx_obj.MemberEnd()) { @@ -369,12 +367,12 @@ bool runtime_config_t::ensure_dev_config_parsed() return true; } -bool runtime_config_t::read_framework_array(const json_parser_t::value_t& frameworks_json, fx_reference_vector_t& frameworks_out, bool name_and_version_only) +bool runtime_config_t::read_framework_array(const json_parser_t::value_t& frameworks_json, bool name_and_version_only, fx_reference_vector_t& frameworks_out) { for (const auto& fx_json : frameworks_json.GetArray()) { fx_reference_t fx_out; - if (!parse_framework(fx_json, fx_out, name_and_version_only)) + if (!parse_framework(fx_json, name_and_version_only, fx_out)) return false; if (std::find_if( diff --git a/src/native/corehost/runtime_config.h b/src/native/corehost/runtime_config.h index 820684460d2748..c345f2d0163a14 100644 --- a/src/native/corehost/runtime_config.h +++ b/src/native/corehost/runtime_config.h @@ -79,8 +79,8 @@ class runtime_config_t // If set to true, all versions (including pre-release) are considered even if starting from a release framework reference. bool m_roll_forward_to_prerelease; - bool parse_framework(const json_parser_t::value_t& fx_obj, fx_reference_t& fx_out, bool name_and_version_only = false); - bool read_framework_array(const json_parser_t::value_t& frameworks, fx_reference_vector_t& frameworks_out, bool name_and_version_only = false); + bool parse_framework(const json_parser_t::value_t& fx_obj, bool name_and_version_only, fx_reference_t& fx_out); + bool read_framework_array(const json_parser_t::value_t& frameworks, bool name_and_version_only, fx_reference_vector_t& frameworks_out); bool mark_specified_setting(specified_setting setting); };