SocketsHttpHandler: Add a switch to allow non-ascii headers

[antonfirsov]

.NET Core's SocketHttpHandler does not allow sending non-ASCII characters in HTTP headers according to RFC, however in reality services accept/require other characters/encodings.

In dotnet/runtime#38711 we are introducing a generic solution for .NET 5. This is a 3.1-only workaround exposing two switches to relax header validation globally before the first usage of HttpClient, enabling advanced users to send Latin-1 encoded headers:

• AppContext switch: System.Net.Http.SocketsHttpHandler.AllowLatin1Headers
• Environment variable: DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWLATIN1HEADERS

Customer Impact

Unblock migration to .NET Core 3.1 for customers who need to send Latin-1 headers.

Testing

The PR adds tests (and minimal necessary test infrastructure changes) based on the ones in dotnet/runtime#39468 and dotnet/runtime#39169 to make sure these scenarios do not regress.

Validation: we sent private builds to the partner asking for this feature, and they confirmed that the change fixes their issue.

Risk

Low. The workaround is hidden behind a switch, our default behavior remains unchanged.

[antonfirsov]

I realized my current tests are far from being correct. Converting to draft until I fix them & address the review comment from Chris.

Done

Addressed

[MihaZupan]

Change LGTM

[ManickaP]

LGTM, thanks!

[antonfirsov]

No test failures are related to the change. (System.Text.RegularExpressions.Tests and System.Diagnostics.Tests.PerformanceDataTests do not use System.Net.Http)

[antonfirsov]

@danmosemsft can you help pushing this forward for servicing approval?

[danmoseley]

@antonfirsov usual procedure - create a port PR, @karelz supports it, let me know..

[danmoseley]

Oops, this is the PR. Got used to seeing 3.1 in the title.

[danmoseley]

Please double check that tests that set appcontext/set environment variables aren't affecting other tests in the same process.

[antonfirsov]

All tests are isolated using RemoteExecutor. The test methods are invoked by derived classes. Couldn't define them in the base class because of RemoteExecutor limitations -- it's not able to invoke delegates defined in an abstract class, since it has no info about what to instantiate.

[danmoseley]

I see it - thanks

[karelz]

@danmosemsft now that it is approved -- can we merge it, or should we wait for someone to tell us to do that?

[danmoseley]

You can go ahead and merge 5.0 PR's when marked servicing-approved plus the usual requirements - code reviewed and tests are good.

[antonfirsov]

You can go ahead and merge 5.0 PR's

I hope this is also true for 3.1-only servicing PR-s :)

[ManickaP]

Little nits, otherwise LGTM, thanks!

[danmoseley]

I hope this is also true for 3.1-only servicing PR-s :)

Oops, my bad. For 3.1/2.1, we normally have @Anipik do the merge, as he knows when the branch is open. Let him know if this is ready to merge.

[Anipik]

No Packaging changes required here. We can merge this once the feedback is addressed the ci is green

[danmoseley]

Ah yes that's the other reason it's good for @Anipik to merge the 2.1/3.1 changes -- because he helps check the packaging was done correctly - something we've gotten wrong more than once.

[antonfirsov]

@Anipik this PR is ready to merge:
important feedback was addressed, changes got approval, failures are unrelated flaky tests (System.Text.RegularExpressions.Tests and System.Diagnostics.Tests.PerformanceDataTests do not use System.Net.Http).