Set host header on new request (async/await only) #536
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
### Motivation If we follow a redirect which changes the origin e.g. from `127.0.0.1` to `localhost` we didn't change the `Host` header to the appropriate new origin and port combination. ### Changes Use the original request which does not include the host instead of the prepared request to form a new request to the redirect URL. ### Alternatives If the user defines a `Host` header themselves on the original `HTTPClientRequest` we currently never touch it, even in the redirect case. Maybe we should change our strategy and do one of the following: 1. We could always override the user defined `Host` header 2. We could only remove the user defined `Host` header on redirect and set it to the new origin and port combination
dnadoba
force-pushed
the
dn-async-await-redirect-fix
branch
from
2d3c0e1 to
7e14224
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
If we follow a redirect which changes the origin e.g. from
127.0.0.1tolocalhostwe didn't change theHostheader to the appropriate new origin and port combination.Changes
Use the original request which does not include the host instead of the prepared request to form a new request to the redirect URL.
Alternatives
If the user defines a
Hostheader themselves on the originalHTTPClientRequestwe currently never touch it, even in the redirect case. Maybe we should change our strategy and do one of the following:HostheaderHostheader on redirect and set it to the new origin and port combination