Invalid nullability annotation on HttpRequest.ContentType

[bart-degreed]

After updating my Web API project from .NET Core 3.1 to .NET 5, I'm getting a warning along the lines of "variable is never null".

var contentType = httpContext.Request.ContentType;

if (contentType != null && contentType != allowedContentType)
{
// ^^^^^^^^^^^^^
}

The warning is incorrect, because when I remove the null check, lots of my integration tests start to fail. Specifically, the ones where no Content-Type header is sent in the request.

This makes me believe the HttpRequest.ContentType property type should be string? instead of string.

[ghost]

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[BrennanConroy]

Good catch thanks! Interested in contributing a change for this?

[bart-degreed]

Thanks for the offer, but I'd be happy to give someone else the opportunity to take this.

[KellenCarl]

I'm interested in contributing. This would be my first non-doc contribution. If you and/or other reviewers don't mind helping me through the process, I'd be more than happy to do the "leg-work." Just let me know what needs to be done, and I can give it my best. I've read the contribution guidelines and code of conduct.

Are you looking to just have the "?" added to the HttpRequest.ContentType to make the type nullable, and run a unit test for both type string and null?

[BrennanConroy]

Are you looking to just have the "?" added to the HttpRequest.ContentType to make the type nullable

Yep! The trick with changing APIs is that we need to update the baseline files. You can read about that at https://github.com/dotnet/aspnetcore/blob/main/docs/APIBaselines.md#updated-apis. Essentially a *REMOVED*some.api and some.api? will be added to the unshipped.txt file.

Another thing with updating nullability on an API is that it can affect other code that might be using the changed API and didn't expect the property to ever be null. Fortunately the PR builds will find any of those, but building a couple projects locally can help find some of those quicker so there is less PR churn 😃

run a unit test for both type string and null?

Don't think that's needed for this change, we're not changing any behavior or adding new code, but good thought 👍

[KellenCarl]

Ok. Sounds good. I'll start taking a look into this.

[Tratcher]

aspnet/Announcements#444 (comment)

[bart-degreed]

Thanks for the fix! Which release will include it?

[Tratcher]

6.0-preview5. since this is an api change it's not something we can backport.