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

Remove modification of HttpClient DefaultRequestHeaders #2156

Open
PetesBreenCoding opened this issue Nov 2, 2023 · 3 comments
Open

Remove modification of HttpClient DefaultRequestHeaders #2156

PetesBreenCoding opened this issue Nov 2, 2023 · 3 comments

Comments

@PetesBreenCoding
Copy link
Contributor

I can create a PR to do this, but I wanted to run it by someone first to make sure it's acceptable, thanks.

Is your feature request related to a problem? Please describe.
If you are re-using a single HttpClient instance in your application (as is recommended by MS) and passing it into multiple instances of RestClient, that HttpClient instance may be modified by the RestSharp.RestClient.ConfigureHttpClient method. For example, the UserAgent header is added to the DefaultRequestHeaders property.

This may be an issue if your RestClient instances are for different APIs that expect different UserAgents. What happens is that you end up with multiple UserAgents and the API you're calling may fail to recognise it. Now, the API probably should be checking if the UserAgent contains a certain string, not equals, but I still think it would be better if the HttpClient instance was left untouched and the UserAgent set on the request message instead.

Describe the solution you'd like

  • ChangeRestSharp.RestClient.ConfigureHttpClient to only set the Timeout property (I think this is the only property that can't be set on each request)
  • Move the setting of UserAgent and ExpectContinue to RestSharp.RestClient.ExecuteRequestAsync and set them on the message.Headers property.

Describe alternatives you've considered
Use multiple instances of HttpClient, or ask for the API being called to use "contains" rather than "equals"

@PetesBreenCoding PetesBreenCoding changed the title Remove modification of HttpClient properties Remove modification of HttpClient DefaultRequestHeaders Nov 2, 2023
@alexeyzimarev
Copy link
Member

Yeah, I think it makes sense. I would not add those headers in Execute but rather add them as default headers in the constructor.

@PetesBreenCoding
Copy link
Contributor Author

Thanks for the quick response, yes the default headers makes sense. I'll work on a PR for it.

@alexeyzimarev
Copy link
Member

The UserAgent is moved to default RestClient headers, but Expect100Continue is a bit wicked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants