GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-66897: Upgrade HTTP CONNECT to protocol HTTP/1.1 by handlerbot · Pull Request #8305 · python/cpython

Conversation

@handlerbot

@handlerbot handlerbot commented Jul 16, 2018 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

  • Issue: httplib/http.client in method _tunnel used HTTP/1.0 CONNECT method #66897

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
roman-vynar, arhadthedev, bgehman, 1sixth, and pwlae-transferwise reacted with thumbs up emoji
All reactions
  • 👍 5 reactions
@handlerbot
bpo-22708: Upgrade HTTP CONNECT to protocol HTTP/1.1 (GH-NNNN)
9f5ce09 
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.
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jul 16, 2018
@bedevere-bot bedevere-bot added the awaiting review label Jul 16, 2018
@handlerbot

handlerbot commented Jul 16, 2018

Copy link
Copy Markdown
Contributor Author

Working on fix for BytesWarning now.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@handlerbot
Refactor tests to pass under -bb (fix ByteWarnings); missed some line…
c3927ed 
…s >80.
@berkerpeksag
berkerpeksag self-requested a review July 17, 2018 14:07
@berkerpeksag

berkerpeksag commented Jul 17, 2018

Copy link
Copy Markdown
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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@berkerpeksag berkerpeksag mentioned this pull request Jul 17, 2018
bpo-22708: Fix http protocol version in CONNECT method, support IDN #742
Closed
handlerbot added 2 commits July 19, 2018 17:27
@handlerbot
Use consistent 'tunnelling' spelling in Lib/http/client.py
2fe3ade
@handlerbot
Lib/test/test_httplib: Remove remnant of obsoleted test.
b2f76d7
handlerbot
handlerbot commented Jul 20, 2018
View reviewed changes
Comment thread Doc/library/http.client.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
merwok
merwok reviewed Jul 23, 2018
View reviewed changes
Comment thread Lib/http/client.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@handlerbot
Use dict.copy() not copy.copy()
9990baa
@handlerbot

handlerbot commented Jan 27, 2019

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@akhayyat

akhayyat commented Jul 2, 2019

Copy link
Copy Markdown

Is this going to be merged any time soon?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@csabella
csabella requested review from berkerpeksag and removed request for berkerpeksag February 6, 2020 22:42
@berkerpeksag

berkerpeksag commented Feb 7, 2020

Copy link
Copy Markdown
Member

Sorry, I will try to review this PR this weekend.
0weight and nametkin reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@0weight 0weight mentioned this pull request Feb 22, 2020
No "host" field in HTTP header in https proxy session boto/botocore#1981
Closed
@keelson

keelson commented Sep 2, 2020

Copy link
Copy Markdown

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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mirkokg mirkokg mentioned this pull request Sep 23, 2020
Host header not being set for CONNECT method of HTTPS proxy psf/requests#5011
Open
@merwok

merwok commented Sep 23, 2020

Copy link
Copy Markdown
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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@merwok

merwok commented Sep 23, 2020

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@handlerbot

handlerbot commented Mar 5, 2021

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@handlerbot

handlerbot commented Mar 5, 2021

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@orsenthil orsenthil self-assigned this Mar 5, 2021
@orsenthil

orsenthil commented Mar 5, 2021

Copy link
Copy Markdown
Member

Hi Michael, Thanks for following up. I will be able to review and bring it to a closure.
handlerbot reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bgehman
bgehman reviewed Mar 30, 2021
View reviewed changes
Comment thread Lib/http/client.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@GtTmy GtTmy mentioned this pull request Apr 19, 2021
Uses HTTP/1.1 and HOST header for CONNECT method websocket-client/websocket-client#677
Merged
@yilinjuang

yilinjuang commented Nov 7, 2021

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@handlerbot
Switch to for/else: syntax, as suggested
aa1508e
@jalopezsilva jalopezsilva mentioned this pull request Oct 11, 2022
Assess impact of bpo-22708 in our library and test suite. urllib3/urllib3#2750
Open
handlerbot added 2 commits October 24, 2022 18:09
@handlerbot
Don't use for: else:
ef8b896
@handlerbot
Merge branch 'main' into fix-bpo-22708
5d2f166
nametkin
nametkin reviewed Oct 25, 2022
View reviewed changes
Comment thread Lib/http/client.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
handlerbot added 4 commits October 25, 2022 17:03
@handlerbot
Sure, fine, w/e
87e514c
@handlerbot
Oops
d224052
@handlerbot
1nm to the left
70ba474
@handlerbot
Merge branch 'main' into fix-bpo-22708
0a5db5f
@vo-va vo-va mannequin mentioned this pull request Dec 26, 2022
httplib/http.client in method _tunnel used HTTP/1.0 CONNECT method #66897
Closed
@lukaszgalezewski

