Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]:Add property to HttpClient #110555

Closed
Mr0N opened this issue Dec 9, 2024 · 7 comments
Closed

[API Proposal]:Add property to HttpClient #110555

Mr0N opened this issue Dec 9, 2024 · 7 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Http

Comments

@Mr0N
Copy link

Mr0N commented Dec 9, 2024

Background and motivation

I propose adding a property called ClientHandler to HttpClient, which would allow direct access to the underlying HttpClientHandler. When creating an instance of HttpClient, parameters like cookies or connection settings can be configured. However, once the object is passed to a class, there's no way to modify these settings further.

сlass Test(HttpClient client)
{
   public void Execute()
    {
     // In this class, it's impossible to configure cookies or other parameters
    }
}

API Proposal

class  HttpClient
{
    public HttpClientHandler ClientHandler {get;}
}

API Usage

var message = new HttpClientHandler();
var client = new HttpClient(message);

void Execute(HttpClient client)
{
    client.ClientHandler.CookieContainer = null;
}

Alternative Designs

No response

Risks

No response

@Mr0N Mr0N added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 9, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Dec 9, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@Mr0N Mr0N changed the title [API Proposal]: [API Proposal]:HttpClientHandler Dec 9, 2024
@Mr0N Mr0N changed the title [API Proposal]:HttpClientHandler [API Proposal]:Add property to HttpClient Dec 9, 2024
@MihaZupan
Copy link
Member

An HttpClient may not be backed by an HttpClientHandler as the user can provide a custom HttpMessageHandler.

It's also not possible to change settings on HttpClientHandler after you've sent a request. See #77668 (comment)

@Mr0N
Copy link
Author

Mr0N commented Dec 10, 2024

An HttpClient may not be backed by an HttpClientHandler as the user can provide a custom HttpMessageHandler.

It's also not possible to change settings on HttpClientHandler after you've sent a request. See #77668 (comment)

I modified it this way, but I’d like to do it without hacks.

Handler.SetToHandlerCookies(data.container);

    public class Handler: HttpClientHandler
    {
        public static void SetToHandlerCookies(CookieContainer cookies)
        {
            foreach (var item in _hand)
            {
                item.CookieContainer = cookies;
            }
        }
        static ConcurrentBag<Hand> _hand = new ConcurrentBag<Hand>();
        public Handler()
        {
            _hand.Add(this);
        }
    }
builder.Services.AddHttpClient()
     .ConfigureHttpClientDefaults(http =>
     {
         http.ConfigurePrimaryHttpMessageHandler(() =>
         {
             var handler = new Handler();
             handler.AllowAutoRedirect = true;
             handler.UseProxy = true;
           /// handler.Proxy = new WebProxy("127.0.0.1", 8888);
             return handler;
         });
     });

@DL444
Copy link

DL444 commented Dec 10, 2024

Wouldn't sharing CookieContainer between primary handlers lead to unexpected information disclosure? IMHO binding cookies to the primary handler is the culprit of most cookie-related issues with IHttpClientFactory. Cookies should really be a session-specific state where "session" is defined by the consumers. It's unfortunate that things have historically been designed this way.

@CarnaViire
Copy link
Member

Per-session / per-request overrides (cookies, proxies, default headers, certs, etc) are already tracked in

Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@MihaZupan
Copy link
Member

The proposed API is problematic for the reasons called out in #110555 (comment).

The scenario itself is already covered by existing discussions that Natalia linked - #110555 (comment).

@MihaZupan MihaZupan closed this as not planned Won't fix, can't repro, duplicate, stale Dec 10, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Http
Projects
None yet
Development

No branches or pull requests

4 participants