Skip to content

Commit 6e14d81

Browse files
elinor-fungelinor-fung
authored
Print specific error when framework in runtimeconfig.json is missing a version (dotnet#117914)
Error should now call out the invalid runtimeconfig.json (rather than just failing to find a matching version later): ``` Framework '<name>' is missing a version. Invalid runtimeconfig.json [<path>] ```
1 parent 988fde5 commit 6e14d81

File tree

8 files changed

+132
-44
lines changed

8 files changed

+132
-44
lines changed

‎src/installer/tests/HostActivation.Tests/FrameworkDependentAppLaunch.cs‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Diagnostics;
77
using System.IO;
88
using System.Linq;
9+
using System.Text.Json;
910

1011
using Microsoft.DotNet.Cli.Build.Framework;
1112
using Microsoft.Extensions.DependencyModel;
@@ -325,14 +326,52 @@ public void MissingFrameworkInRuntimeConfig_Fails(bool useAppHost)
325326
}
326327

327328
command.EnableTracingAndCaptureOutputs()
328-
.MultilevelLookup(false)
329329
.Execute()
330330
.Should().Fail()
331331
.And.HaveStdErrContaining($"The library '{Binaries.HostPolicy.FileName}' required to execute the application was not found")
332332
.And.HaveStdErrContaining("Failed to run as a self-contained app")
333333
.And.HaveStdErrContaining($"'{app.RuntimeConfigJson}' did not specify a framework");
334334
}
335335

336+
[Fact]
337+
public void MissingFrameworkName()
338+
{
339+
TestApp app = sharedTestState.MockApp.Copy();
340+
341+
// Create a runtimeconfig.json with a framework that has no name property
342+
var framework = new RuntimeConfig.Framework(null, TestContext.MicrosoftNETCoreAppVersion);
343+
new RuntimeConfig(app.RuntimeConfigJson)
344+
.WithFramework(framework)
345+
.Save();
346+
347+
Command.Create(app.AppExe)
348+
.DotNetRoot(TestContext.BuiltDotNet.BinPath)
349+
.EnableTracingAndCaptureOutputs()
350+
.Execute()
351+
.Should().Fail()
352+
.And.HaveStdErrContaining($"No framework name specified: {framework.ToJson().ToJsonString(new JsonSerializerOptions { WriteIndented = false })}")
353+
.And.HaveStdErrContaining($"Invalid runtimeconfig.json [{app.RuntimeConfigJson}]");
354+
}
355+
356+
[Fact]
357+
public void MissingFrameworkVersion()
358+
{
359+
TestApp app = sharedTestState.MockApp.Copy();
360+
361+
// Create a runtimeconfig.json with a framework that has no version property
362+
new RuntimeConfig(app.RuntimeConfigJson)
363+
.WithFramework(Constants.MicrosoftNETCoreApp, null)
364+
.Save();
365+
366+
Command.Create(app.AppExe)
367+
.DotNetRoot(TestContext.BuiltDotNet.BinPath)
368+
.EnableTracingAndCaptureOutputs()
369+
.Execute()
370+
.Should().Fail()
371+
.And.HaveStdErrContaining($"Framework '{Constants.MicrosoftNETCoreApp}' is missing a version")
372+
.And.HaveStdErrContaining($"Invalid runtimeconfig.json [{app.RuntimeConfigJson}]");
373+
}
374+
336375
[Theory]
337376
[InlineData(true)]
338377
[InlineData(false)]

‎src/installer/tests/HostActivation.Tests/FrameworkResolution/IncludedFrameworksSettings.cs‎

Lines changed: 18 additions & 3 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.Runtime.CompilerServices;
5+
using System.Text.Json;
56
using Microsoft.DotNet.Cli.Build;
67
using Microsoft.DotNet.Cli.Build.Framework;
78
using Xunit;
@@ -45,14 +46,28 @@ public void SelfContainedCanHaveIncludedFrameworks()
4546
}
4647

