Fix chunked encoding with SSL via proxy by jamespic · Pull Request #4179 · psf/requests

jamespic · 2017-06-27T15:27:33Z

This is a patch applying @bpitman's proposed fix for #3844.

Assuming a proxy is running on port 8875 (docker run --rm --name='tinyproxy' -p 8875:8888 dannydirect/tinyproxy:latest ANY works for me), the issue can be reproduced with

requests.put('https://www.google.com', data=iter('Hello World'), proxies={'https': 'http://localhost:8875'})

Lukasa · 2017-06-27T15:37:15Z

Why aren't we taking the approach I suggested in #3844?

jamespic · 2017-06-27T16:04:16Z

@Lukasa I don't know the codebase that well, so I couldn't figure out exactly how the approach you'd proposed would fit in with the existing code - I tried getting rid of the chunked branch and just adding a chunked parameter to the conn.urlopen, but it lead to a failed test that I couldn't make sense of. Whereas @bpitman's patch worked and passed the tests.

Lukasa · 2017-06-27T16:12:07Z

What test failed? Can you provide the test output?

jamespic · 2017-06-27T16:14:01Z

Sure thing

_________________________________________________________________________________ TestTimeout.test_total_timeout_connect[timeout0] _________________________________________________________________________________
[gw0] linux -- Python 3.5.2 /home/james/workspace/requests/.tox/py35/bin/python

self = <tests.test_requests.TestTimeout object at 0x7f42473c8828>, timeout = (0.1, 0.1)

    @pytest.mark.parametrize(
        'timeout', (
            (0.1, 0.1),
            Urllib3Timeout(connect=0.1, read=0.1)
        ))
    def test_total_timeout_connect(self, timeout):
        try:
>           requests.get(TARPIT, timeout=timeout)

tests/test_requests.py:2077: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
requests/api.py:72: in get
    return request('get', url, params=params, **kwargs)
requests/api.py:58: in request
    return session.request(method=method, url=url, **kwargs)
requests/sessions.py:502: in request
    resp = self.send(prep, **send_kwargs)