lukaszgalezewski commented Jan 12, 2023

Copy link
Copy Markdown

why this PR is still open and not merged? and when can be merged? :)
bgehman and nametkin reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@merwok
merwok requested a review from orsenthil January 12, 2023 15:25
@handlerbot
Merge branch 'main' into fix-bpo-22708
d3b5726
@orsenthil

orsenthil commented Jan 13, 2023

Copy link
Copy Markdown
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
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@l8huang

l8huang commented Mar 15, 2023

Copy link
Copy Markdown

@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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@roman-vynar

roman-vynar commented Mar 22, 2023

Copy link
Copy Markdown

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 emoji 1sixth, Oneiroi, and rhuddleston reacted with laugh emoji
All reactions
  • 👍 4 reactions
  • 😄 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@arhadthedev
Merge branch 'main' into fix-bpo-22708
92c4f20
@arhadthedev arhadthedev changed the title bpo-22708: Upgrade HTTP CONNECT to protocol HTTP/1.1 gh-66897: Upgrade HTTP CONNECT to protocol HTTP/1.1 Mar 22, 2023
@arhadthedev arhadthedev added the stdlib Standard Library Python modules in the Lib/ directory label Mar 22, 2023
@arhadthedev

arhadthedev commented Mar 22, 2023

Copy link
Copy Markdown
Member

@vstinner @gpshead could you review and possible merge this PR please? (as committers into Lib/http/client.py)
roman-vynar, rhuddleston, and wwonigkeit reacted with heart emoji
All reactions
  • ❤️ 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
orsenthil
orsenthil approved these changes Apr 5, 2023
View reviewed changes

@orsenthil orsenthil left a comment

Copy link
Copy Markdown
Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
roman-vynar, 1sixth, bgehman, kkitayam, GtTmy, hugovk, and pwlae-transferwise reacted with hooray emoji
All reactions
  • 🎉 7 reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Apr 5, 2023
@orsenthil
orsenthil merged commit 1a8f862 into python:main Apr 5, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 5, 2023
gaogaotiantian pushed a commit to gaogaotiantian/cpython that referenced this pull request Apr 8, 2023
@handlerbot @merwok
@bgehman @arhadthedev @gaogaotiantian
pythongh-66897: Upgrade HTTP CONNECT to protocol HTTP/1.1 (python#8305)
4867e9d 
* 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 <[email protected]>

* 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 <[email protected]>
Co-authored-by: bgehman <[email protected]>
Co-authored-by: Oleg Iarygin <[email protected]>
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@handlerbot @merwok
@bgehman @arhadthedev @warsaw
pythongh-66897: Upgrade HTTP CONNECT to protocol HTTP/1.1 (python#8305)
3eb10b2 
* 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 <[email protected]>

* 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 <[email protected]>
Co-authored-by: bgehman <[email protected]>
Co-authored-by: Oleg Iarygin <[email protected]>
@nametkin nametkin mentioned this pull request Apr 23, 2023
gh-69152: Add _proxy_response_headers attribute to HTTPConnection #26152
Merged
@hugovk

hugovk commented May 20, 2023

Copy link
Copy Markdown
Member

@orsenthil Shall we also backport this to 3.11?
chicocvenancio reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gpshead

gpshead commented May 20, 2023

Copy link
Copy Markdown
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 emoji chicocvenancio reacted with thumbs down emoji
All reactions
  • 👍 3 reactions
  • 👎 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merwok merwok merwok left review comments
@ammaraskar ammaraskar ammaraskar left review comments
@orsenthil orsenthil orsenthil approved these changes
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag
+2 more reviewers
@bgehman bgehman bgehman left review comments
@nametkin nametkin nametkin left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@orsenthil orsenthil

Labels

stdlib Standard Library Python modules in the Lib/ directory

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

20 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
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.