Don't re-use DefaultHttpContext if IHttpContextAccessor is in use

[davidfowl]

• Consumers may still get null or an ODE but will never end up with data from a different request.
• Make sure an ODE is thrown from all properties on HttpContext after the request is over.

Fixes #14975

cc @benaadams

[davidfowl]

These aren't new but I renamed files (my bad).

• HostingApplicationDiagnosticsTests used to be HostingApplicationTests
• HostingApplicationTests contain the new tests

[benaadams]

I uses git mv to ensure it git picks up file renames

[benaadams]

Oh that's nice :)

[davidfowl]

@anurse, this is something we should patch in the 3.1 timeframe

[analogrelay]

Who approved this for 3.1?

[davidfowl]

@anurse nobody, it's in master

[analogrelay]

Horp, missed that. Sorry! :)

[davidfowl]

I still think we should backport this to 3.1. I'll open an issue (or can I just do a PR?)

[Pilchie]

PR is fine.

[ldwedari]

If possible, this should be backported to 2.1 and 2.2 as soon as possible. It pottentially can cause serious issues to existing applications with wrong impersonation.

[davidfowl]

This never existed on 2.2 and 2.1 that’s a different issue.

[ldwedari]

This never existed on 2.2 and 2.1 that’s a different issue.

@davidfowl I created this demonstration based on the code that you posted in #14975.
In 2.1.19 (master branch) there is a data race. But in 3.1.5 (TestWithNetCore3_1 branch) I cannot see it happening.

Is my test wrong? If it is correct, does this actually happens in 3.1.5?

[ldwedari]

Just a clarification. I cannot reproduce the data race issue with HttpContext in asp.net core 2.2. But it does occur with all versions of 2.1.

[davidfowl]

@ldwedari This is using IIS or Kestrel?

[ldwedari]

@davidfowl Kestrel. In fact, in my test I removed the IIS configuration from launchsettings.json.

[davidfowl]

I had to dig into the code archives to remember the 2.1 behavior. In 2.1 we never cleared the HttpContext state after the request is over which means its entirely possible to get values from the next request since the HttpContext points to the same feature collection when using Kestrel on the same connection.

This isn't related to the HttpContextAccessor itself, but it would also happen if you held onto the HttpContext longer than the request.