requests/sessions.py:612: in send
    r = adapter.send(request, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <requests.adapters.HTTPAdapter object at 0x7f4247413b70>, request = <PreparedRequest [GET]>, stream = False, timeout = <urllib3.util.timeout.Timeout object at 0x7f4247413b38>, verify = True, cert = None
proxies = OrderedDict()

    def send(self, request, stream=False, timeout=None, verify=True, cert=None, proxies=None):
        """Sends PreparedRequest object. Returns Response object.
    
            :param request: The :class:`PreparedRequest <PreparedRequest>` being sent.
            :param stream: (optional) Whether to stream the request content.
            :param timeout: (optional) How long to wait for the server to send
                data before giving up, as a float, or a :ref:`(connect timeout,
                read timeout) <timeouts>` tuple.
            :type timeout: float or tuple or urllib3 Timeout object
            :param verify: (optional) Either a boolean, in which case it controls whether
                we verify the server's TLS certificate, or a string, in which case it
                must be a path to a CA bundle to use
            :param cert: (optional) Any user-provided SSL certificate to be trusted.
            :param proxies: (optional) The proxies dictionary to apply to the request.
            :rtype: requests.Response
            """
    
        conn = self.get_connection(request.url, proxies)
    
        self.cert_verify(conn, request.url, verify, cert)
        url = self.request_url(request, proxies)
        self.add_headers(request)
    
        chunked = not (request.body is None or 'Content-Length' in request.headers)
    
        if isinstance(timeout, tuple):
            try:
                connect, read = timeout
                timeout = TimeoutSauce(connect=connect, read=read)
            except ValueError as e:
                # this may raise a string formatting error.
                err = ("Invalid timeout {0}. Pass a (connect, read) "
                       "timeout tuple, or a single float to set "
                       "both timeouts to the same value".format(timeout))
                raise ValueError(err)
        elif isinstance(timeout, TimeoutSauce):
            pass
        else:
            timeout = TimeoutSauce(connect=timeout, read=timeout)
    
        try:
            resp = conn.urlopen(
                method=request.method,
                url=url,
                body=request.body,
                headers=request.headers,
                redirect=False,
                assert_same_host=False,
                preload_content=False,
                decode_content=False,
                retries=self.max_retries,
                timeout=timeout,
                chunked=chunked
            )
    
        except (ProtocolError, socket.error) as err:
            raise ConnectionError(err, request=request)
    
        except MaxRetryError as e:
            if isinstance(e.reason, ConnectTimeoutError):
                # TODO: Remove this in 3.0.0: see #2811
                if not isinstance(e.reason, NewConnectionError):
                    raise ConnectTimeout(e, request=request)
    
            if isinstance(e.reason, ResponseError):
                raise RetryError(e, request=request)
    
            if isinstance(e.reason, _ProxyError):
                raise ProxyError(e, request=request)
    
>           raise ConnectionError(e, request=request)
E           requests.exceptions.ConnectionError: HTTPConnectionPool(host='10.255.255.1', port=80): Max retries exceeded with url: / (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7f42473bb5f8>: Failed to establish a new connection: [Errno 113] No route to host',))

requests/adapters.py:458: ConnectionError
=========================================================================== 1 failed, 481 passed, 12 skipped, 2 xpassed in 33.83 seconds ===========================================================================

Weirdly it only failed in Python 3. The same test passed in Python 2.

Lukasa · 2017-06-27T16:17:19Z

That failure should be unrelated: it seems to be changing from a connection timeout to an authoritative statement that no route exists to that host. Mind pushing the branch up so I can take a look at your change?

jamespic · 2017-06-27T16:22:03Z

Sure thing: https://github.com/jamespic/requests/tree/chunked-parameter

Lukasa · 2017-06-27T16:48:36Z

Ok, so I think this test was accidentally relying on an environment behaviour. Mind pushing it to a branch that gets CI'd to see if our CI reproduces it? If it does I'll dive in myself.

jamespic · 2017-06-27T16:50:09Z

Yep, absolutely.

forrestchang · 2017-07-25T12:15:53Z

I merge your code and test the code, but it's still hanging.

import requests
resp = requests.get('https://baidu.com', proxies={'https': 'localhost:8899'})

I tried curl with this proxy server, it worked.

HoneyBaked · 2018-05-15T15:56:34Z

We are running in this to as well, and the patch resolved our problem/ Is there a plan to get this merged in to master?

stromnet · 2018-09-17T19:36:40Z

Any update on this? I got the same problem with chunked requests (which I think I can avoid, I accidentally used chunked by providing a None value in the data dict).

Here is my analysis (done before I found #3844 and this PR):
Minimal reproduced case:

Python 3.7.0 (Mac OS)
urllib3 1.23
requests 2.19.1

Install tinyproxy (brew install tinyproxy), no configuration required.
Launch with tinyproxy -d

Python code:

import requests
print(requests.post('https://status.github.com',
					data={'a':None},
					proxies={'https': 'http://127.0.0.1:8888'}))

Result:

Traceback (most recent call last):
  File "/Users/johan/reqbug/venv/lib/python3.7/site-packages/requests/adapters.py", line 471, in send
    low_conn.endheaders()
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/http/client.py", line 1224, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/http/client.py", line 1016, in _send_output
    self.send(msg)
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/http/client.py", line 956, in send
    self.connect()
  File "/Users/johan/reqbug/venv/lib/python3.7/site-packages/urllib3/connection.py", line 356, in connect
    ssl_context=context)
  File "/Users/johan/reqbug/venv/lib/python3.7/site-packages/urllib3/util/ssl_.py", line 359, in ssl_wrap_socket
    return context.wrap_socket(sock, server_hostname=server_hostname)
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/ssl.py", line 412, in wrap_socket
    session=session
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/ssl.py", line 850, in _create
    self.do_handshake()
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/ssl.py", line 1108, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLError: [SSL: UNKNOWN_PROTOCOL] unknown protocol (_ssl.c:1045)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "test.py", line 4, in <module>
    proxies={'https': 'http://localhost:8888'}))
  File "/Users/johan/reqbug/venv/lib/python3.7/site-packages/requests/api.py", line 112, in post
    return request('post', url, data=data, json=json, **kwargs)
  File "/Users/johan/reqbug/venv/lib/python3.7/site-packages/requests/api.py", line 58, in request
    return session.request(method=method, url=url, **kwargs)
  File "/Users/johan/reqbug/venv/lib/python3.7/site-packages/requests/sessions.py", line 512, in request
    resp = self.send(prep, **send_kwargs)
  File "/Users/johan/reqbug/venv/lib/python3.7/site-packages/requests/sessions.py", line 622, in send
    r = adapter.send(request, **kwargs)
  File "/Users/johan/reqbug/venv/lib/python3.7/site-packages/requests/adapters.py", line 503, in send
    raise ConnectionError(err, request=request)
