-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Send only one Host header in chunked request #5391
Conversation
e982e4c
to
73f88ce
Compare
4de7ad2
to
bc3b46e
Compare
925dd80
to
24a3641
Compare
Hi @AaronRobson :) |
requests/adapters.py
Outdated
@@ -459,7 +459,8 @@ def send(self, request, stream=False, timeout=None, verify=True, cert=None, prox | |||
try: | |||
low_conn.putrequest(request.method, | |||
url, | |||
skip_accept_encoding=True) | |||
skip_accept_encoding=True, | |||
skip_host='Host' in request.headers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's good, this appears to be the same way that the same situation is handled in the urllib3 code itself: https://github.com/urllib3/urllib3/blob/74d6be1ab66cef44c0f479c24b0fc1756a8fe4e9/src/urllib3/connection.py#L243-L246
24a3641
to
1936aab
Compare
1936aab
to
4ab2550
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for digging in a finding a simple solution here, @hodbn! This seems like a straight forward win that alleviates the risks of other patches we've had for #5274. Apologies this has been sitting so long.
I've added some minor test refactoring to make these easier to expand later and rebased your changes onto our main
branch. I think this should be set!
@nateprewitt What's the state of this PR? |
It's ready, it just needs to be rebased to rerun tests and we can merge. It's on my list for Monday now that 2.27.0 is unblocked. |
Thanks again @hodbn! |
Send only one Host header when specifying a custom Host header in a chunked request.
Before this fix, the default Host header was sent along with the custom Host header.
Closes #5274