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

Extend DialOptions to allow Host header override #336

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

bendiscz
Copy link
Contributor

The standard http.Request.Host field allows to override the Host header that is sent with the HTTP request (URL.Host is used by default). Such option can be useful in various scenarios, for example when running behind some proxy server.

This patch extends the DialOptions struct with Host field that basically does the same.

@bendiscz bendiscz requested a review from nhooyr as a code owner April 28, 2022 14:11
Copy link
Contributor

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you use Host in HTTPHeader?

@bendiscz
Copy link
Contributor Author

bendiscz commented Mar 6, 2023

Couldn't you use Host in HTTPHeader?

That does not work unfortunately. http.Client.Do ignores Host in HTTPHeader, you need to set the http.Request.Host field if you want to override host from http.Request.URL.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 6, 2023

Sounds good will get this in then after #256

@nhooyr
Copy link
Contributor

nhooyr commented Mar 6, 2023

Or you know what we could actually check for Host in HTTPHeader and special case it to apply to http.Request.Host. The API in net/http isn't something we should follow to the tee anyway as it's hodgepodge.

@bendiscz
Copy link
Contributor Author

Checking for Host in HTTPHeader works too, I just wanted to keep the API similar to net/http... But that is certainly not required :-)

Should I change the patch?

@nhooyr
Copy link
Contributor

nhooyr commented Mar 14, 2023

Go for it and thank you for your contribution. It's still going to take me a while to merge as I need to take care of #256

dejan-lokar added a commit to cookieai-jar/websocket that referenced this pull request Oct 12, 2023
@nhooyr nhooyr changed the base branch from master to dev October 13, 2023 07:59
@nhooyr nhooyr merged commit 2f44d4e into coder:dev Oct 13, 2023
1 of 3 checks passed
@nhooyr
Copy link
Contributor

nhooyr commented Oct 13, 2023

Thanks @bendiscz

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

Successfully merging this pull request may close these issues.

2 participants