requests.exceptions.ConnectionError: [SSL: UNKNOWN_PROTOCOL] unknown protocol (_ssl.c:1045)

The key here is the None value in the data, which makes the HTTPAdapter pick the chunked route:
https://github.com/requests/requests/blame/master/requests/adapters.py#L449

This means relying on urllib3 internal functions.
The above blame indicats this was written 5 years ago.
In urllib3, 4 years ago, the code changed and (re?)introduced the _prepare_proxy call:

https://github.com/urllib3/urllib3/blame/master/src/urllib3/connectionpool.py#L592

By adding the following code just after adapters.py:453 (conn._get_conn(..)), the request works as expected:

                is_new_proxy_conn = conn.proxy is not None and not getattr(low_conn, 'sock', None)
                if is_new_proxy_conn:
                    conn._prepare_proxy(low_conn)

lmvlmv · 2019-07-01T13:33:25Z

Commenting to bump. Am running into this when using chunked uploads viathe POST method using a proxy. Can confirm the above 3 liner is functional. Any chance this PR can get approved and pushed into the full module?

jayvdb · 2019-11-02T13:58:51Z

@jamespic, this needs to be rebased, or closed if it has been resolved by other merged changes, esp #5128 which says it solves this problem.

ynouri · 2020-01-10T22:54:12Z

I was experiencing the same issue (chunked encoding through an HTTPS proxy tunnel resulting in an SSL unknown protocol error) on a test case very similar to the one described by @stromnet above with requests 2.22.0.

I upgraded to a9ee0ee and the issue disappeared 👍 . After spending some time analyzing the issue I'm confident that the fix came from #5128 as suggested by @jayvdb .

Since as of Jan 10, 2020 the fix is not released I used this command to upgrade requests to head of master branch: pip install git+https://github.com/psf/requests.git --upgrade

Fix psf#3844

8fcbeca

HoneyBaked approved these changes May 15, 2018

View reviewed changes

lmvlmv mentioned this pull request Jul 2, 2019

Use urllib for chunked requests #5128

Merged

jamespic closed this Jan 10, 2020

ynouri mentioned this pull request Jan 10, 2020

HTTPS proxy connection not initialized if first request is chunked #3844

Closed

lmvlmv mentioned this pull request Nov 18, 2020

Use urllib for chunked requests (Resubmit) #5664

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2021

Uh oh!

Conversation

jamespic commented Jun 27, 2017

Uh oh!

Lukasa commented Jun 27, 2017

Uh oh!

jamespic commented Jun 27, 2017

Uh oh!

Lukasa commented Jun 27, 2017

Uh oh!

jamespic commented Jun 27, 2017

Uh oh!

Lukasa commented Jun 27, 2017

Uh oh!

jamespic commented Jun 27, 2017

Uh oh!

Lukasa commented Jun 27, 2017

Uh oh!

jamespic commented Jun 27, 2017

Uh oh!

forrestchang commented Jul 25, 2017

Uh oh!

HoneyBaked commented May 15, 2018

Uh oh!

stromnet commented Sep 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lmvlmv commented Jul 1, 2019

Uh oh!

jayvdb commented Nov 2, 2019

Uh oh!

ynouri commented Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

stromnet commented Sep 17, 2018 •

edited

Loading

ynouri commented Jan 10, 2020 •

edited

Loading