Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-66897: Upgrade HTTP CONNECT to protocol HTTP/1.1#8305

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

Merged
orsenthil merged 17 commits intopython:mainfromhandlerbot:fix-bpo-22708
Apr 5, 2023
Merged

gh-66897: Upgrade HTTP CONNECT to protocol HTTP/1.1#8305

orsenthil merged 17 commits intopython:mainfromhandlerbot:fix-bpo-22708
Apr 5, 2023

Conversation

@handlerbot
Copy link
Contributor

@handlerbothandlerbot commentedJul 16, 2018•
edited by bedevere-bot
Loading

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

roman-vynar, arhadthedev, bgehman, 1sixth, and pwlae-transferwise reacted with thumbs up emoji
Use protocol HTTP/1.1 when sending HTTP CONNECT tunnelling requests;generate Host: headers if one is not already provided (required byHTTP/1.1), convert IDN domains to punycode in HTTP CONNECT requests.
@handlerbot
Copy link
ContributorAuthor

Working on fix for BytesWarning now.

@berkerpeksagberkerpeksag self-requested a reviewJuly 17, 2018 14:07
@berkerpeksag
Copy link
Member

Thanks for updating that PR, I've just added this to my TODO list, but it may take a while.

handlerbot reacted with thumbs up emoji

@handlerbot
Copy link
ContributorAuthor

Hi@berkerpeksag, circling back to this one, do you still have time and interest in the topic, or should we find another reviewer?

@akhayyat
Copy link

Is this going to be merged any time soon?

@csabellacsabella requested review fromberkerpeksag and removed request forberkerpeksagFebruary 6, 2020 22:42
@berkerpeksag
Copy link
Member

Sorry, I will try to review this PR this weekend.

0weight and nametkin reacted with thumbs up emoji

@keelson
Copy link

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

stevanmilic reacted with thumbs up emoji

@merwok
Copy link
Member

It crashes in _tunnel on response line:

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?

@merwok
Copy link
Member

Please answer: what exactly do you see when you say «it crashes»?

@handlerbot
Copy link
ContributorAuthor

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 forLib/http/client.py in the past...

@handlerbot
Copy link
ContributorAuthor

Sigh, I did mean@sethmlarson not@SethMichaelLarson, sorry about that, the GitHub autocompleter was determined to mislead me!

@orsenthilorsenthil self-assigned thisMar 5, 2021
@orsenthil
Copy link
Member

Hi Michael, Thanks for following up. I will be able to review and bring it to a closure.

handlerbot reacted with hooray emoji

@yilinjuang
Copy link

any updates on this? would love to see this merge as it prevents our services from using certain proxies. thanks

@lukaszgalezewski
Copy link

why this PR is still open and not merged? and when can be merged? :)

bgehman and nametkin reacted with thumbs up emoji

@orsenthil
Copy link
Member

@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.

rhuddleston and GtTmy reacted with thumbs up emoji

@l8huang
Copy link

@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.

l8huang reacted with thumbs up emoji

@roman-vynar
Copy link

3.12, if features are allowed

It's not a feature, it's a shame. HTTP 1.0 is horribly obsolete,host header was added to HTTP 1.1 in 1997, this is 3000 years ago.
This ticket was open in 2018. Today we are in 2023. If this is merged into 3.12 for October it will be great.

sabretus, Oneiroi, nametkin, and jskacel reacted with thumbs up emoji1sixth, Oneiroi, and rhuddleston reacted with laugh emoji

@arhadthedevarhadthedev changed the titlebpo-22708: Upgrade HTTP CONNECT to protocol HTTP/1.1gh-66897: Upgrade HTTP CONNECT to protocol HTTP/1.1Mar 22, 2023
@arhadthedevarhadthedev added the stdlibStandard Library Python modules in the Lib/ directory labelMar 22, 2023
@arhadthedev
Copy link
Member

@vstinner@gpshead could you review and possible merge this PR please? (as committers intoLib/http/client.py)

roman-vynar, rhuddleston, and wwonigkeit reacted with heart emoji

Copy link
Member

@orsenthilorsenthil left a comment

Choose a reason for hiding this comment

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

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.

roman-vynar, 1sixth, bgehman, kkitayam, GtTmy, hugovk, and pwlae-transferwise reacted with hooray emoji
@orsenthilorsenthil merged commit1a8f862 intopython:mainApr 5, 2023
gaogaotiantian pushed a commit to gaogaotiantian/cpython that referenced this pull requestApr 8, 2023
* 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 byHTTP/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.pyCo-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>
warsaw pushed a commit to warsaw/cpython that referenced this pull requestApr 11, 2023
* 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 byHTTP/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.pyCo-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>
@hugovk
Copy link
Member

@orsenthil Shall we also backport this to 3.11?

chicocvenancio reacted with thumbs up emoji

@gpshead
Copy link
Member

@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.

orsenthil, sethmlarson, and hugovk reacted with thumbs up emojichicocvenancio reacted with thumbs down emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@merwokmerwokmerwok left review comments

@ammaraskarammaraskarammaraskar left review comments

@orsenthilorsenthilorsenthil approved these changes

@berkerpeksagberkerpeksagAwaiting requested review from berkerpeksag

+2 more reviewers

@bgehmanbgehmanbgehman left review comments

@nametkinnametkinnametkin left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@orsenthilorsenthil

Labels

stdlibStandard Library Python modules in the Lib/ directory

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

21 participants

@handlerbot@berkerpeksag@akhayyat@keelson@merwok@orsenthil@yilinjuang@SadPencil@MaxwellDupre@roman-vynar@lukaszgalezewski@l8huang@arhadthedev@hugovk@gpshead@ammaraskar@bgehman@nametkin@the-knights-who-say-ni@ezio-melotti@bedevere-bot
[8]ページ先頭
©2009-2025 Movatter.jp