HttpClient.DefaultRequestHeaders not applied for all Send overloads

patagonaa commented 6 months ago

Library/API/IoT binding

nanoFramework.System.Net.Http

Visual Studio version

No response

.NET nanoFramework extension version

No response

Target name(s)

No response

Firmware version

No response

Device capabilities

No response

Description

When using the HttpClient and adding DefaultRequestHeaders and calling client.Send(HttpRequestMessage) afterwards, the request is sent without the default headers.

Taking a look at the code, it looks like the DefaultRequestHeaders are applied in HttpClient.Send(HttpRequestMessage. HttpCompletionOption). However when calling Send without the HttpCompletionOption, HttpMessageInvoker.Send(HttpRequestMessage) (from the base class) is called directly, which does not apply the default headers.

My proposed fix would be to override HttpMessageInvoker.Send(HttpRequestMessage) and apply all the options there instead (as that is called either way)

How to reproduce

No response

Expected behaviour

HttpClient.DefaultRequestHeaders are applied no matter which method is used

Screenshots

No response

Sample project or code

var client = new HttpClient();

client.DefaultRequestHeaders.Add("Content-Type", "application/json");

var request = new HttpRequestMessage(HttpMethod.Post, uri)
{
    Content = new StringContent("{}")
};

// does not work (request is sent but without header)
var response = client.Send(request);

// works (correctly sends header)
var response = client.Send(request, HttpCompletionOption.ResponseContentRead);

Aditional information

No response

Ellerbach commented 6 months ago

Is your URI a SSL one or non SSL?

The only difference between the base class and the HttpClient is on threating the SSL elements:

patagonaa commented 6 months ago

Is your URI a SSL one or non SSL?

Non-SSL, but it doesn't really matter

The only difference between the base class and the HttpClient is on threating the SSL elements

I'm talking about

This method sets the request used and handles the BaseAddress, DefaultRequestHeaders, etc.

When just calling HttpClient.Send(HttpRequestMessage) the base classes' method (HttpMessageInvoker.Send(HttpRequest)) is called directly and the setup regarding BaseAddress, DefaultRequestHeaders isn't done. Also, because

SendWorker(HttpRequestMessage, HttpCompletionOption) (the second method you linked) is never called either though, so Timeout, SSL, etc. is not set up either.

Ellerbach commented 6 months ago

OK, I see, right. Now, the key question is what's the best way to fix this? I think a simple override of Send in the HttpClient class calling the right function with a default on HttpCompletionOption.ResponseHeadersRead.

As for the other issue, as it's community driven, super happy if you think it's a good idea to do this, then happy to review your PR :-)

nanoframework / Home