Treat empty string value from chained configuration as present

[rosebyte]

Related to #65594

Summary

ChainedConfigurationProvider.TryGet used !string.IsNullOrEmpty(value), so a key present in the chained configuration with an empty value was reported as not found. It therefore failed to shadow values from providers registered earlier in the outer builder, and a chained configuration disagreed with a plain ConfigurationBuilder given the same data.

Continuation of the .NET 10 work

.NET 10 converged the interpretations of "" and null in #116677 ("Support Null configuration", fixing #116700 and #36510). Before that pass the two were used more or less interchangeably to mean "nothing here". That PR separated them:

• the JSON provider stores a real null rather than converting it to "";
• an empty JSON array moved off null and onto "";
• the binder treats null as a value to bind rather than a value to skip;
• ConfigurationSection.TryGetValue was added so callers can tell an absent key from an explicit null.

The rule that fell out of it is that an empty string is a value and null is an absence.

ChainedConfigurationProvider was not part of that pass and still conflates the two. This PR applies the same rule to it.

Null is deliberately left alone

The null rows still disagree, and this PR does not change that.

The wrapped IConfiguration exposes only an indexer, which cannot tell a null value apart from a missing key. Closing the gap would mean downcasting to the in-box configuration types and walking their providers, which turns an implementation detail into a contract: it would commit ConfigurationRoot and ConfigurationManager to never growing lookup logic of their own. That is too high a price for the remaining sliver.

There is also a reading on which the current behaviour is right. A chained IConfiguration is a merged unit rather than a single provider, and a unit reports the absence of a value as null, so "null means nothing to contribute" is defensible on its own terms. ChainedConfiguration_NullValueIsNotContributed pins it so the limit reads as deliberate rather than as an oversight.

Breaking change

