Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Closed
vo-va mannequin opened this issue Oct 23, 2014 · 8 comments
Closed

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

vo-va mannequin opened this issue Oct 23, 2014 · 8 comments
Assignees
@orsenthil
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error type-feature A feature request or enhancement

Comments

@vo-va
Copy link
Mannequin

vo-va mannequin commented Oct 23, 2014

BPO 22708
Nosy @orsenthil, @bitdancer, @berkerpeksag, @vadmium, @demianbrecht, @vo-va
PRs
  • bpo-22708: Fix http protocol version in CONNECT method, support IDN #742
  • gh-66897: Upgrade HTTP CONNECT to protocol HTTP/1.1 #8305
    		•
    Files
  • py2.7.httplib.patch: pach for python 2.7
  • issue22708.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/orsenthil'
closed_at = None
created_at = <Date 2014-10-23.06:21:23.450>
labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
title = 'httplib/http.client in method _tunnel used HTTP/1.0 CONNECT method'
updated_at = <Date 2021-11-07.18:37:03.856>
user = 'https://github.com/vo-va'

    bugs.python.org fields:

    activity = <Date 2021-11-07.18:37:03.856>
actor = 'eric.araujo'
assignee = 'orsenthil'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2014-10-23.06:21:23.450>
creator = 'vova'
dependencies = []
files = ['36996', '38808']
hgrepos = []
issue_num = 22708
keywords = ['patch']
message_count = 6.0
messages = ['229856', '229876', '229915', '239921', '239952', '241237']
nosy_count = 6.0
nosy_names = ['orsenthil', 'r.david.murray', 'berker.peksag', 'martin.panter', 'demian.brecht', 'vova']
pr_nums = ['742', '8305']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22708'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']
    @vo-va
    Copy link
    Mannequin Author

    vo-va mannequin commented Oct 23, 2014

    At my workplace I have to use corporate Internet proxy server with AD/domain/ntlm authorization. I use local cntlm proxy server to authorize myself on corporate proxy. Programs are send requests to cntlm proxy without any authorization information. Cntlm proxy communicate with corporate proxy and handle all authorization stuff and return response to programs.

    But programs which use httplib, like pip, and want to open https url can't work in my network scheme. Because to open https connection httplib send to cntlm proxy

    "CONNECT encrypted.google.com:443 HTTP/1.0\r\n"

    HTTP/1.0 does not assume persistent connection so corporate proxy return http response 407 (need authorization) and close connection. Cntlm proxy detect closed connection and return http response 407 to pip/httplib which can't handle this response or begin ntlm negotiation, throw exception

    ProxyError('Cannot connect to proxy.', error('Tunnel connection failed: 407 Proxy Authentication Required',))

    and close.

    So I suggest change HTTP CONNECT method to

    "CONNECT %s:%d HTTP/1.1\r\n"

    This change allow cntlm proxy keep alive connection to corporate proxy do all authorization stuff and return proper response.

    And also in header of httplib is stated what it is "HTTP/1.1 client library"

    @vo-va vo-va mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Oct 23, 2014
    @bitdancer
    Copy link
    Member

    See bpo-21224 and bpo-9740. It's not 100% clear to me from reading those issues, but it *sounds* like the support is there, you just have to turn it on. So I'm closing this issue as not a bug...either it already works (and you just have to set your code to use 1.1) or this is a duplicate of one of those two issues, and you should comment on whichever one is more appropriate.

    Your point about PIP is a good one, though...that might be worth a specific issue for 1.1 support in pip, if it doesn't already have a way to switch it on.

    @vo-va
    Copy link
    Mannequin Author

    vo-va mannequin commented Oct 24, 2014

    The issue http://bugs.python.org/issue21224 is about http server implementations. The issue http://bugs.python.org/issue9740 is more relevant for what I talking about, but not exactly.

    Look, in this line

    https://hg.python.org/cpython/file/6a2f74811240/Lib/http/client.py#l786

    http protocol version is setted and in this line

    https://hg.python.org/cpython/file/6a2f74811240/Lib/http/client.py#l1036

    variable used to send http method (GET, PUT etc) and it work for direct connection or proxy with http connections. But if required to use CONNECT method through proxy (usually used for https connection) will be used _tunnel() method from http.client (py3k) or httplib (py2.7)

    https://hg.python.org/cpython/file/6a2f74811240/Lib/http/client.py#l871

    and here version off http is hardcoded to HTTP/1.0

    PIP use urllib3, but urllib3 for actual network working (work with socket and send request) use httplib or http.client. So I think it would be better to make changes in httplib than override _tunnel() method in urllib3.

    P.S. I'm not sure about rules how to open/close issues, so I open this issue again. I'm sorry if this causes some inconvenience.

    @vo-va vo-va mannequin reopened this Oct 24, 2014
    @vo-va vo-va mannequin removed the invalid label Oct 24, 2014
    @demianbrecht
    Copy link
    Mannequin

    demianbrecht mannequin commented Apr 2, 2015

    Thanks for the report and follow up Vova. I've come across this line and it puzzled me as well as to why it's hardcoded. Rather than the hardcoded approach in your patch, I've attached a patch that uses _http_vsn_str which is consistent with other areas of the library.

    Additionally, I noticed that a failure will occur if the destination address contains characters outside of ASCII range, so I've added support for IDN.

    I assume that the patch should be committed to tip and maintenance branches as both issues are bugs (there is IDN support elsewhere in the library).

    @vadmium
    Copy link
    Member

    vadmium commented Apr 3, 2015

    Patch looks good to me. I’m not an expert on non-ASCII domain names, but enabling HTTP 1.1 should be okay, at least for new versions of Python. CONNECT for HTTP 1.1 is described at <https://tools.ietf.org/html/rfc7231#section-4.3.6\>.

    Also, see the code review about removing an comment about HTTP 1.1 blessing.

    @vo-va
    Copy link
    Mannequin Author

    vo-va mannequin commented Apr 16, 2015

    I don't know why I hardcoded version of HTTP, it was almost 6 months ago. Maybe because it just POC, or maybe because HTTP 1.0 is not allowed CONNECT method. Thank you for your patch and also for IDN support.

    @berkerpeksag berkerpeksag added the 3.7 (EOL) end of life label Apr 15, 2017
    @orsenthil orsenthil self-assigned this Mar 5, 2021
    @merwok merwok added 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes and removed 3.7 (EOL) end of life labels Nov 7, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @Qubits01
    Copy link

    Sad to see the patch is not implemented 8 years after the issue was opened.

    @bedevere-bot bedevere-bot mentioned this issue Mar 22, 2023
    Merged
    orsenthil pushed a commit that referenced this issue Apr 5, 2023
    @handlerbot @merwok
    @bgehman @arhadthedev
    gh-66897: Upgrade HTTP CONNECT to protocol HTTP/1.1 (#8305)
    1a8f862 
    * 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>
    @arhadthedev
    Copy link
    Member

    gh-8305 is merged so closing as done.

    gaogaotiantian pushed a commit to gaogaotiantian/cpython that referenced this issue 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 <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 issue 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 <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>
    @gpshead gpshead added the type-feature A feature request or enhancement label May 20, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @orsenthil orsenthil

    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    8 participants
    @berkerpeksag @gpshead @orsenthil @bitdancer @merwok @vadmium @arhadthedev @Qubits01