-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Copy of #2582, Added proxy parameters for ClientSession #3405
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 25 commits
8055b96
d3416a3
f23b9ee
19ccf12
5821bc0
5c37878
66c4ce7
8e79f99
8c249e1
3defdef
dc2115c
1f11214
7370f8a
0233316
e592dd6
578de43
9c75827
bcd94cf
36c2b1b
f1fba4d
1936e55
18d9dde
90333ca
4ee84d7
508541e
f266e15
d767587
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Added proxy parameters for ClientSession. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,6 +91,7 @@ Eugene Chernyshov | |
| Eugene Naydenov | ||
| Eugene Tolmachev | ||
| Evert Lammerts | ||
| Fan Yu | ||
| FichteFoll | ||
| Frederik Gladhorn | ||
| Frederik Peter Aalund | ||
|
|
@@ -192,6 +193,7 @@ Pepe Osca | |
| Philipp A. | ||
| Pieter van Beek | ||
| Rafael Viotti | ||
| Ratan Kulshreshtha | ||
| Raúl Cumplido | ||
| Required Field | ||
| Robert Lu | ||
|
|
@@ -262,4 +264,4 @@ Yuriy Shatrov | |
| Yury Selivanov | ||
| Yusuke Tsutsumi | ||
| Марк Коренберг | ||
| Семён Марьясин | ||
| Семён Марьясин | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is unrelated. Fix your editor to preserve original EOF.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -172,7 +172,8 @@ class ClientSession: | |
| '_timeout', '_raise_for_status', '_auto_decompress', | ||
| '_trust_env', '_default_headers', '_skip_auto_headers', | ||
| '_request_class', '_response_class', | ||
| '_ws_response_class', '_trace_configs') | ||
| '_ws_response_class', '_trace_configs', '_session_proxy', | ||
| '_session_proxy_auth', '_session_proxy_headers') | ||
|
|
||
| def __init__(self, *, connector: Optional[BaseConnector]=None, | ||
| loop: Optional[asyncio.AbstractEventLoop]=None, | ||
|
|
@@ -194,7 +195,10 @@ def __init__(self, *, connector: Optional[BaseConnector]=None, | |
| auto_decompress: bool=True, | ||
| trust_env: bool=False, | ||
| requote_redirect_url: bool=True, | ||
| trace_configs: Optional[List[TraceConfig]]=None) -> None: | ||
| trace_configs: Optional[List[TraceConfig]]=None, | ||
| proxy: Optional[StrOrURL]=None, | ||
| proxy_auth: Optional[BasicAuth]=None, | ||
| proxy_headers: Optional[LooseHeaders]=None) -> None: | ||
|
|
||
| if loop is None: | ||
| if connector is not None: | ||
|
|
@@ -259,6 +263,10 @@ def __init__(self, *, connector: Optional[BaseConnector]=None, | |
| self._trust_env = trust_env | ||
| self._requote_redirect_url = requote_redirect_url | ||
|
|
||
| self._session_proxy = proxy | ||
| self._session_proxy_auth = proxy_auth | ||
| self._session_proxy_headers = proxy_headers | ||
|
|
||
| # Convert to list of tuples | ||
| if headers: | ||
| headers = CIMultiDict(headers) | ||
|
|
@@ -372,12 +380,22 @@ async def _request( | |
| for i in skip_auto_headers: | ||
| skip_headers.add(istr(i)) | ||
|
|
||
| if proxy is not None: | ||
| if proxy is not None and self._session_proxy is None: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the behavior if |
||
| try: | ||
| proxy = URL(proxy) | ||
| except ValueError: | ||
| raise InvalidURL(proxy) | ||
|
|
||
| if proxy is None and self._session_proxy is not None: | ||
| try: | ||
| proxy = self._session_proxy | ||
| proxy_auth = self._session_proxy_auth | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By this line you are overriding explicitly passed I have no idea how to resolve the conflict properly but proposed behavior looks confusing.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am trying to use session proxy if proxy is not mentioned explicitly for a particular request. |
||
| proxy_headers = self._prepare_headers( | ||
| self._session_proxy_headers | ||
| ) | ||
| except ValueError: | ||
| raise InvalidURL(proxy) | ||
|
|
||
| if timeout is sentinel: | ||
| real_timeout = self._timeout # type: ClientTimeout | ||
| else: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| from unittest import mock | ||
|
|
||
| import pytest | ||
|
|
||
| import aiohttp | ||
| from aiohttp import web | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def proxy_test_server(aiohttp_raw_server, loop, monkeypatch): | ||
| """Handle all proxy requests and imitate remote server response.""" | ||
|
|
||
| _patch_ssl_transport(monkeypatch) | ||
|
|
||
| default_response = {'status': 200, 'headers': None, 'body': None} | ||
|
|
||
| proxy_mock = mock.Mock() | ||
|
|
||
| async def proxy_handler(request): | ||
| 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) | ||
|
|
||
| headers = response['headers'] | ||
| if not headers: | ||
| headers = {} | ||
|
|
||
| if request.method == 'CONNECT': | ||
| response['body'] = None | ||
|
|
||
| response['headers'] = headers | ||
|
|
||
| resp = web.Response(**response) | ||
| await resp.prepare(request) | ||
| await resp.write_eof() | ||
| return resp | ||
|
|
||
| async def proxy_server(): | ||
| proxy_mock.request = None | ||
| proxy_mock.auth = None | ||
| proxy_mock.requests_list = [] | ||
|
|
||
| server = await aiohttp_raw_server(proxy_handler) | ||
|
|
||
| proxy_mock.server = server | ||
| proxy_mock.url = server.make_url('/') | ||
|
|
||
| return proxy_mock | ||
|
|
||
| return proxy_server | ||
|
|
||
|
|
||
| @pytest.fixture() | ||
| def get_request(loop): | ||
| async def _request(method='GET', *, url, trust_env=False, **kwargs): | ||
| connector = aiohttp.TCPConnector(verify_ssl=False, loop=loop) | ||
| client = aiohttp.ClientSession(connector=connector, | ||
| trust_env=trust_env) | ||
| try: | ||
| resp = await client.request(method, url, **kwargs) | ||
| await resp.release() | ||
| return resp | ||
| finally: | ||
| await client.close() | ||
| return _request | ||
|
|
||
|
|
||
| def _patch_ssl_transport(monkeypatch) -> None: | ||
| """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, | ||
| extra=kwargs.get('extra'), | ||
| server=kwargs.get('server')) | ||
|
|
||
| monkeypatch.setattr( | ||
| "asyncio.selector_events.BaseSelectorEventLoop._make_ssl_transport", | ||
| _make_ssl_transport_dummy) | ||
|
|
||
|
|
||
| async def test_session_proxy_http(proxy_test_server, loop) -> None: | ||
| url = 'http://aiohttp.io/path' | ||
| proxy = await proxy_test_server() | ||
| proxy.return_value = dict(body=b'test') | ||
|
|
||
| conn = aiohttp.TCPConnector(loop=loop) | ||
| sess = aiohttp.ClientSession(connector=conn, loop=loop, proxy=proxy.url) | ||
|
|
||
| resp = await sess.get(url) | ||
| assert (await resp.read()) == b'test' | ||
|
|
||
|
|
||
| async def test_session_proxy_http_auth(proxy_test_server, loop) -> None: | ||
| url = 'http://aiohttp.io/path' | ||
| proxy = await proxy_test_server() | ||
|
|
||
| conn = aiohttp.TCPConnector(loop=loop) | ||
| sess = aiohttp.ClientSession(connector=conn, loop=loop, proxy=proxy.url) | ||
| await sess.get(url) | ||
| assert 'Authorization' not in proxy.request.headers | ||
| assert 'Proxy-Authorization' not in proxy.request.headers | ||
|
|
||
| conn = aiohttp.TCPConnector(loop=loop) | ||
| auth = aiohttp.BasicAuth('user', 'pass') | ||
| sess = aiohttp.ClientSession(connector=conn, loop=loop, | ||
| proxy_auth=auth, proxy=proxy.url) | ||
| await sess.get(url) | ||
| assert 'Authorization' not in proxy.request.headers | ||
| assert 'Proxy-Authorization' in proxy.request.headers | ||
|
|
||
| conn = aiohttp.TCPConnector(loop=loop) | ||
| auth = aiohttp.BasicAuth('user', 'pass') | ||
| sess = aiohttp.ClientSession(connector=conn, loop=loop, | ||
| auth=auth, proxy_auth=auth, proxy=proxy.url) | ||
| await sess.get(url) | ||
| assert 'Authorization' in proxy.request.headers | ||
| assert 'Proxy-Authorization' in proxy.request.headers |
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.
Plz clean-up all unrelated changes.
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.
Should I squash all the changes ?
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.
Up to you. We'll use "Merge and squash" strategy on this PR anyway :)