- Notifications
You must be signed in to change notification settings - Fork13.3k
Added support to override host header.#9092
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nilo85 commentedFeb 26, 2024
I no longer need this hack, however, there might be situations you need to override host header (proxy use etc) so might still make sense to support |
mcspr commentedFeb 27, 2024
Perhaps the client should allow |
nilo85 commentedFeb 27, 2024
@mcspr I suppose, however to be completely http compliant, you can pass multiple values for same header, most other http implementations do setHeader and addHeader, where setHeader sets a single header and replaces whatever would have been there before, and addHeader would send multiple values. However in this case, judging by the code I suspect the restriction here is to kep the code simple as the host header is hardcoded in the request call. I would be fine rejecting this PR as I no longer need it, but possible it could make sense to consider adopting more http client features in the future, if needed... I suspect for most applications using these microchips, explicit proxy settings etc is overkill so I personally would consider it a corner case |
Asdns is not working on my network I decided to workaround the issue for now and figured this might actually be a feature we want here =)