This is a behavioural breaking change, and it is the counterpart to the .NET 10 one documented as Null values preserved in configuration (dotnet/docs#46890). That change stopped providers treating null as missing; this one stops chained configuration treating empty as missing.

Requesting the breaking-change label, and a docs issue to sit alongside the .NET 10 article.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Updates ChainedConfigurationProvider.TryGet so that an empty string ("") from the chained configuration is treated as a present value (only null remains “not found”). This brings chained configuration behavior in line with the provider model where “found” is independent of “non-empty”.

Changes:

• Change ChainedConfigurationProvider.TryGet to return true when the underlying configuration indexer returns a non-null value (including empty string).
• Add tests covering empty-string shadowing behavior and parity with directly-added in-memory sources, plus a test pinning existing null behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/libraries/Microsoft.Extensions.Configuration/src/ChainedConfigurationProvider.cs	Adjusts TryGet presence semantics from “non-empty” to “non-null” and updates its XML doc.
src/libraries/Microsoft.Extensions.Configuration/tests/ChainedConfigurationProviderTests.cs	Adds coverage for empty-string presence/shadowing and null non-contribution scenarios.

Comments suppressed due to low confidence (2)

src/libraries/Microsoft.Extensions.Configuration/tests/ChainedConfigurationProviderTests.cs:313

• TryGet may assign null to the out parameter on failure; using out string value here can trigger nullability warnings. Use out string? to match the contract.

            Assert.False(provider.TryGet("MissingKey", out string value));
            Assert.Null(value);

src/libraries/Microsoft.Extensions.Configuration/tests/ChainedConfigurationProviderTests.cs:396

• IConfiguration.this[string] is nullable (string?). Implementing it as non-nullable string can cause nullability mismatch warnings (and makes the wrapper incorrect for missing keys). Make the indexer string? to match the interface contract.

            public string this[string key]
            {
                get => _inner[key];
                set => _inner[key] = value;
            }

[dotnet-policy-service]

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

[tarekgh]

@rosebyte I have marked this PR with the breaking change label. It is better to have breaking change doc for that and can be linked to the .NET 10 breaking change doc.

[tarekgh]

One thing worth deciding before merge: the PR says Fixes #65594, which will auto-close that issue on merge. But #65594 is specifically about the null case (an inner provider returning true with value == null being swallowed by the chained provider). This PR intentionally addresses only the empty-string case and deliberately leaves null as-is, which is a reasonable call given the IConfiguration indexer cannot distinguish a null-valued key from a missing key.

Since the core scenario in #65594 (respecting null) remains unaddressed, auto-closing it would lose track of that gap. Suggest either:

• change Fixes #65594 to a non-closing reference (e.g. "addresses part of Configuration data allows nullable values, but ChainedConfigurationProvider.TryGet doesn't work correctly with them #65594") and keep the issue open to track the null limitation, or
• keep Fixes #65594 but file a small follow-up issue for the null case so it is not lost.

Either way is fine, just want to make sure the null limitation stays tracked rather than being closed out silently.

[tarekgh]

Building on the Fixes #65594 note above: the null case (an inner provider that returns true with value == null) is the actual subject of that issue, and it can be handled for the in-box configuration types instead of being dropped.

_config[key] goes through the ConfigurationRoot indexer, which returns null both when the key is absent and when it is present with a null value, so the two are indistinguishable. But InternalConfigurationRootExtensions.TryGetConfiguration runs the same reverse provider scan and returns the bool instead of collapsing to null, so it can report true + null. This is not a new contract: ConfigurationSection.TryGetValue already uses _root.TryGetConfiguration(...) for exactly this purpose (added in the .NET 10 null work this PR cites).

Suggested implementation:

public bool TryGet(string key, out string? value) => _config switch
{
    IConfigurationRoot root => root.TryGetConfiguration(key, out value),
    ConfigurationSection section => section.TryGetValue(key, out value),
    _ => (value = _config[key]) is not null
};

TryGetConfiguration is internal and in the same assembly, and ConfigurationSection.TryGetValue is public, so both are callable. This behaves identically to the current change for non-null and empty values; the only difference is that a wrapped root/manager/section with a present-null value now correctly returns true + null, matching the base provider and a directly added source. The residual is limited to custom IConfiguration implementations, which expose only the indexer and genuinely cannot distinguish present-null from absent.

Points to address if we take this:

1. Behavior for the in-box types should respect null rather than drop it, so ChainedConfiguration_NullValueIsNotContributed needs to be updated: null is now contributed for the root/manager/section kinds, and the "not contributed" assertion stays only for the custom IConfiguration kind.
2. The breaking-change doc should cover both cases: chained configuration no longer treats empty as missing, and no longer treats a present null from an in-box configuration as missing.
3. Add test coverage for the null-shadowing case across ConfigurationRoot, ConfigurationManager, and ConfigurationSection, mirroring the existing empty-string theories, so the new null behavior is pinned the same way.

[rosebyte]

On reading the wrapped configuration's providers: Eric suggested the same in the issue, but I do not think we should take it.

There is a problem with the layering. A provider bids. TryGet returns true with a value, and that value may be empty or null, meaning "the final value is this, ignore anything a lower provider says". ConfigurationRoot.GetConfiguration honours that by returning on the first true whatever the value is. An IConfiguration sits at the other end of that process: it is the resolved result, not a bid. ChainedConfigurationProvider turns a resolved result back into a bid, and the resolved world has exactly one token for "nothing", which is null. Empty is unambiguous there, which is why this PR fixes it. Null is not, and nothing we do inside the chained provider changes that, because the ambiguity is in IConfiguration itself. I do not even think it's a gap awaiting a fix.

I have further concerns about bypassing the IConfiguration contract:

1. It makes behaviour a function of the wrapped configuration's concrete type. Two chained providers wrapping configurations having the same underlying configuration that agree on every observable IConfiguration operation would then disagree, which is harder to explain than the inconsistency we have.

2. Matching IConfigurationRoot matches the interface, not our implementation. TryGetConfiguration scans root.Providers in reverse, which is what ConfigurationRoot happens to do, but a third-party root is free to do something else in its indexer: substitution, references, different precedence, caching. For those the chained provider would quietly disagree with the root's own indexer.

3. It turns the current implementation into contract. Once chained behaviour depends on ConfigurationRoot and ConfigurationManager reducing to a plain reverse provider scan, that reduction becomes observable and neither type can grow logic of its own without silently changing chained behaviour.

So we can keep the PR as merely related to the issue, for clarity, but once it is merged I do not see anything else actionable there, and I would propose closing it: what it otherwise suggests would likely make the configuration extensions more inconsistent rather than less.

[rosebyte]

Last but not least, the indexer of the ConfigurationManager does not merely sweep, it sweeps a list it has pinned. That pin is what keeps a read correct while Sources is being modified, since ReplaceProviders delays disposing the old providers until the last reference goes. TryGetConfiguration reads root.Providers instead, which opts out by design:

// We cannot track the duration of the reference to the providers if this property is used.
// If a configuration source is removed after this is accessed but before it's completely enumerated,
// this may allow access to a disposed provider.
IEnumerable<IConfigurationProvider> IConfigurationRoot.Providers => _providerManager.NonReferenceCountedProviders;

and copes by swallowing ObjectDisposedException, which skips the provider rather than reading it, so a lower precedence value can silently win.

[tarekgh]

Thanks for the detailed analysis. Your disposal point is correct and I verified it against the code: TryGetConfiguration reads root.Providers, which for ConfigurationManager is the non-reference-counted list, and it swallows ObjectDisposedException. So reaching in from the chained provider via that path can let a lower-precedence value win under concurrent Sources mutation. Agreed that the downcast as I first suggested is not the right shape.

I think there is a cleaner path that addresses the null case without the problems you raised, and it is the one the framework code already hints at. In TryGetConfiguration there is this note:

// If we want to avoid this possible exception altogether, we could update
// ConfigurationSection.TryGetValue to be virtual and have ConfigurationManager
// implement it with reference counting like it does for the indexer.

GetChildrenImplementation right above it already does exactly that pinning ((root as ConfigurationManager)?.GetProvidersReference() plus eager ToList). So the idea is to push a disposal-safe, bool-returning lookup into the concrete types: make the lookup virtual and have ConfigurationManager override it with reference counting like its indexer, ConfigurationRoot uses its fixed providers, and fix TryGetConfiguration to pin the same way. Then the chained provider forwards through that and faithfully reports present-null.

I think this also answers your three concerns, because the behavior stops being a wrapper reverse-engineering ConfigurationRoot's reverse scan and becomes each root defining its own lookup consistent with its own indexer. A third-party root that does substitution or caching in its indexer would implement the same lookup, so it stays self-consistent rather than being second-guessed by the chained provider. It also removes the current internal inconsistency where GetChildKeys already surfaces a present-null key as a child while TryGet reports it as absent.

The reason I would not fold this into the current PR: it is a separate observable breaking change of the same class as the empty-string one (a wrapped present-null would begin to shadow lower providers with null), and it needs its own breaking-change doc and notification. It is also a larger change touching the in-box configuration types and likely an API-review discussion.

Given that, does it make sense to land the empty-string fix here for .NET 11 and keep #65594 open, then do the null work as its own change in .NET 12? That keeps this PR small and low risk, avoids a second break in the same release, and gives the abstraction-level change room for proper review. If you agree, I would keep this PR as "related to" #65594 rather than closing the issue, so the null case stays tracked.

[rosebyte]

I don't see how ConfigurationSection could help us here since IConfiguration returns IConfigurationSection and IConfigurationSection doesn't have TryGetValue method. Anyway, let's merge this PR as we all agree on it and it aligns with the changes done in net10.0 without compromises, and continue this discussion in the issue, what do you think?

[tarekgh]

You are right that TryGetValue lives on the concrete ConfigurationSection, not on IConfigurationSection, so it cannot be reached through the interface. To be clear, I am not proposing to add it to the interface. A default interface method on IConfiguration would not work anyway: the Abstractions package targets net462 and netstandard2.0, and default interface methods need runtime support that .NET Framework does not have, so that route is out, and any new interface member would be a break for third-party implementers.

What I have in mind does not touch the interface:

1. Internal concrete-type dispatch. ChainedConfigurationProvider is in the same assembly as ConfigurationRoot, ConfigurationManager, ConfigurationSection, and the internal TryGetConfiguration. It can switch on those concrete types and use a disposal-safe, pinned bool lookup (the reference-counting the code comment already describes), and fall back to the indexer for any third-party IConfiguration. That confines the fidelity improvement to the in-box types, which is exactly what ConfigurationSection.TryGetValue already does for the .NET 10 null work, and leaves third-party roots unchanged. No public API change.

2. If we ever want a public bool lookup, it would go on the concrete types as a virtual method like ConfigurationSection.TryGetValue, or as an extension, not on the interface.

Either way it is a separate breaking change and a design discussion, so I fully agree: let us merge this PR as is since we all agree it matches the .NET 10 empty/null direction, and continue the null case in #65594. I will keep the PR as "related to" the issue rather than closing it so the null work stays tracked, and we can take up the approach there for .NET 12.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

src/libraries/Microsoft.Extensions.Configuration/tests/ChainedConfigurationProviderTests.cs:167

• Nullability mismatch: IConfigurationProvider.TryGet has an out string? parameter, but this test passes out string. With nullable warnings enabled (and treated as errors in this repo), this can fail the build. Use out string? instead.

            Assert.True(provider.TryGet("Key", out string actual));

src/libraries/Microsoft.Extensions.Configuration/tests/ChainedConfigurationProviderTests.cs:181

• Nullability mismatch: IConfigurationProvider.TryGet has an out string? parameter, but this test passes out string. Switch the out variable to string? to avoid nullable warnings-as-errors.

            Assert.True(provider.TryGet("Key", out string actual));

src/libraries/Microsoft.Extensions.Configuration/tests/ChainedConfigurationProviderTests.cs:195

• Nullability mismatch: IConfigurationProvider.TryGet has an out string? parameter, but this test passes out string. Use out string? to keep the test build clean under nullable warnings.

            Assert.True(provider.TryGet("Key", out string actual));

src/libraries/Microsoft.Extensions.Configuration/tests/ChainedConfigurationProviderTests.cs:314

• Nullability mismatch: TryGet's out parameter is string?, and this test expects it to be null when the key is missing. Declare the out variable as string? so the call and Assert.Null don't produce nullable warnings (which are typically treated as errors).

            Assert.False(provider.TryGet("MissingKey", out string value));
            Assert.Null(value);
        }

src/libraries/Microsoft.Extensions.Configuration/tests/ChainedConfigurationProviderTests.cs:396

• PlainConfiguration implements IConfiguration, whose indexer returns string?. This implementation declares a non-nullable string indexer, which causes nullability-mismatch warnings (and can fail the build if warnings are treated as errors). Match the interface nullability (string?).

            public string this[string key]
            {
                get => _inner[key];
                set => _inner[key] = value;
            }

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[rosebyte]

Created dotnet/docs#55653 to track the compatibility article and linked it to this PR. The needs-breaking-change-doc-created label has been removed.

@rosebyte, please email dotnet/docs#55653 to the .NET Breaking Change Notifications alias (dotnetbcn@microsoft.com) to complete the notification step.

Note

This comment was drafted with AI assistance from GitHub Copilot.