4748
[Fact]
48-
public void IncludedFrameworkMustSpecifyName()
49+
public void MissingName()
4950
{
51+
var framework = new RuntimeConfig.Framework(null, TestContext.MicrosoftNETCoreAppVersion);
5052
RunSelfContainedTest(
5153
new TestSettings()
5254
.WithRuntimeConfigCustomizer(runtimeConfig => runtimeConfig
53-
.WithIncludedFramework(null, "5.1.2")))
55+
.WithIncludedFramework(framework)))
5456
.Should().Fail()
55-
.And.HaveStdErrContaining("No framework name specified.");
57+
.And.HaveStdErrContaining($"No framework name specified: {framework.ToJson().ToJsonString(new JsonSerializerOptions { WriteIndented = false })}")
58+
.And.HaveStdErrContaining($"Invalid runtimeconfig.json [{SharedState.SelfContainedApp.RuntimeConfigJson}]");
59+
}
60+
61+
[Fact]
62+
public void MissingVersion()
63+
{
64+
RunSelfContainedTest(
65+
new TestSettings()
66+
.WithRuntimeConfigCustomizer(runtimeConfig => runtimeConfig
67+
.WithIncludedFramework(Constants.MicrosoftNETCoreApp, null)))
68+
.Should().Fail()
69+
.And.HaveStdErrContaining($"Framework '{Constants.MicrosoftNETCoreApp}' is missing a version")
70+
.And.HaveStdErrContaining($"Invalid runtimeconfig.json [{SharedState.SelfContainedApp.RuntimeConfigJson}]");
5671
}
5772

5873
[Fact]

‎src/installer/tests/HostActivation.Tests/NativeHostApis.cs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,38 @@ public void Hostfxr_resolve_frameworks_for_runtime_config_InvalidConfig()
749749
}
750750
}
751751

752+
[Theory]
753+
[InlineData(true)]
754+
[InlineData(false)]
755+
public void Hostfxr_resolve_frameworks_for_runtime_config_MissingVersion(bool selfContained)
756+
{
757+
string api = ApiNames.hostfxr_resolve_frameworks_for_runtime_config;
758+
using (TestArtifact artifact = TestArtifact.Create(api))
759+
{
760+
// Create a runtimeconfig.json with a framework reference that has no version property
761+
string configPath = Path.Combine(artifact.Location, "test.runtimeconfig.json");
762+
RuntimeConfig config = RuntimeConfig.FromFile(configPath);
763+
if (selfContained)
764+
{
765+
config.WithIncludedFramework(Constants.MicrosoftNETCoreApp, null);
766+
}
767+
else
768+
{
769+
config.WithFramework(Constants.MicrosoftNETCoreApp, null);
770+
}
771+
772+
config.Save();
773+
774+
TestContext.BuiltDotNet.Exec(sharedTestState.HostApiInvokerApp.AppDll, api, configPath, TestContext.BuiltDotNet.BinPath)
775+
.CaptureStdOut()
776+
.CaptureStdErr()
777+
.Execute()
778+
.Should().Pass()
779+
.And.HaveStdErrContaining($"Framework '{Constants.MicrosoftNETCoreApp}' is missing a version")
780+
.And.ReturnStatusCode(api, Constants.ErrorCode.InvalidConfigFile);
781+
}
782+
}
783+
752784
[Fact]
753785
public void Hostpolicy_corehost_set_error_writer_test()
754786
{

‎src/installer/tests/TestUtils/Assertions/CommandResultAssertions.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public AndConstraint<CommandResultAssertions> HaveStdOut(string expectedOutput)
6363

6464
public AndConstraint<CommandResultAssertions> HaveStdOutContaining(string pattern)
6565
{
66-
CurrentAssertionChain.ForCondition(Result.StdOut.Contains(pattern))
66+
CurrentAssertionChain.ForCondition(!string.IsNullOrEmpty(Result.StdOut) && Result.StdOut.Contains(pattern))
6767
.FailWith($"The command output did not contain expected result: '{pattern}'{GetDiagnosticsInfo()}");
6868
return new AndConstraint<CommandResultAssertions>(this);
6969
}
@@ -98,7 +98,7 @@ public AndConstraint<CommandResultAssertions> HaveStdErr()
9898

9999
public AndConstraint<CommandResultAssertions> HaveStdErrContaining(string pattern)
100100
{
101-
CurrentAssertionChain.ForCondition(Result.StdErr.Contains(pattern))
101+
CurrentAssertionChain.ForCondition(!string.IsNullOrEmpty(Result.StdErr) && Result.StdErr.Contains(pattern))
102102
.FailWith($"The command error output did not contain expected result: '{pattern}'{GetDiagnosticsInfo()}");
103103
return new AndConstraint<CommandResultAssertions>(this);
104104
}

‎src/installer/tests/TestUtils/RuntimeConfig.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public Framework WithApplyPatches(bool? value)
4646
return this;
4747
}
4848

