tmenier / Flurl

Fluent URL builder and testable HTTP client for .NET
https://flurl.dev
MIT License
4.23k stars 387 forks source link

Support for Setting HttpRequestMessage.HttpVersionPolicy in FlurlHttpSettings #834

Closed duranserkan closed 2 months ago

duranserkan commented 3 months ago

Hello,

I am the author of the DRN-Project, which provides out-of-the-box solutions for enterprise application development. In this project, we use Flurl as the default HTTP client for its ease of use and testability, as shown in ExternalRequestTests.cs.

Context

I want to utilize prenegotiated H2C (HTTP/2 over TCP) without SSL to benefit from Linkerd's Automatic mTLS.

Meshed pods must be TLS’d by Linkerd, not by the application itself

Current Limitation

Currently, it is possible to set the HTTP version of the HttpRequestMessage using FlurlHttpSettings when the SendAsync method is called with IFlurlRequest, as shown in the source code here:

var reqMsg = new HttpRequestMessage(request.Verb, request.Url) { Content = request.Content, Version = Version.Parse(settings.HttpVersion) };

However, we cannot set the HttpRequestMessage.HttpVersionPolicy using FlurlHttpSettings. It currently defaults to HttpVersionPolicy.RequestVersionOrLower.

Need for Enhancement

As discussed by the .NET team:

A request with HttpRequestMessage.Version set to 2.0 will use HTTP/1.1 if TLS is not used

RequestVersionOrLower: start as high as possible (HTTP/1.1, or ALPN if available) up to HttpRequestMessage.Version

Even when configuring FlurlHttp Client's default DefaultVersionPolicy at program start as shown below:

FlurlHttp.Clients.WithDefaults(builder => { builder.ConfigureHttpClient(client => client.DefaultVersionPolicy = HttpVersionPolicy.RequestVersionExact); });

I still encounter the following error:

"FlurlExceptionHttpResponse": "An HTTP/1.x request was sent to an HTTP/2 only endpoint.", "FlurlExceptionHttpVersion": "2.0", "FlurlExceptionHttpVersionPolicy": "RequestVersionOrLower",

Error Log

{ "ApplicationId": "f248b4ef-2d72-42af-927f-7e9407838ee3", "ApplicationName": "Sample.Hosted", "ExceptionMessage": "Call failed with status code 400 (Bad Request): GET http://localhost:5988/WeatherForecast", "ExceptionStackTrace": " at Flurl.Http.FlurlClient.HandleExceptionAsync(FlurlCall call, Exception ex, CancellationToken token)\n at Flurl.Http.FlurlClient.SendAsync(IFlurlRequest request, HttpCompletionOption completionOption, CancellationToken cancellationToken)\n at Flurl.Http.FlurlClient.SendAsync(IFlurlRequest request, HttpCompletionOption completionOption, CancellationToken cancellationToken)\n at DRN.Framework.Utils.Http.FlurlResponseExtensions.ToJsonAsync[TResponse](Task1 responseTask)", "ExceptionType": "Flurl.Http.FlurlHttpException", "FlurlExceptionCallCompleted": true, "FlurlExceptionCallDuration_Seconds": 0.215086, "FlurlExceptionCallEndedUtc": "2024-08-03T12:56:47.962689Z", "FlurlExceptionCallStartedUtc": "2024-08-03T12:56:47.747603Z", "FlurlExceptionHttpMethod": "GET", "FlurlExceptionHttpResponse": "An HTTP/1.x request was sent to an HTTP/2 only endpoint.", "FlurlExceptionHttpVersion": "2.0", "FlurlExceptionHttpVersionPolicy": "RequestVersionOrLower", "FlurlExceptionHttpVersionRequestUri": "http://localhost:5988/WeatherForecast", "HttpMethod": "GET", "HttpProtocol": "1.1", "HttpScheme": "http", "l5d-client-id": "", "LoggerName": "HttpScopeHandler", "MachineName": "Durans-MacBook-Air", "RequestContentLength": 0, "RequestHost": "localhost:5998", "RequestIpAddress": "::1", "RequestPath": "/WeatherForecast/nexus", "RequestQueryString": "", "ResponseStatusCode": 400, "ScopeCreatedAt": "2024-08-03T12:56:40.549848+00:00", "ScopedLog": true, "ScopeDuration_Seconds": 7.807381, "SocketsHttp2UnencryptedSupport": true, "TraceIdentifier": "0HN5JN4GKSL28:00000001" }

Request

Can we add support to set HttpRequestMessage.HttpVersionPolicy using FlurlHttpSettings? It appears to be a straightforward addition, and I am willing to create a PR if this proposal is acceptable.

To be future-proof, a configuration action can also be set in FlurlHttpSettings for additional HttpRequestMessage configurations.

Thank you for your attention and consideration.

tmenier commented 2 months ago

Even when configuring FlurlHttp Client's default DefaultVersionPolicy at program start...

Any idea why that doesn't work? Have you confirmed whether using HttpClient (without Flurl) and setting DefaultVersionPolicy works as expected? If so, some else is going on - Flurl isn't using the underlying HttpClient that you think it is.

duranserkan commented 2 months ago

The issue arises because HttpRequestMessage sets a version policy when using its default constructor, and this policy always overrides the HttpClient defaults.

According to the .NET documentation, it states:

“This property has no effect on any of the Send or SendAsync overloads that accept a System.Net.Http.HttpRequestMessage.”

Based on this, it appears that the framework behaves as expected, as you correctly suspected. However, this creates a gray area for consumers of Flurl since we currently have no control over the HttpRequestMessage version policy directly.

To resolve this, I believe it would be reasonable for Flurl to check the HttpClient’s default version policy and apply it to HttpRequestMessage when appropriate. Additionally, I suggest enhancing FlurlHttpSettings by providing an option to explicitly set the HTTP version policy. If this option is configured, Flurl can then override the HttpClient’s version policy accordingly.

tmenier commented 2 months ago

Ah, I get it now, thanks. I think you can accomplish this with BeforeCall, which is a hook that can be used to manipulate the HttpRequestMessage just before it's sent. This is available fluently request-by-request, or globally via the builder, which I suspect is the most useful in your case:

FlurlHttp.Clients.WithDefaults(builder =>
    builder.BeforeCall(call => call.HttpRequestMessage.VersionPolicy = HttpVersionPolicy.RequestVersionExact));
duranserkan commented 2 months ago

Thank you for the guidance. Prenegotiated H2C works now with BeforeCall configuration.