aspnetcore: (string[])Request.Headers["NonExistentHeader"] is null while (IEnumerable)Request.Headers["NonExistentHeader"] is not
Is there an existing issue for this?
- I have searched the existing issues
Describe the bug
Consider the following code:
string value = string.Join("\n", Request.Headers["NonExistentHeader"]);
It causes the following exception in runtime at method string.Join(string separator, string[] value):
ArgumentNullException: Value cannot be null. (Parameter 'value')
For a very strange reason, the implicit cast of an empty StringValues struct to string[] type returns null:
string[] arr = default(StringValues);
Console.WriteLine("string[] is null: {0}", arr is null); // prints 'string[] is null: True' message
While a similar cast to IEnumerable<string> returns an expected non-null value:
var seq = (IEnumerable<string>)default(StringValues);
Console.WriteLine("IEnumerable<string> is null: {0}", seq is null); // prints 'IEnumerable<string> is null: False' message
When C# compiler compiles string.Join("\n", Request.Headers["NonExistentHeader"]) expression, it selects string.Join(string separator, string[] value) overload and this makes the expression to crash at runtime. While the project has nullable checks enabled, C# compiler is not able to catch that null reference error at compile time.
It looks like a bug because structs are never expected to be null and thus their cast operators should avoid producing null values as well, unless there is a very good reason for that.
The following approach allows to workaround the issue:
string value = string.Join("\n", (IEnumerable<string>)Request.Headers["NonExistentHeader"]);
It would be nice to have a consistent behavior here (non-null string[] is expected and preferred). But even if there is a null then C# compiler should be able to catch it at compile time.
Expected Behavior
(string[])default(StringValues) aways returns a non-null value.
Steps To Reproduce
string value = string.Join("\n", Request.Headers["NonExistentHeader"]);
Exceptions (if any)
ArgumentNullException: Value cannot be null. (Parameter ‘value’) at string.Join(string separator, string[] value)
.NET Version
6.0.401
Anything else?
Server: Kestrel
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 24 (14 by maintainers)
I think we can change Kestrel’s KnownHeaders to default to
StringValues.Emptyinstead ofdefault. This will align better with both the plainHeaderDictionarytype andQueryCollectiontype.https://github.com/dotnet/aspnetcore/blob/de8b5afeb180779c2c16f22cc2e392cb4cf59ed6/src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.cs#L47-L48
https://github.com/dotnet/aspnetcore/blob/de8b5afeb180779c2c16f22cc2e392cb4cf59ed6/src/Servers/Kestrel/shared/KnownHeaders.cs#L958-L960
https://github.com/dotnet/aspnetcore/blob/de8b5afeb180779c2c16f22cc2e392cb4cf59ed6/src/Http/Http/src/HeaderDictionary.cs#L63-L77
https://github.com/dotnet/aspnetcore/blob/de8b5afeb180779c2c16f22cc2e392cb4cf59ed6/src/Http/Http/src/QueryCollection.cs#L64-L79
This will make minimal APIs handling of
string[]header parameters just as bad (or good) asstring[]query string parameters, but at least it will be consistent. See #45956I’m not sure this suggestion is inline with the original design of
StringValues. Reading https://github.com/aspnet/Announcements/issues/60, it says:Changing the implicit conversion to
string[]is going to lose the “nullness” of it. Take the following code as an example:This seems pretty inconsistent to be implicitly convertible to and from a Type, but lose information when you do.
To back up this point, see https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/operator-overloads
It seems that the definition was updated in .NET 7 to include the correct nullability annotation for the implicit cast operator.
However, it does not generally look like a good design decision IMHO. I guess there may be some compatibility concerns, but the current behavior causes a friction that could be absent otherwise.