49-
internal JsonObject ToJson()
49+
public JsonObject ToJson()
5050
{
5151
JsonObject frameworkReference = new();
5252

‎src/native/corehost/fxr/hostfxr.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,12 @@ SHARED_API int32_t HOSTFXR_CALLTYPE hostfxr_resolve_frameworks_for_runtime_confi
623623
const runtime_config_t::settings_t override_settings;
624624
app->parse_runtime_config(runtime_config, _X(""), override_settings);
625625

626-
const runtime_config_t app_config = app->get_runtime_config();
626+
const runtime_config_t& app_config = app->get_runtime_config();
627+
if (!app_config.is_valid())
628+
{
629+
trace::error(_X("Invalid runtimeconfig.json [%s]"), app_config.get_path().c_str());
630+
return StatusCode::InvalidConfigFile;
631+
}
627632

628633
// Resolve frameworks for framework-dependent apps.
629634
// Self-contained apps assume the framework is next to the app, so we just treat it as success.

‎src/native/corehost/runtime_config.cpp‎

Lines changed: 31 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ bool runtime_config_t::parse_opts(const json_parser_t::value_t& opts)
188188
m_is_framework_dependent = true;
189189

190190
fx_reference_t fx_out;
191-
if (!parse_framework(framework->value, fx_out))
191+
if (!parse_framework(framework->value, /*name_and_version_only*/ false, fx_out))
192192
{
193193
return false;
194194
}
@@ -201,7 +201,7 @@ bool runtime_config_t::parse_opts(const json_parser_t::value_t& opts)
201201
{
202202
m_is_framework_dependent = true;
203203

204-
if (!read_framework_array(iter->value, m_frameworks))
204+
if (!read_framework_array(iter->value, /*name_and_version_only*/ false, m_frameworks))
205205
{
206206
return false;
207207
}
@@ -216,7 +216,7 @@ bool runtime_config_t::parse_opts(const json_parser_t::value_t& opts)
216216
return false;
217217
}
218218

219-
if (!read_framework_array(includedFrameworks->value, m_included_frameworks, /*name_and_version_only*/ true))
219+
if (!read_framework_array(includedFrameworks->value, /*name_and_version_only*/ true, m_included_frameworks))
220220
{
221221
return false;
222222
}
@@ -241,35 +241,45 @@ namespace
241241
}
242242
}
243243

