-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Null configuration elements deserialized as empty strings #36510
Comments
Fixes #685 In the current code base, the following is true: * Where null values can be represented in the config (e.g. JSON) they are put into the configuration like any other value. Specifically, no provider skips null values are treats them the same as missing values. * Some providers convert the null values to empty strings; some don't. This change changes those providers (notably JSON) that convert nulls to strings to stop doing this. This is means that where nulls can be represented in the configuration source, they are correctly represented in the resultant configuration. Also added TryGet methods so that, for those who care, it is possible to determine the difference at the top level between a null value and a not-set value. (Note this was always possible, but required traversing the configuration structure and looking at configuration sections/values directly.) This is technically a breaking change since in some cases a configuration value that was returned as an empty string will now be returned as null. However, this seems like a good change to make because it both matches common customer expectations about how it should work, and also allows better handling in our binder and other binders that people may have.
Any update here? This is truly a violation of principle of least surprise. |
Hello, Some days ago I had the same disappointed experience regarding configuration. Let's consider this minimal Json example: {
"obj1": [ ],
"obj2": [
"value"
]
} The
In my own case I add the following configuration: {
"obj1": {
"prop1" : [ ]
},
"obj2": {
"prop1": [
{ "subprop" : "value" }
]
}
} class Conf {
public List<Element> Prop1 {get; set; } = new List<Element>();
}
class Element {
public string SubProp {get; set;} = string.Empty;
} services.Configure<Dictionnary<string, Conf>>(Configuration); For me "objx" keys were clearly meaningful, and I supposed to get a This topic can be splitted into 2 issues:
Issue 1: parent properties lost on unique null Json ArrayWhatever the depth of the Json, if there is only one empty Json array as leaf property, then all parent Json objet are lost. So the Binder may create the default But this lead to the second issue. An alternate solution is to update the configuration as follow (by example): [
{
"name" : "obj1",
"prop1" : [ ]
},
{
"name" : "obj2",
"prop1" : [
{ "subprop" : "value" }
]
}
] But this is to the caller to transform the Issue 2: Distinguish explicit null from explicit empty Json arrayThe main question here is to distinguish an explicit null value from an explicit empty Json Array.
Probably the best may be handle empty collection as
PropositionsI don't have a PR to submit, because I don't have the tooling for the .NET5 and so on. But Issue1 this is related to file JsonConfigurationFileParser. Something like that may solve the issue1 described here. var arrayElements = value.EnumerateArray();
if (arrayElements.Count == 0) {
string key = _currentPath;
if (_data.ContainsKey(key))
{
throw new FormatException(SR.Format(SR.Error_KeyIsDuplicated, key));
}
_data[key] = value.ToString(); // or String.Empty internal JsonElement.ToString() does not already set to string.Empty.
} else {
foreach (JsonElement arrayElement in arrayElements) {
EnterContext(index.ToString());
VisitValue(arrayElement);
ExitContext();
index++;
}
}
break; If I have some time I'll try to search in the binder what to eventually update. and for issue 2 this may be ConfigurationBinder.cs private static object BindInstance(Type type, object instance, IConfiguration config, BinderOptions options)
{
//best update here because value is available before attempting bind on collections.
if (convertedValue == String.Empty || config.GetChildren().Any()) {
...
}
else {
return convertedValue;
}
} |
In the PR #43297 that just got closed we discussed we need to fix this by adding an API, introducing a flag Link to guideline for formal API proposal is available in api-review-process.md |
@vdailly would you be interested in submitting an API proposal for a flag that people can use to enable this behavior? |
Hello, Thanks for your proposal. Issues described above are linked to 2 librairies and doesn't happen at the same time. Flag proposition for issue2 (Binder)For the issue2 described above this may be enabled by adding a property to BinderOptions public class BinderOptions
{
/// <summary>
/// When false (the default), the binder will consider null string as null.
/// If true, the binder will attempt to bind strictly null objets, or create default object. This is useful for Json array handling.
/// </summary>
public bool StrictBinding { get; set; }
} This may be called like this services.Configure<MyObject>(context.Configuration.GetSection("section"), binder => binder.StrictBinding = true ); And finally the Flag proposition for issue1 (Json)For the issue1 this may be more complicated because this happens very early. Add a flag to
|
Wasted an hour today trying to debug my .NET5 application when this was the problem all along... is there a fix yet, or should we just always check for empty string for now? |
I spent a few hours debugging this today. For For local development though, we need to use the There are workarounds--setting Either way, it's a lot of cruft to have to manage when we should have been able to just set Spending several hours tracking this down wasn't fun either. I'm going to open an issue over at Azure to try and get them to check for the empty string in addition to null. But it would be nice to have this built in. I realize the ship has sailed on the default behavior, but something opt-in would be nice. In the meantime we may just add a custom version of the json file provider to our common libraries. As we come to understand the actual behavior, we now realize we have code/configuration that is only working properly by chance as we've made the assumption generally across our stack. |
It's really disappointing that #43297 was just closed.. This may easily lead to simply throwing it away instead of escalating this ridiculous bug. While I can somewhat understand trimming 'empty' subtrees, changing NULLs to ""? Pray tell, why? |
Does anyone have a workaround for this? Very annoying bug. Considering just parsing the json and passing that into my options classes. |
I copy/pasted the code and made my own configuration provider that handled the nulls differently. It works, just something extra to maintain. |
@maryamariyan Is one of the proposals of @vdailly appropriate/considered ? The added JsonOptions class looks pretty non-disruptive to the api and would allow further extensions. |
Just wasted a lot of time on this, still no fix? |
This is ridiculous. It's been 4 years since this was reported. Can we get this resolved or should be just accept it as the standard now? |
Still nothing? |
This causes issues when binding multiple sections to a single object, as the I ended up sanitizing this for config sections I know this is safe to do and necessary, before binding: private static void ChangeEmptyStringToNull(IConfigurationSection configSection)
{
if (configSection == null) return;
if (configSection.Value == "") configSection.Value = null;
foreach (var childConfigSection in configSection.GetChildren())
{
ChangeEmptyStringToNull(childConfigSection);
}
} |
Currently working around this issue by having the property convert to null:
I can get away with this because I never expect an empty string in this value, but it's still not a great solution. I can't believe this is still an issue after 4 years. It's such a basic, easy to hit issue. |
@tarekgh Do you have any plans to fix it? |
We'll look at this at some point in the future. This is why we are keeping the issue open. |
2023, this is still an issue on .Net 7. |
In case anyone missed it in the original post, one workaround is to remove any elements from your json file that are strings and should be null. If the element is not found, then it gets set to NULL instead of empty string. Also, I noticed with non-string nullable types, it handles NULL as expected. Sample JSON File |
Any news on solving the problem? Unexpected behavior when on null in json config return empty string is still here. |
This issue is marked to be addressed in the future release. I am seeing some workaround is mentioned above. can't you use that for now? |
@tarekgh |
My two cents as workaround (but no empty string support):
|
Just ran into a production issue with this, and spent far too much time figuring it out. The problem here is that everything else in the configuration API suggests a null value of a setting should be honored in the deserialization. This is a surprising result, and I can't imagine anyone would expect or rely on this behavior. Now I'm wondering what other ticking time bombs are out there in my other configs. |
Hello, I am replying here just to ask if .Net 8 brought an update regarding this ? |
Due to other high-priority work, we haven't had a chance to address this issue yet. |
This is a surprisingly big trap in configuration. I've known about it for a while, but every couple months, it pops up again as someone adds a new config or a new project is started and hours are lost to debugging. |
As already noted, if the property is completely removed from the appsettings.json file, the value will show up as null. My workaround makes use of this and reads the appsettings.json file into a stream while removing properties that have null values and then using using var appSettingsStream = GetAppsettingStream("appsettings.json");
var builder = new ConfigurationBuilder()
.SetBasePath(Directory.GetCurrentDirectory())
.AddJsonStream(appSettingsStream)
private static MemoryStream GetAppsettingStream(string filename)
{
MemoryStream? tempStream = null;
MemoryStream? stream = null;
try
{
ReadOnlySpan<byte> utf8Bom = new byte[] { 0xEF, 0xBB, 0xBF };
ReadOnlySpan<byte> jsonReadOnlySpan = File.ReadAllBytes(filename);
// Read past the UTF-8 BOM bytes if a BOM exists.
if (jsonReadOnlySpan.StartsWith(utf8Bom))
{
jsonReadOnlySpan = jsonReadOnlySpan.Slice(utf8Bom.Length);
}
tempStream = new MemoryStream();
using var writer = new System.Text.Json.Utf8JsonWriter(tempStream);
var reader = new System.Text.Json.Utf8JsonReader(jsonReadOnlySpan);
string propertyName = string.Empty;
while (reader.Read())
{
System.Text.Json.JsonTokenType tokenType = reader.TokenType;
switch (tokenType)
{
case System.Text.Json.JsonTokenType.StartObject:
if (propertyName == string.Empty)
writer.WriteStartObject();
else
writer.WriteStartObject(propertyName);
break;
case System.Text.Json.JsonTokenType.EndObject:
writer.WriteEndObject();
break;
case System.Text.Json.JsonTokenType.PropertyName:
propertyName = reader.GetString()!;
break;
case System.Text.Json.JsonTokenType.String:
var value = reader.GetString();
writer.WriteString(propertyName, value);
break;
case System.Text.Json.JsonTokenType.Number:
var number = reader.GetInt64();
writer.WriteNumber(propertyName, number);
break;
case System.Text.Json.JsonTokenType.True:
writer.WriteBoolean(propertyName, true);
break;
case System.Text.Json.JsonTokenType.False:
writer.WriteBoolean(propertyName, false);
break;
case System.Text.Json.JsonTokenType.StartArray:
writer.WriteStartArray(propertyName);
break;
case System.Text.Json.JsonTokenType.EndArray:
writer.WriteEndArray();
break;
case System.Text.Json.JsonTokenType.Null:
// This case branch could be left out since we are simply ignoring properties with null values and not copying them to the output stream.
// I'm leaving it in to make it clear what we are doing.
break;
}
}
writer.Flush();
tempStream.Seek(0, SeekOrigin.Begin);
stream = tempStream;
tempStream = null;
}
finally
{
if (tempStream != null)
{
tempStream.Dispose();
}
}
return stream;
} |
I run into this issue too. I'm using .NET 8. |
Describe the bug
When a property is explicitly set a value of
null
in appsettings.json, I expected that to be null after the configuration was loaded, but it actually has a value of""
. This led to a bug in our code that was hard to track down. Ideally, I'd like a way to change this behaviour, although it is possible to work around it, now that I know this is how it behaves.To Reproduce
Steps to reproduce the behavior:
In Visual Studio 2017, create a project of type "ASP.NET Core Web Application".
When prompted for a template, choose the following options:
Add a new class called "MyConfig" and add the following properties:
Add the following elements to the "appsettings.json" file:
(IMPORTANT: Do not add an entry for
Prop4
.)Edit the
Startup
class:Append this line to the
ConfigureServices
method:Append the following parameter to the
Configure
method:Add a breakpoint in the
Configure
method and debug the project in IIS Express.When the breakpoint is hit, inspect the value of the
config
parameter.Results
config.Value.Prop1
"Prop 1 Value"
"Prop 1 Value"
config.Value.Prop2
""
""
config.Value.Prop3
null
""
config.Value.Prop4
null
null
Additional context
Output of
dotnet --info
:The text was updated successfully, but these errors were encountered: