gh-66897: Upgrade HTTP CONNECT to protocol HTTP/1.1 #8305
Conversation
Use protocol HTTP/1.1 when sending HTTP CONNECT tunnelling requests; generate Host: headers if one is not already provided (required by HTTP/1.1), convert IDN domains to punycode in HTTP CONNECT requests.
|
Working on fix for BytesWarning now. |
|
Thanks for updating that PR, I've just added this to my TODO list, but it may take a while. |
|
Hi @berkerpeksag, circling back to this one, do you still have time and interest in the topic, or should we find another reviewer? |
|
Is this going to be merged any time soon? |
csabella
requested review from
berkerpeksag
and removed request for
berkerpeksag
|
Sorry, I will try to review this PR this weekend. |
|
any update? this bug currently blocks any integretion with the linkerd2 proxy as this throws a 400 bad request for connections inititaded by python apps https://github.com/linkerd/linkerd2/pull/1200/files |
Can you show the exact error for this? «crash» usually describes a segfault of the interpreter, is that what happended or was it a regular Python exception? |
|
Please answer: what exactly do you see when you say «it crashes»? |
|
Hi folks -- would love to try again to get this merged. :-) cc @berkerpeksag who I solicited the original review from, and tagging in @orsenthil and @SethMichaelLarson (I think this is the same GitHub identity for the person who approved #20959, that's who I am looking for, sorry if I tagged the wrong person) who have also reviewed PRs for |
|
Sigh, I did mean @sethmlarson not @SethMichaelLarson, sorry about that, the GitHub autocompleter was determined to mislead me! |
|
Hi Michael, Thanks for following up. I will be able to review and bring it to a closure. |
|
any updates on this? would love to see this merge as it prevents our services from using certain proxies. thanks |
|
why this PR is still open and not merged? and when can be merged? :) |
|
@handlerbot , @lukaszgalezewski - apologies. None of us go to it. I will review this in coming week and target for upcoming Python release (3.12, if features are allowed) otherwise we can go with 3.13. |
|
@orsenthil do you have any update on this? Hope this can be merged in python 3.12. I recently run into this issue, even latest python can't set protocol version for HTTP CONNECT is kind of embarrassment. Thanks. |
It's not a feature, it's a shame. HTTP 1.0 is horribly obsolete, |
arhadthedev
changed the title
There was a problem hiding this comment.
This looks good to me.
I reviewed for the choices made for handling unicode, and sending Host with HTTP 1.1 for CONNECT, when not specified. And verified if it is backwards compatible.
The feedback in the comments seem to say this helps for folks who had a need too.
This change looks good to me.
Apologize for taking long time.
* bpo-22708: Upgrade HTTP CONNECT to protocol HTTP/1.1 (GH-NNNN) Use protocol HTTP/1.1 when sending HTTP CONNECT tunnelling requests; generate Host: headers if one is not already provided (required by HTTP/1.1), convert IDN domains to punycode in HTTP CONNECT requests. * Refactor tests to pass under -bb (fix ByteWarnings); missed some lines >80. * Use consistent 'tunnelling' spelling in Lib/http/client.py * Lib/test/test_httplib: Remove remnant of obsoleted test. * Use dict.copy() not copy.copy() * fix version changed * Update Lib/http/client.py Co-authored-by: bgehman <bgehman@users.noreply.github.com> * Switch to for/else: syntax, as suggested * Don't use for: else: * Sure, fine, w/e * Oops * 1nm to the left --------- Co-authored-by: Éric <merwok@netwok.org> Co-authored-by: bgehman <bgehman@users.noreply.github.com> Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>
* bpo-22708: Upgrade HTTP CONNECT to protocol HTTP/1.1 (GH-NNNN) Use protocol HTTP/1.1 when sending HTTP CONNECT tunnelling requests; generate Host: headers if one is not already provided (required by HTTP/1.1), convert IDN domains to punycode in HTTP CONNECT requests. * Refactor tests to pass under -bb (fix ByteWarnings); missed some lines >80. * Use consistent 'tunnelling' spelling in Lib/http/client.py * Lib/test/test_httplib: Remove remnant of obsoleted test. * Use dict.copy() not copy.copy() * fix version changed * Update Lib/http/client.py Co-authored-by: bgehman <bgehman@users.noreply.github.com> * Switch to for/else: syntax, as suggested * Don't use for: else: * Sure, fine, w/e * Oops * 1nm to the left --------- Co-authored-by: Éric <merwok@netwok.org> Co-authored-by: bgehman <bgehman@users.noreply.github.com> Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>
|
@orsenthil Shall we also backport this to 3.11? |
No, this is more of a feature. it's a notable behavior change, not a mere unintended behavior fix. |
Use protocol HTTP/1.1 when sending HTTP CONNECT tunnelling requests; generate Host: headers if one is not already provided (required by HTTP/1.1), convert IDN domains to punycode in HTTP CONNECT requests.
This builds upon and obsoletes #742. cc @berkerpeksag who reviewed that PR.
(I have signed the CLA, but it was less than a day ago, so the human review may not yet be completed.)
https://bugs.python.org/issue22708