244-
bool runtime_config_t::parse_framework(const json_parser_t::value_t& fx_obj, fx_reference_t& fx_out, bool name_and_version_only)
244+
bool runtime_config_t::parse_framework(const json_parser_t::value_t& fx_obj, bool name_and_version_only, fx_reference_t& fx_out)
245245
{
246246
if (!name_and_version_only)
247247
{
248248
apply_settings_to_fx_reference(m_default_settings, fx_out);
249249
}
250250

251251
const auto& fx_name = fx_obj.FindMember(_X("name"));
252-
if (fx_name != fx_obj.MemberEnd())
252+
if (fx_name == fx_obj.MemberEnd())
253253
{
254-
fx_out.set_fx_name(fx_name->value.GetString());
254+
using string_buffer_t = rapidjson::GenericStringBuffer<json_parser_t::internal_encoding_type_t>;
255+
string_buffer_t sb;
256+
rapidjson::Writer<string_buffer_t, json_parser_t::internal_encoding_type_t,
257+
json_parser_t::internal_encoding_type_t> writer{sb};
258+
fx_obj.Accept(writer);
259+
260+
trace::error(_X("No framework name specified: %s"), sb.GetString());
261+
return false;
255262
}
256263

264+
fx_out.set_fx_name(fx_name->value.GetString());
265+
257266
const auto& fx_ver = fx_obj.FindMember(_X("version"));
258-
if (fx_ver != fx_obj.MemberEnd())
267+
if (fx_ver == fx_obj.MemberEnd())
259268
{
260-
fx_out.set_fx_version(fx_ver->value.GetString());
261-
262-
// Release version should prefer release versions, unless the rollForwardToPrerelease is set
263-
// in which case no preference should be applied.
264-
if (!name_and_version_only && !fx_out.get_fx_version_number().is_prerelease() && !m_roll_forward_to_prerelease)
265-
{
266-
fx_out.set_prefer_release(true);
267-
}
269+
trace::error(_X("Framework '%s' is missing a version."), fx_out.get_fx_name().c_str());
270+
return false;
268271
}
269272

273+
fx_out.set_fx_version(fx_ver->value.GetString());
274+
270275
if (name_and_version_only)
271-
{
272276
return true;
277+
278+
// Release version should prefer release versions, unless the rollForwardToPrerelease is set
279+
// in which case no preference should be applied.
280+
if (!fx_out.get_fx_version_number().is_prerelease() && !m_roll_forward_to_prerelease)
281+
{
282+
fx_out.set_prefer_release(true);
273283
}
274284

275285
const auto& roll_forward = fx_obj.FindMember(_X("rollForward"));
@@ -357,25 +367,13 @@ bool runtime_config_t::ensure_dev_config_parsed()
357367
return true;
358368
}
359369

360-
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)
370+
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)
361371
{
362-
bool rc = true;
363-
364372
for (const auto& fx_json : frameworks_json.GetArray())
365373
{
366374
fx_reference_t fx_out;
367-
rc = parse_framework(fx_json, fx_out, name_and_version_only);
368-
if (!rc)
369-
{
370-
break;
371-
}
372-
373-
if (fx_out.get_fx_name().length() == 0)
374-
{
375-
trace::verbose(_X("No framework name specified."));
376-
rc = false;
377-
break;
378-
}
375+
if (!parse_framework(fx_json, name_and_version_only, fx_out))
376+
return false;
379377

380378
if (std::find_if(
381379
frameworks_out.begin(),
@@ -384,14 +382,13 @@ bool runtime_config_t::read_framework_array(const json_parser_t::value_t& framew
384382
!= frameworks_out.end())
385383
{
386384
trace::verbose(_X("Framework %s already specified."), fx_out.get_fx_name().c_str());
387-
rc = false;
388-
break;
385+
return false;
389386
}
390387

391388
frameworks_out.push_back(fx_out);
392389
}
393390

394-
return rc;
391+
return true;
395392
}
396393

397394
bool runtime_config_t::ensure_parsed()

‎src/native/corehost/runtime_config.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ class runtime_config_t
7979
// If set to true, all versions (including pre-release) are considered even if starting from a release framework reference.
8080
bool m_roll_forward_to_prerelease;
8181

82-
bool parse_framework(const json_parser_t::value_t& fx_obj, fx_reference_t& fx_out, bool name_and_version_only = false);
83-
bool read_framework_array(const json_parser_t::value_t& frameworks, fx_reference_vector_t& frameworks_out, bool name_and_version_only = false);
82+
bool parse_framework(const json_parser_t::value_t& fx_obj, bool name_and_version_only, fx_reference_t& fx_out);
83+
bool read_framework_array(const json_parser_t::value_t& frameworks, bool name_and_version_only, fx_reference_vector_t& frameworks_out);
8484

8585
bool mark_specified_setting(specified_setting setting);
8686
};

0 commit comments

Comments
 (0)