Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration binding does not throw when it should #110147

Closed
0xced opened this issue Nov 25, 2024 · 2 comments · Fixed by #110209
Closed

Configuration binding does not throw when it should #110147

0xced opened this issue Nov 25, 2024 · 2 comments · Fixed by #110209
Assignees
Labels
area-Extensions-Configuration in-pr There is an active PR which will close this issue when it is merged regression-from-last-release
Milestone

Comments

@0xced
Copy link
Contributor

0xced commented Nov 25, 2024

Description

Binding a configuration using Microsoft.Extensions.Configuration.Binder 9.0.0 with ErrorOnUnknownConfiguration set to true should throw an InvalidOperationException as documented in .NET 8 breaking changes - ConfigurationBinder throws for mismatched value.

Previously, BinderOptions.ErrorOnUnknownConfiguration was used solely to raise an exception if a value existed in the configuration but not in the model being bound to. Now, if this property is set to true, an exception is also thrown if the value in the configuration can't be converted to the type of value in the model.

Unfortunately, it doesn't throw that exception and binding succeeds when it should fail.

Reproduction Steps

BindConfiguration.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net9.0</TargetFramework>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.Configuration" Version="9.0.0" />
    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="9.0.0" />
  </ItemGroup>

</Project>

Program.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Microsoft.Extensions.Configuration;

var hasSourceGenerator = typeof(Program).Assembly.GetTypes().Any(e => e.FullName?.StartsWith("Microsoft.Extensions.Configuration.Binder.SourceGeneration") == true);
Console.WriteLine($"Using configuration binding source generator: {hasSourceGenerator}");
WriteAssemblyInfo(typeof(ConfigurationBuilder).Assembly);
WriteAssemblyInfo(typeof(ConfigurationBinder).Assembly);

var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary<string, string?> { ["Values:Monday"] = "not-an-array-of-string" }).Build();
try
{
    var testSettings = configuration.Get<TestSettings>(options => options.ErrorOnUnknownConfiguration = true);
    Console.WriteLine($"❌ Should have thrown InvalidOperationException but bound {testSettings?.Values.Count} values");
    foreach (var (key, value) in testSettings?.Values ?? [])
    {
        Console.WriteLine($"{key}: {string.Join(", ", value)}");
    }
    return 1;
}
catch (InvalidOperationException exception)
{
    Console.Error.WriteLine($"✅ Expected exception: {exception.Message}");
    return 0;
}

static void WriteAssemblyInfo(Assembly assembly)
{
    Console.WriteLine($"{assembly.GetName().Name} {assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion}");
}

internal class TestSettings
{
    public Dictionary<DayOfWeek, string[]> Values { get; init; } = [];
}

Expected behavior

Running dotnet run should print

✅ Expected exception: 'ErrorOnUnknownConfiguration' was set and binding has failed. The likely cause is an invalid configuration value.

Actual behavior

Running dotnet run prints

❌ Should have thrown InvalidOperationException but bound 0 values

Regression?

This is a regression from Microsoft.Extensions.Configuration.Binder version 8.0.2 where it properly throws an InvalidOperationException.

Note that the regression was introduced between version 9.0.0-preview.1.24080.9 (1d1bf92) where it still throws as expected and version 9.0.0-preview.2.24128.5 (8e5e748) where it doesn't throw anymore.

Known Workarounds

Since it's a regression the workaround is to stay on Microsoft.Extensions.Configuration.Binder version 8.0.2. ☹️

Configuration

dotnet --info
.NET SDK:
 Version:           9.0.100
 Commit:            59db016f11
 Workload version:  9.0.100-manifests.3068a692
 MSBuild version:   17.12.7+5b8665660

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  13.6
 OS Platform: Darwin
 RID:         osx-arm64
 Base Path:   /usr/local/share/dotnet/sdk/9.0.100/

.NET workloads installed:
There are no installed workloads to display.
Configured to use loose manifests when installing new manifests.

Host:
  Version:      9.0.0
  Architecture: arm64
  Commit:       9d5a6a9aa4

.NET SDKs installed:
  6.0.428 [/usr/local/share/dotnet/sdk]
  7.0.410 [/usr/local/share/dotnet/sdk]
  8.0.404 [/usr/local/share/dotnet/sdk]
  9.0.100 [/usr/local/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.36 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.20 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.11 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 9.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.36 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.20 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.11 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 9.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  x64   [/usr/local/share/dotnet/x64]
    registered at [/etc/dotnet/install_location_x64]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download

Other information

Note that setting <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator> in the project to enable the binder source generator never works as expected (i.e. never throws), even with version 8.0.2. So a separate issue should probably be opened about it.

Also it's "funny" that this issue is basically the inverse of #98231 where it throws even though ErrorOnUnknownConfiguration is set to false.

Finally, I still stand by my point that repurposing the ErrorOnUnknownConfiguration property to control whether an exception must be thrown when a conversion fails was not a good idea and that it's never too late to introduce a new property to the options to control the exception behaviour separately from unknown/missing configuration keys.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Nov 25, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

@tarekgh tarekgh added this to the 10.0.0 milestone Nov 25, 2024
@tarekgh tarekgh added regression-from-last-release and removed untriaged New issue has not been triaged by the area owner labels Nov 25, 2024
@tarekgh tarekgh self-assigned this Nov 25, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Nov 27, 2024
@tarekgh
Copy link
Member

tarekgh commented Nov 27, 2024

The change caused this regression is #97777.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Extensions-Configuration in-pr There is an active PR which will close this issue when it is merged regression-from-last-release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants