Throw BadHttpRequestException in HttpSys

[Kahbazi]

Fixes #20688

[halter73]

LGTM. It better matches Kestrel's and IIS in-proc's behavior.

[Tratcher]

This only covers half of #20688 so far. The other half is to catch the exception and return that status code. That would probobly happen here:

aspnetcore/src/Servers/HttpSys/src/RequestProcessing/RequestContext.cs

Lines 274 to 292 in e9d989f

	catch (Exception ex)
	{
	Logger.LogError(LoggerEventIds.RequestProcessError, ex, "ProcessRequestAsync");
	application.DisposeContext(context, ex);
	if (Response.HasStarted)
	{
	// HTTP/2 INTERNAL_ERROR = 0x2 https://tools.ietf.org/html/rfc7540#section-7
	// Otherwise the default is Cancel = 0x8.
	SetResetCode(2);
	Abort();
	}
	else
	{
	// We haven't sent a response yet, try to send a 500 Internal Server Error
	Response.Headers.IsReadOnly = false;
	Response.Trailers.IsReadOnly = false;
	Response.Headers.Clear();
	Response.Trailers.Clear();
	SetFatalResponse(500);

[Tratcher]

Better. There should be existing tests for this scenario that you can update.

[Tratcher]

Ah, like these:
https://dev.azure.com/dnceng/public/_build/results?buildId=741901&view=ms.vss-test-web.build-test-results-tab

[Tratcher]

Nice. You're still missing test coverage for the new catch behavior. It's probably a combination of one of the tests you updated and something like this:

aspnetcore/src/Servers/HttpSys/test/FunctionalTests/ServerTests.cs

Lines 197 to 213 in 8b79720

	[ConditionalFact]
	public async Task Server_AppException_ClientReset()
	{
	string address;
	using (Utilities.CreateHttpServer(out address, httpContext =>
	{
	throw new InvalidOperationException();
	}))
	{
	Task<string> requestTask = SendRequestAsync(address);
	await Assert.ThrowsAsync<HttpRequestException>(async () => await requestTask);
	// Do it again to make sure the server didn't crash
	requestTask = SendRequestAsync(address);
	await Assert.ThrowsAsync<HttpRequestException>(async () => await requestTask);
	}
	}

[Tratcher]

Thanks