diff --git a/CHANGES.rst b/CHANGES.rst index a1af3fbcb4c..070e82dac98 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -28,9 +28,9 @@ CHANGES - Call worker_int and worker_abort callbacks in `Gunicorn[UVLoop]WebWorker` #1202 -- +- Has functional tests for client proxy #1218 -- +- Fix bugs with client proxy target path and proxy host with port #1413 - diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index a107e3387b1..3f33d8253f2 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -118,6 +118,7 @@ Sebastian Hüther SeongSoo Cho Sergey Ninua Sergey Skripnick +Serhii Kostel Simon Kennedy Sin-Woo Bang Stanislas Plum diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index b226c011df8..c1901b1f29d 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -432,9 +432,19 @@ def write_bytes(self, request, reader): self._writer = None def send(self, writer, reader): - path = self.url.raw_path - if self.url.raw_query_string: - path += '?' + self.url.raw_query_string + # Specify request target: + # - CONNECT request must send authority form URI + # - not CONNECT proxy must send absolute form URI + # - most common is origin form URI + if self.method == hdrs.METH_CONNECT: + path = '{}:{}'.format(self.url.host, self.url.port) + elif self.proxy and not self.ssl: + path = str(self.url) + else: + path = self.url.raw_path + if self.url.raw_query_string: + path += '?' + self.url.raw_query_string + request = aiohttp.Request(writer, self.method, path, self.version) diff --git a/aiohttp/connector.py b/aiohttp/connector.py index f24354f2969..ab32a54d05d 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -634,7 +634,7 @@ def _create_direct_connection(self, req): def _create_proxy_connection(self, req): proxy_req = ClientRequest( hdrs.METH_GET, req.proxy, - headers={hdrs.HOST: req.host}, + headers={hdrs.HOST: req.headers[hdrs.HOST]}, auth=req.proxy_auth, loop=self._loop) try: @@ -644,8 +644,6 @@ def _create_proxy_connection(self, req): except OSError as exc: raise ProxyConnectionError(*exc.args) from exc - if not req.ssl: - req.path = str(req.url) if hdrs.AUTHORIZATION in proxy_req.headers: auth = proxy_req.headers[hdrs.AUTHORIZATION] del proxy_req.headers[hdrs.AUTHORIZATION] @@ -665,7 +663,7 @@ def _create_proxy_connection(self, req): # to do this we must wrap raw socket into secure one # asyncio handles this perfectly proxy_req.method = hdrs.METH_CONNECT - proxy_req.path = '{}:{}'.format(req.host, req.port) + proxy_req.url = req.url key = (req.host, req.port, req.ssl) conn = Connection(self, key, proxy_req, transport, proto, self._loop) diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 7b27d42a368..5d13125328b 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -40,7 +40,7 @@ def test_connect(self, ClientRequestMock): tr, proto = mock.Mock(), mock.Mock() self.loop.create_connection = make_mocked_coro((tr, proto)) conn = self.loop.run_until_complete(connector.connect(req)) - self.assertEqual(req.path, 'http://www.python.org') + self.assertEqual(req.url, URL('http://www.python.org')) self.assertIs(conn._transport, tr) self.assertIs(conn._protocol, proto) @@ -105,7 +105,7 @@ def test_auth(self, ClientRequestMock): self.assertNotIn('PROXY-AUTHORIZATION', req.headers) conn = self.loop.run_until_complete(connector.connect(req)) - self.assertEqual(req.path, 'http://www.python.org') + self.assertEqual(req.url, URL('http://www.python.org')) self.assertNotIn('AUTHORIZATION', req.headers) self.assertIn('PROXY-AUTHORIZATION', req.headers) self.assertNotIn('AUTHORIZATION', proxy_req.headers) @@ -148,7 +148,7 @@ def test_auth_from_url(self, ClientRequestMock): self.assertNotIn('PROXY-AUTHORIZATION', req.headers) conn = self.loop.run_until_complete(connector.connect(req)) - self.assertEqual(req.path, 'http://www.python.org') + self.assertEqual(req.url, URL('http://www.python.org')) self.assertNotIn('AUTHORIZATION', req.headers) self.assertIn('PROXY-AUTHORIZATION', req.headers) self.assertNotIn('AUTHORIZATION', proxy_req.headers) @@ -212,7 +212,7 @@ def test_https_connect(self, ClientRequestMock): self.assertEqual(req.url.path, '/') self.assertEqual(proxy_req.method, 'CONNECT') - self.assertEqual(proxy_req.path, 'www.python.org:443') + self.assertEqual(proxy_req.url, URL('https://www.python.org')) tr.pause_reading.assert_called_once_with() tr.get_extra_info.assert_called_with('socket', default=None) @@ -341,7 +341,7 @@ def test_request_port(self, ClientRequestMock): loop=self.loop, ) self.loop.run_until_complete(connector._create_connection(req)) - self.assertEqual(req.path, 'http://localhost:1234/path') + self.assertEqual(req.url, URL('http://localhost:1234/path')) def test_proxy_auth_property(self): req = aiohttp.ClientRequest( @@ -393,7 +393,7 @@ def test_https_connect_pass_ssl_context(self, ClientRequestMock): self.assertEqual(req.url.path, '/') self.assertEqual(proxy_req.method, 'CONNECT') - self.assertEqual(proxy_req.path, 'www.python.org:443') + self.assertEqual(proxy_req.url, URL('https://www.python.org')) tr.pause_reading.assert_called_once_with() tr.get_extra_info.assert_called_with('socket', default=None) @@ -494,7 +494,7 @@ def test_connect(self, ClientRequestMock): tr, proto = mock.Mock(), mock.Mock() self.loop.create_connection = make_mocked_coro((tr, proto)) conn = self.loop.run_until_complete(connector.connect(req)) - self.assertEqual(req.path, 'http://www.python.org') + self.assertEqual(req.url, URL('http://www.python.org')) self.assertIs(conn._transport, tr) self.assertIs(conn._protocol, proto) diff --git a/tests/test_proxy_functional.py b/tests/test_proxy_functional.py new file mode 100644 index 00000000000..72116361601 --- /dev/null +++ b/tests/test_proxy_functional.py @@ -0,0 +1,279 @@ +import asyncio +from functools import partial +from unittest import mock + +import pytest +from yarl import URL + +import aiohttp +import aiohttp.helpers +import aiohttp.web + + +@asyncio.coroutine +def test_proxy_http_absolute_path(proxy_test_server, get_request): + url = 'http://aiohttp.io/path?query=yes' + proxy = yield from proxy_test_server() + + yield from get_request(url=url, proxy=proxy.url) + + assert len(proxy.requests_list) == 1 + assert proxy.request.method == 'GET' + assert proxy.request.host == 'aiohttp.io' + assert proxy.request.path_qs == 'http://aiohttp.io/path?query=yes' + + +@asyncio.coroutine +def test_proxy_http_raw_path(proxy_test_server, get_request): + url = 'http://aiohttp.io:2561/space sheep?q=can:fly' + raw_url = 'http://aiohttp.io:2561/space%20sheep?q=can%3Afly' + proxy = yield from proxy_test_server() + + yield from get_request(url=url, proxy=proxy.url) + + assert proxy.request.host == 'aiohttp.io:2561' + assert proxy.request.path_qs == raw_url + + +@asyncio.coroutine +def test_proxy_http_connection_error(get_request): + url = 'http://aiohttp.io/path' + proxy_url = 'http://localhost:2242/' + + with pytest.raises(aiohttp.ProxyConnectionError): + yield from get_request(url=url, proxy=proxy_url) + + +@asyncio.coroutine +def test_proxy_http_bad_response(proxy_test_server, get_request): + url = 'http://aiohttp.io/path' + proxy = yield from proxy_test_server() + proxy.return_value = dict( + status=502, + headers={'Proxy-Agent': 'TestProxy'}) + + resp = yield from get_request(url=url, proxy=proxy.url) + + assert resp.status == 502 + assert resp.headers['Proxy-Agent'] == 'TestProxy' + + +@asyncio.coroutine +def test_proxy_http_auth(proxy_test_server, get_request): + url = 'http://aiohttp.io/path' + proxy = yield from proxy_test_server() + + yield from get_request(url=url, proxy=proxy.url) + + assert 'Authorization' not in proxy.request.headers + assert 'Proxy-Authorization' not in proxy.request.headers + + auth = aiohttp.helpers.BasicAuth('user', 'pass') + yield from get_request(url=url, auth=auth, proxy=proxy.url) + + assert 'Authorization' in proxy.request.headers + assert 'Proxy-Authorization' not in proxy.request.headers + + yield from get_request(url=url, proxy_auth=auth, proxy=proxy.url) + + assert 'Authorization' not in proxy.request.headers + assert 'Proxy-Authorization' in proxy.request.headers + + yield from get_request(url=url, auth=auth, + proxy_auth=auth, proxy=proxy.url) + + assert 'Authorization' in proxy.request.headers + assert 'Proxy-Authorization' in proxy.request.headers + + +@asyncio.coroutine +def test_proxy_http_auth_utf8(proxy_test_server, get_request): + url = 'http://aiohttp.io/path' + auth = aiohttp.helpers.BasicAuth('юзер', 'пасс', 'utf-8') + proxy = yield from proxy_test_server() + + yield from get_request(url=url, auth=auth, proxy=proxy.url) + + assert 'Authorization' in proxy.request.headers + assert 'Proxy-Authorization' not in proxy.request.headers + + +@asyncio.coroutine +def test_proxy_http_auth_from_url(proxy_test_server, get_request): + url = 'http://aiohttp.io/path' + proxy = yield from proxy_test_server() + + auth_url = URL(url).with_user('user').with_password('pass') + yield from get_request(url=auth_url, proxy=proxy.url) + + assert 'Authorization' in proxy.request.headers + assert 'Proxy-Authorization' not in proxy.request.headers + + proxy_url = URL(proxy.url).with_user('user').with_password('pass') + yield from get_request(url=url, proxy=proxy_url) + + assert 'Authorization' not in proxy.request.headers + assert 'Proxy-Authorization' in proxy.request.headers + + +@asyncio.coroutine +def test_proxy_https_connect(proxy_test_server, get_request): + proxy = yield from proxy_test_server() + url = 'https://www.google.com.ua/search?q=aiohttp proxy' + + yield from get_request(url=url, proxy=proxy.url) + + connect = proxy.requests_list[0] + assert connect.method == 'CONNECT' + assert connect.path == 'www.google.com.ua:443' + assert connect.host == 'www.google.com.ua' + + assert proxy.request.host == 'www.google.com.ua' + assert proxy.request.path_qs == '/search?q=aiohttp+proxy' + + +@asyncio.coroutine +def test_proxy_https_connect_with_port(proxy_test_server, get_request): + proxy = yield from proxy_test_server() + url = 'https://secure.aiohttp.io:2242/path' + + yield from get_request(url=url, proxy=proxy.url) + + connect = proxy.requests_list[0] + assert connect.method == 'CONNECT' + assert connect.path == 'secure.aiohttp.io:2242' + assert connect.host == 'secure.aiohttp.io:2242' + + assert proxy.request.host == 'secure.aiohttp.io:2242' + assert proxy.request.path_qs == '/path' + + +@asyncio.coroutine +def test_proxy_https_connection_error(get_request): + url = 'https://secure.aiohttp.io/path' + proxy_url = 'http://localhost:2242/' + + with pytest.raises(aiohttp.ProxyConnectionError): + yield from get_request(url=url, proxy=proxy_url) + + +@asyncio.coroutine +def test_proxy_https_bad_response(proxy_test_server, get_request): + url = 'https://secure.aiohttp.io/path' + proxy = yield from proxy_test_server() + proxy.return_value = dict( + status=502, + headers={'Proxy-Agent': 'TestProxy'}) + + with pytest.raises(aiohttp.HttpProxyError): + yield from get_request(url=url, proxy=proxy.url) + + assert len(proxy.requests_list) == 1 + assert proxy.request.method == 'CONNECT' + assert proxy.request.path == 'secure.aiohttp.io:443' + + +@asyncio.coroutine +def test_proxy_https_auth(proxy_test_server, get_request): + url = 'https://secure.aiohttp.io/path' + auth = aiohttp.helpers.BasicAuth('user', 'pass') + + proxy = yield from proxy_test_server() + yield from get_request(url=url, proxy=proxy.url) + + connect = proxy.requests_list[0] + assert 'Authorization' not in connect.headers + assert 'Proxy-Authorization' not in connect.headers + assert 'Authorization' not in proxy.request.headers + assert 'Proxy-Authorization' not in proxy.request.headers + + proxy = yield from proxy_test_server() + yield from get_request(url=url, auth=auth, proxy=proxy.url) + + connect = proxy.requests_list[0] + assert 'Authorization' not in connect.headers + assert 'Proxy-Authorization' not in connect.headers + assert 'Authorization' in proxy.request.headers + assert 'Proxy-Authorization' not in proxy.request.headers + + proxy = yield from proxy_test_server() + yield from get_request(url=url, proxy_auth=auth, proxy=proxy.url) + + connect = proxy.requests_list[0] + assert 'Authorization' not in connect.headers + assert 'Proxy-Authorization' in connect.headers + assert 'Authorization' not in proxy.request.headers + assert 'Proxy-Authorization' not in proxy.request.headers + + proxy = yield from proxy_test_server() + yield from get_request(url=url, auth=auth, + proxy_auth=auth, proxy=proxy.url) + + connect = proxy.requests_list[0] + assert 'Authorization' not in connect.headers + assert 'Proxy-Authorization' in connect.headers + assert 'Authorization' in proxy.request.headers + assert 'Proxy-Authorization' not in proxy.request.headers + + +def _patch_ssl_transport(monkeypatch): + """Make ssl transport substitution to prevent ssl handshake.""" + def _make_ssl_transport_dummy(self, rawsock, protocol, sslcontext, + waiter=None, **kwargs): + return self._make_socket_transport(rawsock, protocol, waiter) + + monkeypatch.setattr( + "asyncio.selector_events.BaseSelectorEventLoop._make_ssl_transport", + _make_ssl_transport_dummy) + + +@pytest.fixture +def proxy_test_server(raw_test_server, loop, monkeypatch): + """Handle all proxy requests and imitate remote server response.""" + + _patch_ssl_transport(monkeypatch) + + default_response = dict( + status=200, + headers=None, + body=None) + + @asyncio.coroutine + def proxy_handler(request, proxy_mock): + proxy_mock.request = request + proxy_mock.requests_list.append(request) + + response = default_response.copy() + if isinstance(proxy_mock.return_value, dict): + response.update(proxy_mock.return_value) + + return aiohttp.web.Response(**response) + + @asyncio.coroutine + def proxy_server(): + proxy_mock = mock.Mock() + proxy_mock.request = None + proxy_mock.requests_list = [] + + handler = partial(proxy_handler, proxy_mock=proxy_mock) + server = yield from raw_test_server(handler, loop=loop) + + proxy_mock.server = server + proxy_mock.url = server.make_url('/') + + return proxy_mock + + return proxy_server + + +@asyncio.coroutine +def _request(method, url, loop=None, **kwargs): + with aiohttp.ClientSession(loop=loop) as client: + resp = yield from client.request(method, url, **kwargs) + yield from resp.release() + return resp + + +@pytest.fixture() +def get_request(loop): + return partial(_request, method='GET', loop=loop)