From e47676bf2577f42a30b50b4d55761bbbd3a712bf Mon Sep 17 00:00:00 2001 From: Al Date: Tue, 5 Dec 2017 14:42:42 -0500 Subject: [PATCH 01/34] code and unit tests --- aiohttp/client.py | 3 +++ aiohttp/client_reqrep.py | 45 +++++++++++++++++++++++++++++++-- tests/test_client_functional.py | 22 ++++++++++++++++ tests/test_client_request.py | 30 ++++++++++++++++++++++ tests/test_client_session.py | 34 ++++++++++++++++++++++++- 5 files changed, 131 insertions(+), 3 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 724941d228d..d7f825be9dc 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -312,6 +312,9 @@ async def _request(self, method, url, *, self._cookie_jar.update_cookies(resp.cookies, resp.url) + # emit event + resp = await req.dispatch_hooks("response") + # redirects if resp.status in ( 301, 302, 303, 307, 308) and allow_redirects: diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 51876cc6b6c..9e63f4b32ba 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -7,6 +7,7 @@ import sys import traceback import warnings +import inspect from collections import namedtuple from hashlib import md5, sha1, sha256 from http.cookies import CookieError, Morsel, SimpleCookie @@ -56,7 +57,32 @@ ConnectionKey = namedtuple('ConnectionKey', ['host', 'port', 'ssl']) -class ClientRequest: +class HooksMixin: + def register_hook(self, event, hook): + """Register an event hook.""" + if event not in self._hooks: + raise ValueError('Unsupported event specified, with event name {0}'.format(event)) + + assert inspect.iscoroutinefunction(hook), "hook {0} must be a coroutine".format(getattr(hook, '__name__', '')) + + self._hooks[event].append(hook) + + def deregister_hook(self, event, hook): + """Deregister a previously registered event hook. + Returns True if the hook existed, False if not. + """ + try: + self._hooks[event].remove(hook) + return True + except ValueError: + return False + + async def dispatch_hooks(self, event, **extras): + """implement this method to call hooks""" + raise NotImplementedError() + + +class ClientRequest(HooksMixin): GET_METHODS = { hdrs.METH_GET, hdrs.METH_HEAD, @@ -70,6 +96,7 @@ class ClientRequest: hdrs.ACCEPT: '*/*', hdrs.ACCEPT_ENCODING: 'gzip, deflate', } + EVENTS = ['response'] body = b'' auth = None @@ -123,6 +150,7 @@ def __init__(self, method, url, *, self._auto_decompress = auto_decompress self._verify_ssl = verify_ssl self._ssl_context = ssl_context + self._hooks = dict((event, []) for event in self.EVENTS) if loop.get_debug(): self._source_traceback = traceback.extract_stack(sys._getframe(1)) @@ -279,7 +307,10 @@ def update_transfer_encoding(self): self.headers[hdrs.CONTENT_LENGTH] = str(len(self.body)) def update_auth(self, auth): - """Set basic auth.""" + """Set basic auth. or custom if callable""" + if callable(auth): + auth(self) + return if auth is None: auth = self.auth if auth is None: @@ -488,6 +519,15 @@ def terminate(self): self._writer.cancel() self._writer = None + async def dispatch_hooks(self, event, **extras): + """call and hooks associated to an event""" + """return results which should be a response or None""" + rsp = None + for hook in self._hooks[event]: + rsp = await hook(self, **extras) + + return rsp or self.response + class ClientResponse(HeadersMixin): @@ -821,3 +861,4 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): # for exceptions, response object can closes connection # is state is broken self.release() + diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index d06c97eb067..7b2d1e56384 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -2535,3 +2535,25 @@ async def canceller(): fut2.cancel() await asyncio.gather(fetch1(), fetch2(), canceller(), loop=loop) + + +async def test_custom_auth_using_response_hook(loop, test_client): + + class CustomAuth(): + async def do_custom_auth(self, req, **extras): + return req.response + + def __call__(self, r): + r.register_hook('response', self.do_custom_auth) + + async def handler(request): + return web.Response() + + app = web.Application() + app.router.add_route('GET', '/', handler) + + client = await test_client(app) + resp = await client.get('/', auth=CustomAuth()) + + assert resp.status == 200 + assert resp diff --git a/tests/test_client_request.py b/tests/test_client_request.py index a60d93c31e9..5c04b7f5879 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -358,6 +358,36 @@ def test_basic_auth_from_url_overriden(make_request): assert 'python.org' == req.host +def test_custom_auth_register_hook(make_request): + auth = mock.MagicMock() + req = make_request('get', 'http://0.0.0.0/', auth=auth) + auth.assert_called_once_with(req) + + +def test_hook_registration(make_request): + async def hook(): + await asyncio.sleep(0) + + req = make_request('get', 'http://0.0.0.0/') + req.register_hook('response', hook) + assert hook in req._hooks['response'] + + req.deregister_hook('response', hook) + assert hook not in req._hooks['response'] + + +async def test_response_hook_called(make_request): + async def hook(req): + return req.response + + response = mock.Mock() + req = make_request('get', 'http://0.0.0.0/') + req.register_hook('response', hook) + req.response = response + rsp = await req.dispatch_hooks('response') + assert rsp == response + + def test_path_is_not_double_encoded1(make_request): req = make_request('get', "http://0.0.0.0/get/test case") assert req.url.raw_path == "/get/test%20case" diff --git a/tests/test_client_session.py b/tests/test_client_session.py index a2896681d87..459afd104de 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -12,7 +12,7 @@ import aiohttp from aiohttp import hdrs, web from aiohttp.client import ClientSession -from aiohttp.client_reqrep import ClientRequest +from aiohttp.client_reqrep import ClientRequest, ClientResponse from aiohttp.connector import BaseConnector, TCPConnector @@ -563,3 +563,35 @@ async def new_headers( await session.get('http://example.com') assert MyClientRequest.headers['foo'] == 'bar' + + +async def test_dispatch_response_hooks(loop): + + with mock.patch("aiohttp.client.TCPConnector.connect") as connect_patched: + request = mock.Mock(ClientRequest) + request_class = mock.MagicMock(return_value=request) + request_class.return_value = request + response = mock.MagicMock() + request.send.return_value = response + + f = loop.create_future() + f.set_result(None) + response.start.return_value = f + response.status = 200 + + f = loop.create_future() + f.set_result(response) + request.dispatch_hooks.return_value = f + + f = loop.create_future() + f.set_result(mock.MagicMock()) + connect_patched.return_value = f + + session = aiohttp.ClientSession( + loop=loop, + request_class=request_class, + cookie_jar=mock.MagicMock() + ) + await session.get('http://test.example.com') + await session.close() + request.dispatch_hooks.assert_called_with('response') From 312e72b9d6f81eb6f819f64d650253accec7bb30 Mon Sep 17 00:00:00 2001 From: Al Date: Wed, 6 Dec 2017 16:34:49 -0500 Subject: [PATCH 02/34] per flake8 recomendations --- aiohttp/client_reqrep.py | 11 +++++++---- tests/test_client_session.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 9e63f4b32ba..3d8759be0af 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -1,13 +1,13 @@ import asyncio import codecs import collections +import inspect import io import json import ssl import sys import traceback import warnings -import inspect from collections import namedtuple from hashlib import md5, sha1, sha256 from http.cookies import CookieError, Morsel, SimpleCookie @@ -61,9 +61,13 @@ class HooksMixin: def register_hook(self, event, hook): """Register an event hook.""" if event not in self._hooks: - raise ValueError('Unsupported event specified, with event name {0}'.format(event)) + raise ValueError( + "Unsupported event specified, " + "with event name {0}".format(event)) - assert inspect.iscoroutinefunction(hook), "hook {0} must be a coroutine".format(getattr(hook, '__name__', '')) + assert inspect.iscoroutinefunction(hook), \ + "hook {0} must be a coroutine".format( + getattr(hook, '__name__', '')) self._hooks[event].append(hook) @@ -861,4 +865,3 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): # for exceptions, response object can closes connection # is state is broken self.release() - diff --git a/tests/test_client_session.py b/tests/test_client_session.py index 459afd104de..9edf3e0b78f 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -12,7 +12,7 @@ import aiohttp from aiohttp import hdrs, web from aiohttp.client import ClientSession -from aiohttp.client_reqrep import ClientRequest, ClientResponse +from aiohttp.client_reqrep import ClientRequest from aiohttp.connector import BaseConnector, TCPConnector From 083172dbf919a3fa993d5da2c4dea750e6774da5 Mon Sep 17 00:00:00 2001 From: Al Date: Tue, 12 Dec 2017 08:39:57 -0500 Subject: [PATCH 03/34] - Update docs - retrofit BasicAuth --- CHANGES/434.feature | 1 + aiohttp/client_reqrep.py | 10 +++------- aiohttp/helpers.py | 3 +++ docs/client_reference.rst | 9 +++++++-- 4 files changed, 14 insertions(+), 9 deletions(-) create mode 100644 CHANGES/434.feature diff --git a/CHANGES/434.feature b/CHANGES/434.feature new file mode 100644 index 00000000000..09b3a4647cf --- /dev/null +++ b/CHANGES/434.feature @@ -0,0 +1 @@ +Allow Custom Authentication handlers to be passed into ClientSession \ No newline at end of file diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 3d8759be0af..27ce6ef441d 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -312,18 +312,14 @@ def update_transfer_encoding(self): def update_auth(self, auth): """Set basic auth. or custom if callable""" - if callable(auth): - auth(self) - return if auth is None: auth = self.auth if auth is None: return - if not isinstance(auth, helpers.BasicAuth): - raise TypeError('BasicAuth() tuple is required instead') - - self.headers[hdrs.AUTHORIZATION] = auth.encode() + if not callable(auth): + raise TypeError('Auth must be a callable') + auth(self) def update_body_from_data(self, body): if not body: diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 478138a6985..5c5299dca08 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -102,6 +102,9 @@ def encode(self): creds = ('%s:%s' % (self.login, self.password)).encode(self.encoding) return 'Basic %s' % base64.b64encode(creds).decode(self.encoding) + def __call__(self, r): + r.headers[hdrs.AUTHORIZATION] = self.encode() + def strip_auth_from_url(url): auth = BasicAuth.from_url(url) diff --git a/docs/client_reference.rst b/docs/client_reference.rst index 572a3567229..ced74e37531 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -86,8 +86,9 @@ The client session supports the context manager protocol for self closing. Iterable of :class:`str` or :class:`~aiohttp.istr` (optional) - :param aiohttp.BasicAuth auth: an object that represents HTTP Basic - Authorization (optional) + :param auth: a callable that represents an authentication scheme. + the object will be called passing in the current request object + during the initialization of a request (optional) :param version: supported HTTP version, ``HTTP 1.1`` by default. @@ -1404,6 +1405,10 @@ BasicAuth :return: encoded authentication data, :class:`str`. + .. method:: __call__() + + Used to attach the ``Authorization`` header on the request + CookieJar ^^^^^^^^^ From a29dc9ceb8bdfb73c179901edfd5066d46835b2c Mon Sep 17 00:00:00 2001 From: Al Date: Tue, 12 Dec 2017 17:38:23 -0500 Subject: [PATCH 04/34] - add myself to contributors.txt --- CONTRIBUTORS.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 8f409ce906b..528e398201a 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -209,3 +209,4 @@ Yury Selivanov Yusuke Tsutsumi Марк Коренберг Семён Марьясин +Alberto Zuniga \ No newline at end of file From b988674acf69f5ba9aa9eadac695631d1ce277f1 Mon Sep 17 00:00:00 2001 From: Al Date: Wed, 13 Dec 2017 09:10:31 -0500 Subject: [PATCH 05/34] increase coverage --- tests/test_client_request.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_client_request.py b/tests/test_client_request.py index 5c04b7f5879..fdaef0333d2 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -364,6 +364,24 @@ def test_custom_auth_register_hook(make_request): auth.assert_called_once_with(req) +def test_unsupportd_hook_raises(make_request): + with pytest.raises(ValueError): + req = make_request('get', 'http://0.0.0.0/') + req.register_hook('request', mock.MagicMock()) + + +def test_deregister_non_existing_hook(make_request): + req = make_request('get', 'http://0.0.0.0/') + result = req.deregister_hook('response', mock.MagicMock()) + assert result is False + + +def test_custom_auth_non_callable_raises(make_request): + with pytest.raises(TypeError): + auth = 'no call' + req = make_request('get', 'http://0.0.0.0/', auth=auth) + + def test_hook_registration(make_request): async def hook(): await asyncio.sleep(0) From 7e3f55514f616d32eb4fa468323f089c756d8793 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 13 Dec 2017 17:37:45 +0200 Subject: [PATCH 06/34] Drop encoding param (#2606) * Drop encoding param * Add CHANGES --- CHANGES/2606.removal | 1 + aiohttp/client.py | 9 --------- tests/test_client_functional.py | 13 ------------- 3 files changed, 1 insertion(+), 22 deletions(-) create mode 100644 CHANGES/2606.removal diff --git a/CHANGES/2606.removal b/CHANGES/2606.removal new file mode 100644 index 00000000000..3230db74f9f --- /dev/null +++ b/CHANGES/2606.removal @@ -0,0 +1 @@ +Drop deprecated `encoding` parameter from client API diff --git a/aiohttp/client.py b/aiohttp/client.py index 8dd5fd90e16..6f205326bfb 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -154,7 +154,6 @@ async def _request(self, method, url, *, auth=None, allow_redirects=True, max_redirects=10, - encoding=None, compress=None, chunked=None, expect100=False, @@ -172,12 +171,6 @@ async def _request(self, method, url, *, # set the default to None because we need to detect if the user wants # to use the existing timeouts by setting timeout to None. - if encoding is not None: - warnings.warn( - "encoding parameter is not supported, " - "please use FormData(charset='utf-8') instead", - DeprecationWarning) - if self.closed: raise RuntimeError('Session is closed') @@ -793,7 +786,6 @@ def request(method, url, *, auth=None, allow_redirects=True, max_redirects=10, - encoding=None, version=http.HttpVersion11, compress=None, chunked=None, @@ -854,7 +846,6 @@ def request(method, url, *, auth=auth, allow_redirects=allow_redirects, max_redirects=max_redirects, - encoding=encoding, compress=compress, chunked=chunked, expect100=expect100, diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index d06c97eb067..f7892ed766a 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -2004,19 +2004,6 @@ async def handler_redirect(request): assert data == body -async def test_encoding_deprecated(loop, test_client): - - async def handler_redirect(request): - return web.Response(status=301) - - app = web.Application() - app.router.add_route('GET', '/redirect', handler_redirect) - client = await test_client(app) - - with pytest.warns(DeprecationWarning): - await client.get('/', encoding='utf-8') - - async def test_chunked_deprecated(loop, test_client): async def handler_redirect(request): From 70546390041af52eed33090c3c57e30dda213e63 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 14 Dec 2017 07:47:15 +0200 Subject: [PATCH 07/34] Fix docstrings for req.host and req.remote (#2607) --- CHANGES/2591.doc | 1 + CHANGES/2592.doc | 1 + aiohttp/web_request.py | 20 +++++++++++--------- 3 files changed, 13 insertions(+), 9 deletions(-) create mode 100644 CHANGES/2591.doc create mode 100644 CHANGES/2592.doc diff --git a/CHANGES/2591.doc b/CHANGES/2591.doc new file mode 100644 index 00000000000..ae0e5fb3f5b --- /dev/null +++ b/CHANGES/2591.doc @@ -0,0 +1 @@ +Fix docstring for request.host diff --git a/CHANGES/2592.doc b/CHANGES/2592.doc new file mode 100644 index 00000000000..6ad8c8e94ab --- /dev/null +++ b/CHANGES/2592.doc @@ -0,0 +1 @@ +Fix docstring for request.remote diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index ea94c4ce7e5..26e4558bc40 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -255,6 +255,11 @@ def forwarded(self): def scheme(self): """A string representing the scheme of the request. + Hostname is resolved in this order: + + - overridden value by .clone(scheme=new_scheme) call. + - type of connection to peer: HTTPS if socket is SSL, HTTP otherwise. + 'http' or 'https'. """ scheme = self._scheme @@ -285,13 +290,11 @@ def version(self): def host(self): """Hostname of the request. - Hostname is resolved through the following headers, in this order: - - - Forwarded - - X-Forwarded-Host - - Host + Hostname is resolved in this order: - Returns str, or None if no hostname is found in the headers. + - overridden value by .clone(host=new_host) call. + - HOST HTTP header + - socket.getfqdn() value """ host = self._host if host is not None: @@ -306,10 +309,9 @@ def host(self): def remote(self): """Remote IP of client initiated HTTP request. - The IP is resolved through the following headers, in this order: + The IP is resolved in this order: - - Forwarded - - X-Forwarded-For + - overridden value by .clone(remote=new_remote) call. - peername of opened socket """ remote = self._remote From 245d3bf50e39ba7cf4ba6327a4be1934e7d9c19d Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 14 Dec 2017 08:00:24 +0200 Subject: [PATCH 08/34] Update spelling whitelist --- docs/spelling_wordlist.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 9dc2509c13a..751f1f9dd44 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -71,6 +71,7 @@ django Django dns DNSResolver +docstring Dup elasticsearch encodings From 76f92744556e4d7770f9b58db29a8fe1bc845685 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE?= <815750986@qq.com> Date: Thu, 14 Dec 2017 18:32:55 +0800 Subject: [PATCH 09/34] Support .netrc by trust_env (#2584) * Support .netrc by trust_env --- CHANGES/2581.feature | 1 + CONTRIBUTORS.txt | 1 + aiohttp/helpers.py | 38 ++++++++++++++++ tests/test_proxy_functional.py | 81 ++++++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+) create mode 100644 CHANGES/2581.feature diff --git a/CHANGES/2581.feature b/CHANGES/2581.feature new file mode 100644 index 00000000000..f18f42bf286 --- /dev/null +++ b/CHANGES/2581.feature @@ -0,0 +1 @@ +Support `.netrc` by `trust_env` diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 8f409ce906b..489a5b1e21a 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -200,6 +200,7 @@ W. Trevor King Will McGugan Willem de Groot Wilson Ong +Wei Lin Yannick Koechlin Yannick Péroux Yegor Roganov diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 478138a6985..abfacf293af 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -7,6 +7,7 @@ import datetime import functools import inspect +import netrc import os import re import time @@ -111,12 +112,40 @@ def strip_auth_from_url(url): return url.with_user(None), auth +def netrc_from_env(): + netrc_obj = None + netrc_path = os.environ.get('NETRC') + try: + if netrc_path is not None: + netrc_path = Path(netrc_path) + else: + home_dir = Path.home() + if os.name == 'nt': # pragma: no cover + netrc_path = home_dir.joinpath('_netrc') + else: + netrc_path = home_dir.joinpath('.netrc') + + if netrc_path and netrc_path.is_file(): + try: + netrc_obj = netrc.netrc(str(netrc_path)) + except (netrc.NetrcParseError, OSError) as e: + client_logger.warning(".netrc file parses fail: %s", e) + + if netrc_obj is None: + client_logger.warning("could't find .netrc file") + except RuntimeError as e: # pragma: no cover + """ handle error raised by pathlib """ + client_logger.warning("could't find .netrc file: %s", e) + return netrc_obj + + ProxyInfo = namedtuple('ProxyInfo', 'proxy proxy_auth') def proxies_from_env(): proxy_urls = {k: URL(v) for k, v in getproxies().items() if k in ('http', 'https')} + netrc_obj = netrc_from_env() stripped = {k: strip_auth_from_url(v) for k, v in proxy_urls.items()} ret = {} for proto, val in stripped.items(): @@ -125,6 +154,15 @@ def proxies_from_env(): client_logger.warning( "HTTPS proxies %s are not supported, ignoring", proxy) continue + if netrc_obj and auth is None: + auth_from_netrc = netrc_obj.authenticators(proxy.host) + if auth_from_netrc is not None: + # auth_from_netrc is a (`user`, `account`, `password`) tuple, + # `user` and `account` both can be username, + # if `user` is None, use `account` + *logins, password = auth_from_netrc + auth = BasicAuth(logins[0] if logins[0] else logins[-1], + password) ret[proto] = ProxyInfo(proxy, auth) return ret diff --git a/tests/test_proxy_functional.py b/tests/test_proxy_functional.py index 85d3b35033d..f968710ba4b 100644 --- a/tests/test_proxy_functional.py +++ b/tests/test_proxy_functional.py @@ -1,5 +1,6 @@ import asyncio import os +import pathlib from unittest import mock import pytest @@ -478,10 +479,22 @@ def _make_ssl_transport_dummy(self, rawsock, protocol, sslcontext, _make_ssl_transport_dummy) +original_is_file = pathlib.Path.is_file + + +def mock_is_file(self): + """ make real netrc file invisible in home dir """ + if self.name in ['_netrc', '.netrc'] and self.parent == self.home(): + return False + else: + return original_is_file(self) + + async def test_proxy_from_env_http(proxy_test_server, get_request, mocker): url = 'http://aiohttp.io/path' proxy = await proxy_test_server() mocker.patch.dict(os.environ, {'http_proxy': str(proxy.url)}) + mocker.patch('pathlib.Path.is_file', mock_is_file) await get_request(url=url, trust_env=True) @@ -511,11 +524,79 @@ async def test_proxy_from_env_http_with_auth(proxy_test_server, assert proxy.request.headers['Proxy-Authorization'] == auth.encode() +async def test_proxy_from_env_http_with_auth_from_netrc( + proxy_test_server, get_request, tmpdir, mocker): + url = 'http://aiohttp.io/path' + proxy = await proxy_test_server() + auth = aiohttp.BasicAuth('user', 'pass') + netrc_file = tmpdir.join('test_netrc') + netrc_file_data = 'machine 127.0.0.1 login %s password %s' % ( + auth.login, auth.password) + with open(str(netrc_file), 'w') as f: + f.write(netrc_file_data) + mocker.patch.dict(os.environ, {'http_proxy': str(proxy.url), + 'NETRC': str(netrc_file)}) + + await get_request(url=url, trust_env=True) + + 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' + assert proxy.request.headers['Proxy-Authorization'] == auth.encode() + + +async def test_proxy_from_env_http_without_auth_from_netrc( + proxy_test_server, get_request, tmpdir, mocker): + url = 'http://aiohttp.io/path' + proxy = await proxy_test_server() + auth = aiohttp.BasicAuth('user', 'pass') + netrc_file = tmpdir.join('test_netrc') + netrc_file_data = 'machine 127.0.0.2 login %s password %s' % ( + auth.login, auth.password) + with open(str(netrc_file), 'w') as f: + f.write(netrc_file_data) + mocker.patch.dict(os.environ, {'http_proxy': str(proxy.url), + 'NETRC': str(netrc_file)}) + + await get_request(url=url, trust_env=True) + + 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' + assert 'Proxy-Authorization' not in proxy.request.headers + + +async def test_proxy_from_env_http_without_auth_from_wrong_netrc( + proxy_test_server, get_request, tmpdir, mocker): + url = 'http://aiohttp.io/path' + proxy = await proxy_test_server() + auth = aiohttp.BasicAuth('user', 'pass') + netrc_file = tmpdir.join('test_netrc') + invalid_data = 'machine 127.0.0.1 %s pass %s' % ( + auth.login, auth.password) + with open(str(netrc_file), 'w') as f: + f.write(invalid_data) + + mocker.patch.dict(os.environ, {'http_proxy': str(proxy.url), + 'NETRC': str(netrc_file)}) + + await get_request(url=url, trust_env=True) + + 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' + assert 'Proxy-Authorization' not in proxy.request.headers + + @pytest.mark.xfail async def xtest_proxy_from_env_https(proxy_test_server, get_request, mocker): url = 'https://aiohttp.io/path' proxy = await proxy_test_server() mocker.patch.dict(os.environ, {'https_proxy': str(proxy.url)}) + mock.patch('pathlib.Path.is_file', mock_is_file) await get_request(url=url, trust_env=True) From dd234f33f206e0feabfe6f7364b2d45832d1336d Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 14 Dec 2017 12:38:57 +0200 Subject: [PATCH 10/34] Document .netrc support --- docs/client_advanced.rst | 3 +++ docs/client_reference.rst | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/docs/client_advanced.rst b/docs/client_advanced.rst index 02bed9995b4..1e69ae13721 100644 --- a/docs/client_advanced.rst +++ b/docs/client_advanced.rst @@ -503,6 +503,9 @@ insensitive):: async with session.get("http://python.org", trust_env=True) as resp: print(resp.status) +Proxy credentials are given from ``~/.netrc`` file if present (see +:class:`aiohttp.ClientSession` for more details). + Graceful Shutdown ----------------- diff --git a/docs/client_reference.rst b/docs/client_reference.rst index fe8292329fd..9d9b45424f4 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -141,8 +141,19 @@ The client session supports the context manager protocol for self closing. *HTTPS_PROXY* environment variables if the parameter is ``True`` (``False`` by default). + Get proxy credentials from ``~/.netrc`` file if + present. + + .. seealso:: + + ``.netrc`` documentation: https://www.gnu.org/software/inetutils/manual/html_node/The-_002enetrc-file.html + .. versionadded:: 2.3 + .. versionchanged:: 3.0 + + Added support for ``~/.netrc`` file. + .. attribute:: closed ``True`` if the session has been closed, ``False`` otherwise. From c256c29af5e18282631f0ee3e58ea6cd305c89fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=BE=D1=80=D0=B5=D0=BD=D0=B1=D0=B5=D1=80=D0=B3=20?= =?UTF-8?q?=D0=9C=D0=B0=D1=80=D0=BA?= Date: Thu, 14 Dec 2017 17:30:43 +0500 Subject: [PATCH 11/34] Fix small typos (#2609) --- aiohttp/client.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 6f205326bfb..41ecd3e96e1 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -206,7 +206,7 @@ async def _request(self, method, url, *, try: proxy = URL(proxy) except ValueError: - raise InvalidURL(url) + raise InvalidURL(proxy) # timeout is cumulative for all request operations # (request, redirects, responses, data consuming) @@ -215,8 +215,6 @@ async def _request(self, method, url, *, timeout if timeout is not sentinel else self._read_timeout) handle = tm.start() - url = URL(url) - traces = [ Trace( self, From b57caab20899f7e00607d63645eecbff8cf4a161 Mon Sep 17 00:00:00 2001 From: Sebastien Geffroy Date: Thu, 14 Dec 2017 14:52:08 +0100 Subject: [PATCH 12/34] Avoid to create unnecessary resources (#2586) (#2603) * Avoid to create unnecessary resources (#2586) Do not create a new resource when adding a route with the same name and path of the last added resource. * Add a test for AbstractResource().raw_match() * PrefixResource().raw_match() should always return False --- CHANGES/2586.feature | 2 ++ aiohttp/web_urldispatcher.py | 18 +++++++++++++++++ tests/test_web_urldispatcher.py | 36 +++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 CHANGES/2586.feature diff --git a/CHANGES/2586.feature b/CHANGES/2586.feature new file mode 100644 index 00000000000..c5082930050 --- /dev/null +++ b/CHANGES/2586.feature @@ -0,0 +1,2 @@ +Avoid to create a new resource when adding a route with the same +name and path of the last added resource \ No newline at end of file diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index ad5d7ecf616..09cf4b65603 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -92,6 +92,10 @@ def get_info(self): def freeze(self): pass + @abc.abstractmethod + def raw_match(self, path): + """Perform a raw match against path""" + class AbstractRoute(abc.ABC): @@ -332,6 +336,9 @@ def _match(self, path): else: return None + def raw_match(self, path): + return self._path == path + def get_info(self): return {'path': self._path} @@ -400,6 +407,9 @@ def _match(self, path): return {key: unquote(value, unsafe='+') for key, value in match.groupdict().items()} + def raw_match(self, path): + return self._formatter == path + def get_info(self): return {'formatter': self._formatter, 'pattern': self._pattern} @@ -428,6 +438,9 @@ def add_prefix(self, prefix): assert len(prefix) > 1 self._prefix = prefix + self._prefix + def raw_match(self, prefix): + return False + # TODO: impl missing abstract methods @@ -830,6 +843,11 @@ def register_resource(self, resource): def add_resource(self, path, *, name=None): if path and not path.startswith('/'): raise ValueError("path should be started with / or be empty") + # Reuse last added resource if path and name are the same + if self._resources: + resource = self._resources[-1] + if resource.name == name and resource.raw_match(path): + return resource if not ('{' in path or '}' in path or ROUTE_RE.search(path)): url = URL(path) resource = PlainResource(url.raw_path, name=name) diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index e11e786531d..09475dcbed6 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -342,3 +342,39 @@ async def handler(_): r = await client.head('/b') assert r.status == 405 await r.release() + + +@pytest.mark.parametrize("path", [ + '/a', + '/{a}', +]) +def test_reuse_last_added_resource(path): + """ + Test that adding a route with the same name and path of the last added + resource doesn't create a new resource. + """ + app = web.Application() + + async def handler(request): + return web.Response() + + app.router.add_get(path, handler, name="a") + app.router.add_post(path, handler, name="a") + + assert len(app.router.resources()) == 1 + + +def test_resource_raw_match(): + app = web.Application() + + async def handler(request): + return web.Response() + + route = app.router.add_get("/a", handler, name="a") + assert route.resource.raw_match("/a") + + route = app.router.add_get("/{b}", handler, name="b") + assert route.resource.raw_match("/{b}") + + resource = app.router.add_static("/static", ".") + assert not resource.raw_match("/static") From 29e5eac3e8110574446f794fbddae70b1582debb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9A=D0=BE=D1=80=D0=B5=D0=BD=D0=B1=D0=B5=D1=80=D0=B3=20?= =?UTF-8?q?=D0=9C=D0=B0=D1=80=D0=BA?= Date: Mon, 11 Dec 2017 13:27:09 +0500 Subject: [PATCH 13/34] Fix documentation around JSON and content-type (#2598) --- docs/client_advanced.rst | 26 +++++++++++++++++--------- docs/client_quickstart.rst | 18 +++++++++++------- docs/web_advanced.rst | 19 +++++++------------ 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/docs/client_advanced.rst b/docs/client_advanced.rst index 1e69ae13721..2524687f70a 100644 --- a/docs/client_advanced.rst +++ b/docs/client_advanced.rst @@ -26,27 +26,35 @@ Custom Request Headers If you need to add HTTP headers to a request, pass them in a :class:`dict` to the *headers* parameter. -For example, if you want to specify the content-type for the previous -example:: +For example, if you want to specify the content-type directly:: - import json - url = 'https://api.github.com/some/endpoint' - payload = {'some': 'data'} - headers = {'content-type': 'application/json'} + url = 'http://example.com/image' + payload = b'GIF89a\x01\x00\x01\x00\x00\xff\x00,\x00\x00' + b'\x00\x00\x01\x00\x01\x00\x00\x02\x00;' + headers = {'content-type': 'image/gif'} await session.post(url, - data=json.dumps(payload), + data=payload, headers=headers) You also can set default headers for all session requests:: - async with aiohttp.ClientSession( - headers={"Authorization": "Basic bG9naW46cGFzcw=="}) as session: + headers={"Authorization": "Basic bG9naW46cGFzcw=="} + async with aiohttp.ClientSession(headers) as session: async with session.get("http://httpbin.org/headers") as r: json_body = await r.json() assert json_body['headers']['Authorization'] == \ 'Basic bG9naW46cGFzcw==' +Typical use case is sending JSON body. You can specify content type +directly as shown above, but it is more convenient to use special keyword +``json``:: + + await session.post(url, json={'example': 'text'}) + +The same for *text/plain*:: + + await session.post(url, text='Привет, Мир!') Custom Cookies -------------- diff --git a/docs/client_quickstart.rst b/docs/client_quickstart.rst index c059e2e4349..977329052c4 100644 --- a/docs/client_quickstart.rst +++ b/docs/client_quickstart.rst @@ -251,18 +251,22 @@ dictionary of data will automatically be form-encoded when the request is made:: } If you want to send data that is not form-encoded you can do it by -passing a :class:`str` instead of a :class:`dict`. This data will be -posted directly. +passing a :class:`bytes` instead of a :class:`dict`. This data will be +posted directly and content-type set to 'application/octet-stream' by +default:: -For example, the GitHub API v3 accepts JSON-Encoded POST/PATCH data:: + async with session.post(url, data=b'\x00Binary-data\x00') as resp: + ... - import json - url = 'https://api.github.com/some/endpoint' - payload = {'some': 'data'} +If you want to send JSON data:: - async with session.post(url, data=json.dumps(payload)) as resp: + async with session.post(url, json={'example': 'test'}) as resp: ... +To send text with appropriate content-type just use ``text`` attribute :: + + async with session.post(url, text='Тест') as resp: + ... POST a Multipart-Encoded File ----------------------------- diff --git a/docs/web_advanced.rst b/docs/web_advanced.rst index 486e32a97b4..b38da24eafb 100644 --- a/docs/web_advanced.rst +++ b/docs/web_advanced.rst @@ -389,25 +389,20 @@ A common use of middlewares is to implement custom error pages. The following example will render 404 errors using a JSON response, as might be appropriate a JSON REST service:: - import json from aiohttp import web - def json_error(message): - return web.Response( - body=json.dumps({'error': message}).encode('utf-8'), - content_type='application/json') - @web.middleware async def error_middleware(request, handler): try: response = await handler(request) - if response.status == 404: - return json_error(response.message) - return response + if response.status != 404: + return response + message = response.message except web.HTTPException as ex: - if ex.status == 404: - return json_error(ex.reason) - raise + if ex.status != 404: + raise + message = ex.reason + return web.json_response({'error': message}) app = web.Application(middlewares=[error_middleware]) From 1cb4fa276a7bae85429ee2eb7eedebe064c699d2 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Fri, 15 Dec 2017 00:12:53 +0200 Subject: [PATCH 14/34] Small documentation fix --- docs/client_advanced.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/client_advanced.rst b/docs/client_advanced.rst index 2524687f70a..3aeff04a71e 100644 --- a/docs/client_advanced.rst +++ b/docs/client_advanced.rst @@ -40,7 +40,7 @@ For example, if you want to specify the content-type directly:: You also can set default headers for all session requests:: headers={"Authorization": "Basic bG9naW46cGFzcw=="} - async with aiohttp.ClientSession(headers) as session: + async with aiohttp.ClientSession(headers)=headers as session: async with session.get("http://httpbin.org/headers") as r: json_body = await r.json() assert json_body['headers']['Authorization'] == \ From d02ca2ad2a41a30c3d843373732ba841d1f3825a Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Fri, 15 Dec 2017 08:34:13 +0200 Subject: [PATCH 15/34] Fix docs typo --- docs/client_advanced.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/client_advanced.rst b/docs/client_advanced.rst index 3aeff04a71e..66792de2ece 100644 --- a/docs/client_advanced.rst +++ b/docs/client_advanced.rst @@ -40,7 +40,7 @@ For example, if you want to specify the content-type directly:: You also can set default headers for all session requests:: headers={"Authorization": "Basic bG9naW46cGFzcw=="} - async with aiohttp.ClientSession(headers)=headers as session: + async with aiohttp.ClientSession(headers=headers) as session: async with session.get("http://httpbin.org/headers") as r: json_body = await r.json() assert json_body['headers']['Authorization'] == \ From 05a5a6c3576319eec9224648a2a288f28a238927 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Sat, 16 Dec 2017 23:04:34 +0200 Subject: [PATCH 16/34] Add Nikolay Kim to contributors --- CONTRIBUTORS.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 489a5b1e21a..b57b76924db 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -140,6 +140,7 @@ Morgan Delahaye-Prat Moss Collum Mun Gwan-gyeong Nicolas Braem +Nikolay Kim Nikolay Novik Olaf Conradi Pahaz Blinov From c178a99c871cb93afe641d2f654a35bdec1c466d Mon Sep 17 00:00:00 2001 From: Sebastien Geffroy Date: Sat, 16 Dec 2017 23:11:58 +0100 Subject: [PATCH 17/34] Add support to Flask-style decorators with class-based Views (#2611) * Add support to Flask-style decorators with class-based Views * Add versionadded 3.0 in documentation --- CHANGES/2472.feature | 1 + aiohttp/web_urldispatcher.py | 15 +++++- docs/web_quickstart.rst | 16 ++++++- docs/web_reference.rst | 34 ++++++++++++- tests/test_web_urldispatcher.py | 85 +++++++++++++++++++++++++++++++++ 5 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 CHANGES/2472.feature diff --git a/CHANGES/2472.feature b/CHANGES/2472.feature new file mode 100644 index 00000000000..21e7c4b7592 --- /dev/null +++ b/CHANGES/2472.feature @@ -0,0 +1 @@ +Added support to Flask-style decorators with class-based Views. \ No newline at end of file diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 09cf4b65603..e4d6489d357 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -33,7 +33,7 @@ 'AbstractResource', 'Resource', 'PlainResource', 'DynamicResource', 'AbstractRoute', 'ResourceRoute', 'StaticResource', 'View', 'RouteDef', 'RouteTableDef', - 'head', 'get', 'post', 'patch', 'put', 'delete', 'route') + 'head', 'get', 'post', 'patch', 'put', 'delete', 'route', 'view') HTTP_METHOD_RE = re.compile(r"^[0-9A-Za-z!#\$%&'\*\+\-\.\^_`\|~]+$") ROUTE_RE = re.compile(r'(\{[_a-zA-Z][^{}]*(?:\{[^{}]*\}[^{}]*)*\})') @@ -926,6 +926,12 @@ def add_delete(self, path, handler, **kwargs): """ return self.add_route(hdrs.METH_DELETE, path, handler, **kwargs) + def add_view(self, path, handler, **kwargs): + """ + Shortcut for add_route with ANY methods for a class-based view + """ + return self.add_route(hdrs.METH_ANY, path, handler, **kwargs) + def freeze(self): super().freeze() for resource in self._resources: @@ -970,6 +976,10 @@ def delete(path, handler, **kwargs): return route(hdrs.METH_DELETE, path, handler, **kwargs) +def view(path, handler, **kwargs): + return route(hdrs.METH_ANY, path, handler, **kwargs) + + class RouteTableDef(Sequence): """Route definition table""" def __init__(self): @@ -1013,3 +1023,6 @@ def patch(self, path, **kwargs): def delete(self, path, **kwargs): return self.route(hdrs.METH_DELETE, path, **kwargs) + + def view(self, path, **kwargs): + return self.route(hdrs.METH_ANY, path, **kwargs) diff --git a/docs/web_quickstart.rst b/docs/web_quickstart.rst index e0dce54f235..219501c6dd3 100644 --- a/docs/web_quickstart.rst +++ b/docs/web_quickstart.rst @@ -282,7 +282,7 @@ retrieved by :attr:`View.request` property. After implementing the view (``MyView`` from example above) should be registered in application's router:: - app.router.add_route('*', '/path/to', MyView) + app.router.add_view('/path/to', MyView) Example will process GET and POST requests for */path/to* but raise *405 Method not allowed* exception for unimplemented HTTP methods. @@ -348,6 +348,20 @@ Route decorators are closer to Flask approach:: app.router.add_routes(routes) +It is also possible to use decorators with class-based views:: + + routes = web.RouteTableDef() + + @routes.view("/view") + class MyView(web.View): + async def get(self): + ... + + async def post(self): + ... + + app.router.add_routes(routes) + The example creates a :class:`aiohttp.web.RouteTableDef` container first. The container is a list-like object with additional decorators diff --git a/docs/web_reference.rst b/docs/web_reference.rst index f332eb91712..5302189a58e 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -1583,6 +1583,13 @@ Router is any object that implements :class:`AbstractRouter` interface. Shortcut for adding a DELETE handler. Calls the :meth:`add_route` with \ ``method`` equals to ``'DELETE'``. + .. method:: add_view(path, handler, **kwargs) + + Shortcut for adding a class-based view handler. Calls the \ + :meth:`add_routre` with ``method`` equals to ``'*'``. + + .. versionadded:: 3.0 + .. method:: add_static(prefix, path, *, name=None, expect_handler=None, \ chunk_size=256*1024, \ response_factory=StreamResponse, \ @@ -2081,6 +2088,13 @@ The definition is created by functions like :func:`get` or .. versionadded:: 2.3 +.. function:: view(path, handler, *, name=None, expect_handler=None) + + Return :class:`RouteDef` for processing ``ANY`` requests. See + :meth:`UrlDispatcher.add_view` for information about parameters. + + .. versionadded:: 3.0 + .. function:: route(method, path, handler, *, name=None, expect_handler=None) Return :class:`RouteDef` for processing ``POST`` requests. See @@ -2111,6 +2125,15 @@ A routes table definition used for describing routes by decorators app.router.add_routes(routes) + + @routes.view("/view") + class MyView(web.View): + async def get(self): + ... + + async def post(self): + ... + .. class:: RouteTableDef() A sequence of :class:`RouteDef` instances (implements @@ -2157,6 +2180,15 @@ A routes table definition used for describing routes by decorators See :meth:`UrlDispatcher.add_delete` for information about parameters. + .. decoratormethod:: view(path, *, name=None, expect_handler=None) + + Add a new :class:`RouteDef` item for registering ``ANY`` methods + against a class-based view. + + See :meth:`UrlDispatcher.add_view` for information about parameters. + + .. versionadded:: 3.0 + .. decoratormethod:: route(method, path, *, name=None, expect_handler=None) Add a new :class:`RouteDef` item for registering a web-handler @@ -2217,7 +2249,7 @@ View resp = await post_response(self.request) return resp - app.router.add_route('*', '/view', MyView) + app.router.add_view('/view', MyView) The view raises *405 Method Not allowed* (:class:`HTTPMethodNowAllowed`) if requested web verb is not diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index 09475dcbed6..b0d4a965f4d 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -378,3 +378,88 @@ async def handler(request): resource = app.router.add_static("/static", ".") assert not resource.raw_match("/static") + + +async def test_add_view(loop, test_client): + app = web.Application() + + class MyView(web.View): + async def get(self): + return web.Response() + + async def post(self): + return web.Response() + + app.router.add_view("/a", MyView) + + client = await test_client(app) + + r = await client.get("/a") + assert r.status == 200 + await r.release() + + r = await client.post("/a") + assert r.status == 200 + await r.release() + + r = await client.put("/a") + assert r.status == 405 + await r.release() + + +async def test_decorate_view(loop, test_client): + routes = web.RouteTableDef() + + @routes.view("/a") + class MyView(web.View): + async def get(self): + return web.Response() + + async def post(self): + return web.Response() + + app = web.Application() + app.router.add_routes(routes) + + client = await test_client(app) + + r = await client.get("/a") + assert r.status == 200 + await r.release() + + r = await client.post("/a") + assert r.status == 200 + await r.release() + + r = await client.put("/a") + assert r.status == 405 + await r.release() + + +async def test_web_view(loop, test_client): + app = web.Application() + + class MyView(web.View): + async def get(self): + return web.Response() + + async def post(self): + return web.Response() + + app.router.add_routes([ + web.view("/a", MyView) + ]) + + client = await test_client(app) + + r = await client.get("/a") + assert r.status == 200 + await r.release() + + r = await client.post("/a") + assert r.status == 200 + await r.release() + + r = await client.put("/a") + assert r.status == 405 + await r.release() From 486eaf2b134eff2b2188bdeab8879c8d0339fbcc Mon Sep 17 00:00:00 2001 From: Pau Freixes Date: Mon, 18 Dec 2017 08:49:38 +0100 Subject: [PATCH 18/34] Drop access to TCP tuning options outside of the stream scope (#2612) Before this commit, the TCP tunning was allowed from PayloadWriter and Response classes, this was exclusively used by just some edge cases and caused some overhead in performance and maintainability. Since now, the TCP tunning has to be done using the stream and its the responsibility of the developer set and set back the proper values. --- CHANGES/2604.removal | 1 + aiohttp/http_writer.py | 14 -------- aiohttp/web_fileresponse.py | 22 ++++++------- aiohttp/web_protocol.py | 11 +------ aiohttp/web_response.py | 27 ---------------- tests/test_web_response.py | 64 ------------------------------------- 6 files changed, 11 insertions(+), 128 deletions(-) create mode 100644 CHANGES/2604.removal diff --git a/CHANGES/2604.removal b/CHANGES/2604.removal new file mode 100644 index 00000000000..4b569903492 --- /dev/null +++ b/CHANGES/2604.removal @@ -0,0 +1 @@ +Drop access to TCP tuning options from PayloadWriter and Response classes diff --git a/aiohttp/http_writer.py b/aiohttp/http_writer.py index 06bc79b5656..1ca0759910f 100644 --- a/aiohttp/http_writer.py +++ b/aiohttp/http_writer.py @@ -164,20 +164,6 @@ async def get_transport(self): return self._transport - @property - def tcp_nodelay(self): - return self._stream.tcp_nodelay - - def set_tcp_nodelay(self, value): - self._stream.set_tcp_nodelay(value) - - @property - def tcp_cork(self): - return self._stream.tcp_cork - - def set_tcp_cork(self, value): - self._stream.set_tcp_cork(value) - def enable_chunking(self): self.chunked = True diff --git a/aiohttp/web_fileresponse.py b/aiohttp/web_fileresponse.py index 2b4f1a6d65d..f8777a6bcd0 100644 --- a/aiohttp/web_fileresponse.py +++ b/aiohttp/web_fileresponse.py @@ -130,19 +130,15 @@ async def _sendfile_fallback(self, request, fobj, count): writer = (await super().prepare(request)) - self.set_tcp_cork(True) - try: - chunk_size = self._chunk_size - - chunk = fobj.read(chunk_size) - while True: - await writer.write(chunk) - count = count - chunk_size - if count <= 0: - break - chunk = fobj.read(min(chunk_size, count)) - finally: - self.set_tcp_nodelay(True) + chunk_size = self._chunk_size + + chunk = fobj.read(chunk_size) + while True: + await writer.write(chunk) + count = count - chunk_size + if count <= 0: + break + chunk = fobj.read(min(chunk_size, count)) await writer.drain() return writer diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index 5057c523b71..cc3a41c5ef6 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -209,6 +209,7 @@ def connection_made(self, transport): if self._tcp_keepalive: tcp_keepalive(self, transport) + self.writer.set_tcp_cork(False) self.writer.set_tcp_nodelay(True) self._manager.connection_made(self, transport) @@ -421,11 +422,6 @@ async def start(self, message, payload, handler): # notify server about keep-alive self._keepalive = resp.keep_alive - # Restore default state. - # Should be no-op if server code didn't touch these attributes. - writer.set_tcp_cork(False) - writer.set_tcp_nodelay(True) - # log access if self.access_log: self.log_access(request, resp, loop.time() - now) @@ -541,8 +537,3 @@ async def handle_parse_error(self, writer, status, exc=None, message=None): resp = self.handle_error(request, status, exc, message) await resp.prepare(request) await resp.write_eof() - - # Restore default state. - # Should be no-op if server code didn't touch these attributes. - self.writer.set_tcp_cork(False) - self.writer.set_tcp_nodelay(True) diff --git a/aiohttp/web_response.py b/aiohttp/web_response.py index 5f7b5d193a6..e5fd254849e 100644 --- a/aiohttp/web_response.py +++ b/aiohttp/web_response.py @@ -263,33 +263,6 @@ def last_modified(self, value): elif isinstance(value, str): self.headers[hdrs.LAST_MODIFIED] = value - @property - def tcp_nodelay(self): - payload_writer = self._payload_writer - assert payload_writer is not None, \ - "Cannot get tcp_nodelay for not prepared response" - return payload_writer.tcp_nodelay - - def set_tcp_nodelay(self, value): - payload_writer = self._payload_writer - assert payload_writer is not None, \ - "Cannot set tcp_nodelay for not prepared response" - payload_writer.set_tcp_nodelay(value) - - @property - def tcp_cork(self): - payload_writer = self._payload_writer - assert payload_writer is not None, \ - "Cannot get tcp_cork for not prepared response" - return payload_writer.tcp_cork - - def set_tcp_cork(self, value): - payload_writer = self._payload_writer - assert payload_writer is not None, \ - "Cannot set tcp_cork for not prepared response" - - payload_writer.set_tcp_cork(value) - def _generate_content_type_header(self, CONTENT_TYPE=hdrs.CONTENT_TYPE): params = '; '.join("%s=%s" % i for i in self._content_dict.items()) if params: diff --git a/tests/test_web_response.py b/tests/test_web_response.py index 673a776c967..7b88f5e3dc2 100644 --- a/tests/test_web_response.py +++ b/tests/test_web_response.py @@ -775,70 +775,6 @@ async def test_prepare_calls_signal(): sig.assert_called_with(req, resp) -def test_get_nodelay_unprepared(): - resp = StreamResponse() - with pytest.raises(AssertionError): - resp.tcp_nodelay - - -def test_set_nodelay_unprepared(): - resp = StreamResponse() - with pytest.raises(AssertionError): - resp.set_tcp_nodelay(True) - - -async def test_get_nodelay_prepared(): - resp = StreamResponse() - writer = mock.Mock() - writer.tcp_nodelay = False - req = make_request('GET', '/', payload_writer=writer) - - await resp.prepare(req) - assert not resp.tcp_nodelay - - -async def test_set_nodelay_prepared(): - resp = StreamResponse() - writer = mock.Mock() - req = make_request('GET', '/', payload_writer=writer) - - await resp.prepare(req) - resp.set_tcp_nodelay(True) - writer.set_tcp_nodelay.assert_called_with(True) - - -def test_get_cork_unprepared(): - resp = StreamResponse() - with pytest.raises(AssertionError): - resp.tcp_cork - - -def test_set_cork_unprepared(): - resp = StreamResponse() - with pytest.raises(AssertionError): - resp.set_tcp_cork(True) - - -async def test_get_cork_prepared(): - resp = StreamResponse() - writer = mock.Mock() - writer.tcp_cork = False - req = make_request('GET', '/', payload_writer=writer) - - await resp.prepare(req) - assert not resp.tcp_cork - - -async def test_set_cork_prepared(): - resp = StreamResponse() - writer = mock.Mock() - req = make_request('GET', '/', payload_writer=writer) - - await resp.prepare(req) - resp.set_tcp_cork(True) - writer.set_tcp_cork.assert_called_with(True) - - # Response class From 4412fc92518e2c24cdddeee1edb412610a97f5ea Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 18 Dec 2017 13:33:44 +0200 Subject: [PATCH 19/34] Fix #2604: Drop tcp_cork/tcp_nodelay from docs --- docs/web_advanced.rst | 46 ------------------------------------------ docs/web_reference.rst | 31 ---------------------------- 2 files changed, 77 deletions(-) diff --git a/docs/web_advanced.rst b/docs/web_advanced.rst index b38da24eafb..2b746442b99 100644 --- a/docs/web_advanced.rst +++ b/docs/web_advanced.rst @@ -577,52 +577,6 @@ use the following explicit technique:: admin = request.app['admin'] url = admin.router['name'].url_for() -.. _aiohttp-web-flow-control: - -Flow control ------------- - -:mod:`aiohttp.web` has sophisticated flow control for underlying TCP -sockets write buffer. - -The problem is: by default TCP sockets use `Nagle's algorithm -`_ for output -buffer which is not optimal for streaming data protocols like HTTP. - -Web server response may have one of the following states: - -1. **CORK** (:attr:`~StreamResponse.tcp_cork` is ``True``). - Don't send out partial TCP/IP frames. All queued partial frames - are sent when the option is cleared again. Optimal for sending big - portion of data since data will be sent using minimum - frames count. - - If OS does not support **CORK** mode (neither ``socket.TCP_CORK`` - nor ``socket.TCP_NOPUSH`` exists) the mode is equal to *Nagle's - enabled* one. The most widespread OS without **CORK** support is - *Windows*. - -2. **NODELAY** (:attr:`~StreamResponse.tcp_nodelay` is - ``True``). Disable the Nagle algorithm. This means that small - data pieces are always sent as soon as possible, even if there is - only a small amount of data. Optimal for transmitting short messages. - -3. Nagle's algorithm enabled (both - :attr:`~StreamResponse.tcp_cork` and - :attr:`~StreamResponse.tcp_nodelay` are ``False``). - Data is buffered until there is a sufficient amount to send out. - Avoid using this mode for sending HTTP data until you have no doubts. - -By default streaming data (:class:`StreamResponse`), regular responses -(:class:`Response` and http exceptions derived from it) and websockets -(:class:`WebSocketResponse`) use **NODELAY** mode, static file -handlers work in **CORK** mode. - -To manual mode switch :meth:`~StreamResponse.set_tcp_cork` and -:meth:`~StreamResponse.set_tcp_nodelay` methods can be used. It may -be helpful for better streaming control for example. - - .. _aiohttp-web-expect-header: *Expect* Header diff --git a/docs/web_reference.rst b/docs/web_reference.rst index 5302189a58e..f30a44f438c 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -728,37 +728,6 @@ StreamResponse as an :class:`int` or a :class:`float` object, and the value ``None`` to unset the header. - .. attribute:: tcp_cork - - :const:`~socket.TCP_CORK` (linux) or :const:`~socket.TCP_NOPUSH` - (FreeBSD and MacOSX) is applied to underlying transport if the - property is ``True``. - - Use :meth:`set_tcp_cork` to assign new value to the property. - - Default value is ``False``. - - .. method:: set_tcp_cork(value) - - Set :attr:`tcp_cork` property to *value*. - - Clear :attr:`tcp_nodelay` if *value* is ``True``. - - .. attribute:: tcp_nodelay - - :const:`~socket.TCP_NODELAY` is applied to underlying transport - if the property is ``True``. - - Use :meth:`set_tcp_nodelay` to assign new value to the property. - - Default value is ``True``. - - .. method:: set_tcp_nodelay(value) - - Set :attr:`tcp_nodelay` property to *value*. - - Clear :attr:`tcp_cork` if *value* is ``True``. - .. comethod:: prepare(request) :param aiohttp.web.Request request: HTTP request object, that the From 9cc03cd548a188d60e77d08bef2b9d7f606f76bc Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Mon, 18 Dec 2017 17:24:31 +0000 Subject: [PATCH 20/34] allow custom port to TestServer (#2613) * allow custom port to TestServer * tweaks and add CHANGES * add docs * add docs for pytest fixture unused_port * addinv versionadded:: 3.0 --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- CHANGES/2613.feature | 1 + aiohttp/pytest_plugin.py | 8 ++--- aiohttp/test_utils.py | 19 ++++++---- docs/testing.rst | 59 ++++++++++++++++++++++++++------ tests/test_pytest_plugin.py | 21 ++++++++++-- tests/test_test_utils.py | 15 ++++++++ 7 files changed, 101 insertions(+), 24 deletions(-) create mode 100644 CHANGES/2613.feature diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 11c3e068626..6194973277e 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -21,7 +21,7 @@ * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [ ] Add a new news fragment into the `CHANGES` folder - * name it `.` for example (588.bug) + * name it `.` for example (588.bugfix) * if you don't have an `issue_id` change it to the pr id after creating the pr * ensure type is one of the following: * `.feature`: Signifying a new feature. diff --git a/CHANGES/2613.feature b/CHANGES/2613.feature new file mode 100644 index 00000000000..88987e03be3 --- /dev/null +++ b/CHANGES/2613.feature @@ -0,0 +1 @@ +Allow a custom port to be used by `TestServer` (and associated pytest fixtures) diff --git a/aiohttp/pytest_plugin.py b/aiohttp/pytest_plugin.py index c3ebb74ee74..407c9fc9159 100644 --- a/aiohttp/pytest_plugin.py +++ b/aiohttp/pytest_plugin.py @@ -225,8 +225,8 @@ def test_server(loop): """ servers = [] - async def go(app, **kwargs): - server = TestServer(app) + async def go(app, *, port=None, **kwargs): + server = TestServer(app, port=port) await server.start_server(loop=loop, **kwargs) servers.append(server) return server @@ -248,8 +248,8 @@ def raw_test_server(loop): """ servers = [] - async def go(handler, **kwargs): - server = RawTestServer(handler) + async def go(handler, *, port=None, **kwargs): + server = RawTestServer(handler, port=port) await server.start_server(loop=loop, **kwargs) servers.append(server) return server diff --git a/aiohttp/test_utils.py b/aiohttp/test_utils.py index 9c689c333d0..68a67b9e6cf 100644 --- a/aiohttp/test_utils.py +++ b/aiohttp/test_utils.py @@ -39,7 +39,8 @@ def unused_port(): class BaseTestServer(ABC): def __init__(self, *, scheme=sentinel, loop=None, - host='127.0.0.1', skip_url_asserts=False, **kwargs): + host='127.0.0.1', port=None, skip_url_asserts=False, + **kwargs): self._loop = loop self.port = None self.server = None @@ -49,14 +50,18 @@ def __init__(self, *, scheme=sentinel, loop=None, self._closed = False self.scheme = scheme self.skip_url_asserts = skip_url_asserts + self.port = port async def start_server(self, loop=None, **kwargs): if self.server: return self._loop = loop self._socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - self._socket.bind((self.host, 0)) - self.port = self._socket.getsockname()[1] + if self.port: + self._socket.bind((self.host, self.port)) + else: + self._socket.bind((self.host, 0)) + self.port = self._socket.getsockname()[1] self._ssl = kwargs.pop('ssl', None) if self.scheme is sentinel: if self._ssl: @@ -134,9 +139,9 @@ async def __aexit__(self, exc_type, exc_value, traceback): class TestServer(BaseTestServer): def __init__(self, app, *, - scheme=sentinel, host='127.0.0.1', **kwargs): + scheme=sentinel, host='127.0.0.1', port=None, **kwargs): self.app = app - super().__init__(scheme=scheme, host=host, **kwargs) + super().__init__(scheme=scheme, host=host, port=port, **kwargs) async def _make_factory(self, **kwargs): self.app._set_loop(self._loop) @@ -154,9 +159,9 @@ async def _close_hook(self): class RawTestServer(BaseTestServer): def __init__(self, handler, *, - scheme=sentinel, host='127.0.0.1', **kwargs): + scheme=sentinel, host='127.0.0.1', port=None, **kwargs): self._handler = handler - super().__init__(scheme=scheme, host=host, **kwargs) + super().__init__(scheme=scheme, host=host, port=port, **kwargs) async def _make_factory(self, debug=True, **kwargs): self.handler = Server( diff --git a/docs/testing.rst b/docs/testing.rst index 85504879c59..a4e5bdc6e63 100644 --- a/docs/testing.rst +++ b/docs/testing.rst @@ -128,7 +128,7 @@ app test client:: Pytest tooling has the following fixtures: -.. data:: test_server(app, **kwargs) +.. data:: test_server(app, *, port=None, **kwargs) A fixture factory that creates :class:`~aiohttp.test_utils.TestServer`:: @@ -144,11 +144,16 @@ Pytest tooling has the following fixtures: *app* is the :class:`aiohttp.web.Application` used to start server. + *port* optional, port the server is run at, if + not provided a random unused port is used. + + .. versionadded:: 3.0 + *kwargs* are parameters passed to :meth:`aiohttp.web.Application.make_handler` -.. data:: test_client(app, **kwargs) +.. data:: test_client(app, server_kwargs=None, **kwargs) test_client(server, **kwargs) test_client(raw_server, **kwargs) @@ -168,17 +173,17 @@ Pytest tooling has the following fixtures: :class:`aiohttp.test_utils.TestServer` or :class:`aiohttp.test_utils.RawTestServer` instance. + *server_kwargs* are parameters passed to the test server if an app + is passed, else ignored. + *kwargs* are parameters passed to :class:`aiohttp.test_utils.TestClient` constructor. -.. data:: raw_test_server(handler, **kwargs) +.. data:: raw_test_server(handler, *, port=None, **kwargs) A fixture factory that creates :class:`~aiohttp.test_utils.RawTestServer` instance from given web - handler. - - *handler* should be a coroutine which accepts a request and returns - response, e.g.:: + handler.:: async def test_f(raw_test_server, test_client): @@ -189,6 +194,27 @@ Pytest tooling has the following fixtures: client = await test_client(raw_server) resp = await client.get('/') + *handler* should be a coroutine which accepts a request and returns + response, e.g. + + *port* optional, port the server is run at, if + not provided a random unused port is used. + + .. versionadded:: 3.0 + +.. data:: unused_port() + + Function to return an unused port number for IPv4 TCP protocol:: + + async def test_f(test_client, unused_port): + port = unused_port() + app = web.Application() + # fill route table + + client = await test_client(app, server_kwargs={'port': port}) + ... + + .. _aiohttp-testing-unittest-example: .. _aiohttp-testing-unittest-style: @@ -563,7 +589,7 @@ Test server usually works in conjunction with :class:`aiohttp.test_utils.TestClient` which provides handy client methods for accessing to the server. -.. class:: BaseTestServer(*, scheme='http', host='127.0.0.1') +.. class:: BaseTestServer(*, scheme='http', host='127.0.0.1', port=None) Base class for test servers. @@ -572,6 +598,10 @@ for accessing to the server. :param str host: a host for TCP socket, IPv4 *local host* (``'127.0.0.1'``) by default. + :param int port: optional port for TCP socket, if not provided a + random unused port is used. + + .. versionadded:: 3.0 .. attribute:: scheme @@ -584,7 +614,7 @@ for accessing to the server. .. attribute:: port - A random *port* used to start a server. + *port* used to start the test server. .. attribute:: handler @@ -630,6 +660,11 @@ for accessing to the server. :param str host: a host for TCP socket, IPv4 *local host* (``'127.0.0.1'``) by default. + :param int port: optional port for TCP socket, if not provided a + random unused port is used. + + .. versionadded:: 3.0 + .. class:: TestServer(app, *, scheme="http", host='127.0.0.1') @@ -643,6 +678,10 @@ for accessing to the server. :param str host: a host for TCP socket, IPv4 *local host* (``'127.0.0.1'``) by default. + :param int port: optional port for TCP socket, if not provided a + random unused port is used. + + .. versionadded:: 3.0 .. attribute:: app @@ -688,7 +727,7 @@ Test Client .. attribute:: port - A random *port* used to start a server. + *port* used to start the server .. attribute:: server diff --git a/tests/test_pytest_plugin.py b/tests/test_pytest_plugin.py index ccbf09a338d..807148139f5 100644 --- a/tests/test_pytest_plugin.py +++ b/tests/test_pytest_plugin.py @@ -22,7 +22,7 @@ async def hello(request): return web.Response(body=b'Hello, world') -def create_app(loop): +def create_app(loop=None): app = web.Application() app.router.add_route('GET', '/', hello) return app @@ -124,10 +124,27 @@ def make_app(loop): with pytest.raises(RuntimeError): await test_client(make_app) + +async def test_custom_port_test_client(test_client, unused_port): + port = unused_port() + client = await test_client(create_app, server_kwargs={'port': port}) + assert client.port == port + resp = await client.get('/') + assert resp.status == 200 + text = await resp.text() + assert 'Hello, world' in text + + +async def test_custom_port_test_server(test_server, unused_port): + app = create_app() + port = unused_port() + server = await test_server(app, port=port) + assert server.port == port + """) testdir.makeconftest(CONFTEST) result = testdir.runpytest('-p', 'no:sugar', '--loop=pyloop') - result.assert_outcomes(passed=10) + result.assert_outcomes(passed=12) def test_warning_checks(testdir): diff --git a/tests/test_test_utils.py b/tests/test_test_utils.py index bd32e3a03cb..cc8ab73a9b3 100644 --- a/tests/test_test_utils.py +++ b/tests/test_test_utils.py @@ -273,3 +273,18 @@ async def test_client_context_manager_response(method, app, loop): if method != 'head': text = await resp.text() assert "Hello, world" in text + + +async def test_custom_port(loop, app, unused_port): + port = unused_port() + client = _TestClient(_TestServer(app, loop=loop, port=port), loop=loop) + await client.start_server() + + assert client.server.port == port + + resp = await client.get('/') + assert resp.status == 200 + text = await resp.text() + assert _hello_world_str == text + + await client.close() From bd1c61785f813e1802cc809c7a3001de1727f333 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 19 Dec 2017 11:20:27 +0200 Subject: [PATCH 21/34] web_site -> web_runner --- aiohttp/web.py | 6 +++--- aiohttp/{web_site.py => web_runner.py} | 0 tests/{test_web_site.py => test_web_runner.py} | 0 3 files changed, 3 insertions(+), 3 deletions(-) rename aiohttp/{web_site.py => web_runner.py} (100%) rename tests/{test_web_site.py => test_web_runner.py} (100%) diff --git a/aiohttp/web.py b/aiohttp/web.py index f42c83c1958..199319a25aa 100644 --- a/aiohttp/web.py +++ b/aiohttp/web.py @@ -6,7 +6,7 @@ from importlib import import_module from . import (web_exceptions, web_fileresponse, web_middlewares, web_protocol, - web_request, web_response, web_server, web_site, + web_request, web_response, web_runner, web_server, web_urldispatcher, web_ws) from .http import HttpVersion # noqa from .log import access_logger @@ -17,8 +17,8 @@ from .web_protocol import * # noqa from .web_request import * # noqa from .web_response import * # noqa +from .web_runner import AppRunner, GracefulExit, SockSite, TCPSite, UnixSite from .web_server import * # noqa -from .web_site import AppRunner, GracefulExit, SockSite, TCPSite, UnixSite from .web_urldispatcher import * # noqa from .web_ws import * # noqa @@ -31,7 +31,7 @@ web_urldispatcher.__all__ + web_ws.__all__ + web_server.__all__ + - web_site.__all__ + + web_runner.__all__ + web_middlewares.__all__ + ('Application', 'HttpVersion', 'MsgType')) diff --git a/aiohttp/web_site.py b/aiohttp/web_runner.py similarity index 100% rename from aiohttp/web_site.py rename to aiohttp/web_runner.py diff --git a/tests/test_web_site.py b/tests/test_web_runner.py similarity index 100% rename from tests/test_web_site.py rename to tests/test_web_runner.py From 51961a7ebad20af91a8067c2d17d3594edda0ec0 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 19 Dec 2017 12:13:46 +0200 Subject: [PATCH 22/34] Add a link to aiohttp-remotes --- docs/conf.py | 2 ++ docs/web.rst | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index e70b691a538..3224ffbce18 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -73,6 +73,8 @@ ('https://yarl.readthedocs.io/en/stable/', None), 'aiohttpjinja2': ('https://aiohttp-jinja2.readthedocs.io/en/stable/', None), + 'aiohttpremotes': + ('https://aiohttp-remotes.readthedocs.io/en/stable/', None), 'aiohttpsession': ('https://aiohttp-session.readthedocs.io/en/stable/', None)} diff --git a/docs/web.rst b/docs/web.rst index 9d3639c5d58..f9e1d4d971a 100644 --- a/docs/web.rst +++ b/docs/web.rst @@ -1552,8 +1552,11 @@ For changing :attr:`~BaseRequest.scheme` :attr:`~BaseRequest.host` and :attr:`~BaseRequest.remote` the middleware might use :meth:`~BaseRequest.clone`. -TBD: add a link to third-party project with proper middleware -implementation. +.. seealso:: + + https://github.com/aio-libs/aiohttp-remotes provides secure helpers + for modifiying *scheme*, *host* and *remote* attributes according + to ``Forwarded`` and ``X-Forwarded-*`` HTTP headers. Swagger support --------------- From c4efde8309218bec2a4984369bd74ae73ec0d4fa Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 19 Dec 2017 12:30:23 +0200 Subject: [PATCH 23/34] Fix spelling --- docs/web.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/web.rst b/docs/web.rst index f9e1d4d971a..9598ab99e6d 100644 --- a/docs/web.rst +++ b/docs/web.rst @@ -1555,7 +1555,7 @@ For changing :attr:`~BaseRequest.scheme` :attr:`~BaseRequest.host` and .. seealso:: https://github.com/aio-libs/aiohttp-remotes provides secure helpers - for modifiying *scheme*, *host* and *remote* attributes according + for modifying *scheme*, *host* and *remote* attributes according to ``Forwarded`` and ``X-Forwarded-*`` HTTP headers. Swagger support From 19c138c864adb37823c4723901a9b84bd2197085 Mon Sep 17 00:00:00 2001 From: codeif Date: Thu, 21 Dec 2017 14:11:35 +0800 Subject: [PATCH 24/34] web.run_app support access_log_class param (#2616) * Function web.run_app add param access_log_class * Docs of web.run_app add param access_log_class * Add changelog and contributor * make isort * Update docs/web_reference.rst --- CHANGES/2615.feature | 1 + CONTRIBUTORS.txt | 1 + aiohttp/web.py | 10 ++++++---- docs/web_reference.rst | 8 ++++++++ 4 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 CHANGES/2615.feature diff --git a/CHANGES/2615.feature b/CHANGES/2615.feature new file mode 100644 index 00000000000..1afdeea8ce5 --- /dev/null +++ b/CHANGES/2615.feature @@ -0,0 +1 @@ +Add param access_log_class to web.run_app function diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index b57b76924db..f60411588f9 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -202,6 +202,7 @@ Will McGugan Willem de Groot Wilson Ong Wei Lin +Weiwei Wang Yannick Koechlin Yannick Péroux Yegor Roganov diff --git a/aiohttp/web.py b/aiohttp/web.py index 199319a25aa..87bdb89586e 100644 --- a/aiohttp/web.py +++ b/aiohttp/web.py @@ -5,8 +5,8 @@ from collections import Iterable from importlib import import_module -from . import (web_exceptions, web_fileresponse, web_middlewares, web_protocol, - web_request, web_response, web_runner, web_server, +from . import (helpers, web_exceptions, web_fileresponse, web_middlewares, + web_protocol, web_request, web_response, web_runner, web_server, web_urldispatcher, web_ws) from .http import HttpVersion # noqa from .log import access_logger @@ -38,12 +38,14 @@ def run_app(app, *, host=None, port=None, path=None, sock=None, shutdown_timeout=60.0, ssl_context=None, - print=print, backlog=128, access_log_format=None, - access_log=access_logger, handle_signals=True): + print=print, backlog=128, access_log_class=helpers.AccessLogger, + access_log_format=None, access_log=access_logger, + handle_signals=True): """Run an app locally""" loop = asyncio.get_event_loop() runner = AppRunner(app, handle_signals=handle_signals, + access_log_class=access_log_class, access_log_format=access_log_format, access_log=access_log) diff --git a/docs/web_reference.rst b/docs/web_reference.rst index f30a44f438c..1926806f8df 100644 --- a/docs/web_reference.rst +++ b/docs/web_reference.rst @@ -2270,6 +2270,7 @@ Utilities .. function:: run_app(app, *, host=None, port=None, path=None, \ sock=None, shutdown_timeout=60.0, \ ssl_context=None, print=print, backlog=128, \ + access_log_class=aiohttp.helpers.AccessLogger, \ access_log_format=None, \ access_log=aiohttp.log.access_logger, \ handle_signals=True) @@ -2330,6 +2331,10 @@ Utilities system will allow before refusing new connections (``128`` by default). + :param access_log_class: class for `access_logger`. Default: + :data:`aiohttp.helpers.AccessLogger`. + Must to be a subclass of :class:`aiohttp.abc.AbstractAccessLogger`. + :param access_log: :class:`logging.Logger` instance used for saving access logs. Use ``None`` for disabling logs for sake of speedup. @@ -2341,6 +2346,9 @@ Utilities :param bool handle_signals: override signal TERM handling to gracefully exit the application. + .. versionadded:: 3.0 + + Support *access_log_class* parameter. Constants --------- From b513a1e0e068c7f04cd208af5d395d0e3c2efaf8 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 21 Dec 2017 14:04:13 +0200 Subject: [PATCH 25/34] Add source=self for ResourceWarning --- aiohttp/client.py | 5 +++-- aiohttp/client_reqrep.py | 3 ++- aiohttp/connector.py | 6 ++++-- aiohttp/payload.py | 3 ++- aiohttp/web_app.py | 3 ++- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 41ecd3e96e1..8e8e547f530 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -83,7 +83,7 @@ def __init__(self, *, connector=None, loop=None, cookies=None, if implicit_loop and not loop.is_running(): warnings.warn("Creating a client session outside of coroutine is " - "a very dangerous idea", ResourceWarning, + "a very dangerous idea", stacklevel=2) context = {'client_session': self, 'message': 'Creating a client session outside ' @@ -134,7 +134,8 @@ def __init__(self, *, connector=None, loop=None, cookies=None, def __del__(self, _warnings=warnings): if not self.closed: _warnings.warn("Unclosed client session {!r}".format(self), - ResourceWarning) + ResourceWarning, + source=self) context = {'client_session': self, 'message': 'Unclosed client session'} if self._source_traceback is not None: diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 51876cc6b6c..86406dc8d24 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -582,7 +582,8 @@ def __del__(self, _warnings=warnings): if __debug__: if self._loop.get_debug(): _warnings.warn("Unclosed response {!r}".format(self), - ResourceWarning) + ResourceWarning, + source=self) context = {'client_response': self, 'message': 'Unclosed response'} if self._source_traceback: diff --git a/aiohttp/connector.py b/aiohttp/connector.py index 04f988a5ea1..ab1d636c7f9 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -62,7 +62,8 @@ def __repr__(self): def __del__(self, _warnings=warnings): if self._protocol is not None: _warnings.warn('Unclosed connection {!r}'.format(self), - ResourceWarning) + ResourceWarning, + source=self) if self._loop.is_closed(): return @@ -211,7 +212,8 @@ def __del__(self, _warnings=warnings): self.close() _warnings.warn("Unclosed connector {!r}".format(self), - ResourceWarning) + ResourceWarning, + source=self) context = {'connector': self, 'connections': conns, 'message': 'Unclosed connector'} diff --git a/aiohttp/payload.py b/aiohttp/payload.py index 39d9ed20a40..7359440e124 100644 --- a/aiohttp/payload.py +++ b/aiohttp/payload.py @@ -149,7 +149,8 @@ def __init__(self, value, *args, **kwargs): if self._size > TOO_LARGE_BYTES_BODY: warnings.warn("Sending a large body directly with raw bytes might" " lock the event loop. You should probably pass an " - "io.BytesIO object instead", ResourceWarning) + "io.BytesIO object instead", ResourceWarning, + source=self) async def write(self, writer): await writer.write(self._value) diff --git a/aiohttp/web_app.py b/aiohttp/web_app.py index 493109bb8ef..304f0c15168 100644 --- a/aiohttp/web_app.py +++ b/aiohttp/web_app.py @@ -30,7 +30,8 @@ def __init__(self, *, assert isinstance(router, AbstractRouter), router if loop is not None: - warnings.warn("loop argument is deprecated", ResourceWarning) + warnings.warn("loop argument is deprecated", DeprecationWarning, + stacklevel=2) self._debug = debug self._router = router From dc7d0c5e63e8ef3db47d64e487292d17e5433f61 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 21 Dec 2017 14:33:57 +0200 Subject: [PATCH 26/34] Fix tests --- tests/test_client_session.py | 2 +- tests/test_web_app.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_client_session.py b/tests/test_client_session.py index 4fa5c29ea89..6802f5a2dab 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -445,7 +445,7 @@ def test_client_session_implicit_loop_warn(): loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) - with pytest.warns(ResourceWarning): + with pytest.warns(UserWarning): session = aiohttp.ClientSession() assert session._loop is loop loop.run_until_complete(session.close()) diff --git a/tests/test_web_app.py b/tests/test_web_app.py index bf80d8f85ab..3f020cba7bf 100644 --- a/tests/test_web_app.py +++ b/tests/test_web_app.py @@ -9,7 +9,7 @@ def test_app_ctor(loop): - with pytest.warns(ResourceWarning): + with pytest.warns(DeprecationWarning): app = web.Application(loop=loop) assert loop is app.loop assert app.logger is log.web_logger From 6650b353c1238e12c87c9f26d75a6775edb93670 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 21 Dec 2017 14:44:54 +0200 Subject: [PATCH 27/34] Restore compatibility with Python 3.5 --- aiohttp/client.py | 10 +++++++--- aiohttp/client_reqrep.py | 26 ++++++++++++++------------ aiohttp/connector.py | 14 +++++++++++--- aiohttp/helpers.py | 2 ++ aiohttp/payload.py | 8 ++++++-- 5 files changed, 40 insertions(+), 20 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 8e8e547f530..1c722181174 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -24,8 +24,8 @@ from .connector import * # noqa from .connector import TCPConnector from .cookiejar import CookieJar -from .helpers import (CeilTimeout, TimeoutHandle, proxies_from_env, sentinel, - strip_auth_from_url) +from .helpers import (PY_36, CeilTimeout, TimeoutHandle, proxies_from_env, + sentinel, strip_auth_from_url) from .http import WS_KEY, WebSocketReader, WebSocketWriter from .http_websocket import WSHandshakeError, ws_ext_gen, ws_ext_parse from .streams import FlowControlDataQueue @@ -133,9 +133,13 @@ def __init__(self, *, connector=None, loop=None, cookies=None, def __del__(self, _warnings=warnings): if not self.closed: + if PY_36: + kwargs = {'source': self} + else: + kwargs = {} _warnings.warn("Unclosed client session {!r}".format(self), ResourceWarning, - source=self) + **kwargs) context = {'client_session': self, 'message': 'Unclosed client session'} if self._source_traceback is not None: diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 86406dc8d24..7dd0cb09b32 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -20,7 +20,7 @@ ClientResponseError, ContentTypeError, InvalidURL) from .formdata import FormData -from .helpers import HeadersMixin, TimerNoop, noop, reify, set_result +from .helpers import PY_36, HeadersMixin, TimerNoop, noop, reify, set_result from .http import SERVER_SOFTWARE, HttpVersion10, HttpVersion11, PayloadWriter from .log import client_logger from .streams import StreamReader @@ -578,17 +578,19 @@ def __del__(self, _warnings=warnings): self._connection.release() self._cleanup_writer() - # warn - if __debug__: - if self._loop.get_debug(): - _warnings.warn("Unclosed response {!r}".format(self), - ResourceWarning, - source=self) - context = {'client_response': self, - 'message': 'Unclosed response'} - if self._source_traceback: - context['source_traceback'] = self._source_traceback - self._loop.call_exception_handler(context) + if self._loop.get_debug(): + if PY_36: + kwargs = {'source': self} + else: + kwargs = {} + _warnings.warn("Unclosed response {!r}".format(self), + ResourceWarning, + **kwargs) + context = {'client_response': self, + 'message': 'Unclosed response'} + if self._source_traceback: + context['source_traceback'] = self._source_traceback + self._loop.call_exception_handler(context) def __repr__(self): out = io.StringIO() diff --git a/aiohttp/connector.py b/aiohttp/connector.py index ab1d636c7f9..f51641c4046 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -21,7 +21,7 @@ ssl_errors) from .client_proto import ResponseHandler from .client_reqrep import ClientRequest -from .helpers import is_ip_address, noop, sentinel +from .helpers import PY_36, is_ip_address, noop, sentinel from .locks import EventResultOrError from .resolver import DefaultResolver @@ -61,9 +61,13 @@ def __repr__(self): def __del__(self, _warnings=warnings): if self._protocol is not None: + if PY_36: + kwargs = {'source': self} + else: + kwargs = {} _warnings.warn('Unclosed connection {!r}'.format(self), ResourceWarning, - source=self) + **kwargs) if self._loop.is_closed(): return @@ -211,9 +215,13 @@ def __del__(self, _warnings=warnings): self.close() + if PY_36: + kwargs = {'source': self} + else: + kwargs = {} _warnings.warn("Unclosed connector {!r}".format(self), ResourceWarning, - source=self) + **kwargs) context = {'connector': self, 'connections': conns, 'message': 'Unclosed connector'} diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index abfacf293af..23ddeaa0d70 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -10,6 +10,7 @@ import netrc import os import re +import sys import time import weakref from collections import namedtuple @@ -29,6 +30,7 @@ __all__ = ('BasicAuth',) +PY_36 = sys.version_info > (3, 6) sentinel = object() NO_EXTENSIONS = bool(os.environ.get('AIOHTTP_NO_EXTENSIONS')) diff --git a/aiohttp/payload.py b/aiohttp/payload.py index 7359440e124..1a65e1701d4 100644 --- a/aiohttp/payload.py +++ b/aiohttp/payload.py @@ -8,7 +8,7 @@ from multidict import CIMultiDict from . import hdrs -from .helpers import (content_disposition_header, guess_filename, +from .helpers import (PY_36, content_disposition_header, guess_filename, parse_mimetype, sentinel) from .streams import DEFAULT_LIMIT @@ -147,10 +147,14 @@ def __init__(self, value, *args, **kwargs): self._size = len(value) if self._size > TOO_LARGE_BYTES_BODY: + if PY_36: + kwargs = {'source': self} + else: + kwargs = {} warnings.warn("Sending a large body directly with raw bytes might" " lock the event loop. You should probably pass an " "io.BytesIO object instead", ResourceWarning, - source=self) + **kwargs) async def write(self, writer): await writer.write(self._value) From d45a1eedf1eeb7b16305ca6f18fcfc862df15708 Mon Sep 17 00:00:00 2001 From: Al Date: Thu, 21 Dec 2017 11:05:40 -0500 Subject: [PATCH 28/34] fix test case --- tests/test_client_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_client_request.py b/tests/test_client_request.py index fdaef0333d2..20f90097333 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -379,7 +379,7 @@ def test_deregister_non_existing_hook(make_request): def test_custom_auth_non_callable_raises(make_request): with pytest.raises(TypeError): auth = 'no call' - req = make_request('get', 'http://0.0.0.0/', auth=auth) + make_request('get', 'http://0.0.0.0/', auth=auth) def test_hook_registration(make_request): From 3406c8646fd462d67d1f546db385fb56bc03d841 Mon Sep 17 00:00:00 2001 From: Al Date: Tue, 5 Dec 2017 14:42:42 -0500 Subject: [PATCH 29/34] code and unit tests --- aiohttp/client.py | 3 +++ aiohttp/client_reqrep.py | 45 +++++++++++++++++++++++++++++++-- tests/test_client_functional.py | 22 ++++++++++++++++ tests/test_client_request.py | 30 ++++++++++++++++++++++ tests/test_client_session.py | 34 ++++++++++++++++++++++++- 5 files changed, 131 insertions(+), 3 deletions(-) diff --git a/aiohttp/client.py b/aiohttp/client.py index 1c722181174..0984989456b 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -310,6 +310,9 @@ async def _request(self, method, url, *, self._cookie_jar.update_cookies(resp.cookies, resp.url) + # emit event + resp = await req.dispatch_hooks("response") + # redirects if resp.status in ( 301, 302, 303, 307, 308) and allow_redirects: diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 7dd0cb09b32..030b59d8a11 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -7,6 +7,7 @@ import sys import traceback import warnings +import inspect from collections import namedtuple from hashlib import md5, sha1, sha256 from http.cookies import CookieError, Morsel, SimpleCookie @@ -56,7 +57,32 @@ ConnectionKey = namedtuple('ConnectionKey', ['host', 'port', 'ssl']) -class ClientRequest: +class HooksMixin: + def register_hook(self, event, hook): + """Register an event hook.""" + if event not in self._hooks: + raise ValueError('Unsupported event specified, with event name {0}'.format(event)) + + assert inspect.iscoroutinefunction(hook), "hook {0} must be a coroutine".format(getattr(hook, '__name__', '')) + + self._hooks[event].append(hook) + + def deregister_hook(self, event, hook): + """Deregister a previously registered event hook. + Returns True if the hook existed, False if not. + """ + try: + self._hooks[event].remove(hook) + return True + except ValueError: + return False + + async def dispatch_hooks(self, event, **extras): + """implement this method to call hooks""" + raise NotImplementedError() + + +class ClientRequest(HooksMixin): GET_METHODS = { hdrs.METH_GET, hdrs.METH_HEAD, @@ -70,6 +96,7 @@ class ClientRequest: hdrs.ACCEPT: '*/*', hdrs.ACCEPT_ENCODING: 'gzip, deflate', } + EVENTS = ['response'] body = b'' auth = None @@ -123,6 +150,7 @@ def __init__(self, method, url, *, self._auto_decompress = auto_decompress self._verify_ssl = verify_ssl self._ssl_context = ssl_context + self._hooks = dict((event, []) for event in self.EVENTS) if loop.get_debug(): self._source_traceback = traceback.extract_stack(sys._getframe(1)) @@ -279,7 +307,10 @@ def update_transfer_encoding(self): self.headers[hdrs.CONTENT_LENGTH] = str(len(self.body)) def update_auth(self, auth): - """Set basic auth.""" + """Set basic auth. or custom if callable""" + if callable(auth): + auth(self) + return if auth is None: auth = self.auth if auth is None: @@ -488,6 +519,15 @@ def terminate(self): self._writer.cancel() self._writer = None + async def dispatch_hooks(self, event, **extras): + """call and hooks associated to an event""" + """return results which should be a response or None""" + rsp = None + for hook in self._hooks[event]: + rsp = await hook(self, **extras) + + return rsp or self.response + class ClientResponse(HeadersMixin): @@ -824,3 +864,4 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): # for exceptions, response object can closes connection # is state is broken self.release() + diff --git a/tests/test_client_functional.py b/tests/test_client_functional.py index f7892ed766a..f38261c15a1 100644 --- a/tests/test_client_functional.py +++ b/tests/test_client_functional.py @@ -2522,3 +2522,25 @@ async def canceller(): fut2.cancel() await asyncio.gather(fetch1(), fetch2(), canceller(), loop=loop) + + +async def test_custom_auth_using_response_hook(loop, test_client): + + class CustomAuth(): + async def do_custom_auth(self, req, **extras): + return req.response + + def __call__(self, r): + r.register_hook('response', self.do_custom_auth) + + async def handler(request): + return web.Response() + + app = web.Application() + app.router.add_route('GET', '/', handler) + + client = await test_client(app) + resp = await client.get('/', auth=CustomAuth()) + + assert resp.status == 200 + assert resp diff --git a/tests/test_client_request.py b/tests/test_client_request.py index a60d93c31e9..5c04b7f5879 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -358,6 +358,36 @@ def test_basic_auth_from_url_overriden(make_request): assert 'python.org' == req.host +def test_custom_auth_register_hook(make_request): + auth = mock.MagicMock() + req = make_request('get', 'http://0.0.0.0/', auth=auth) + auth.assert_called_once_with(req) + + +def test_hook_registration(make_request): + async def hook(): + await asyncio.sleep(0) + + req = make_request('get', 'http://0.0.0.0/') + req.register_hook('response', hook) + assert hook in req._hooks['response'] + + req.deregister_hook('response', hook) + assert hook not in req._hooks['response'] + + +async def test_response_hook_called(make_request): + async def hook(req): + return req.response + + response = mock.Mock() + req = make_request('get', 'http://0.0.0.0/') + req.register_hook('response', hook) + req.response = response + rsp = await req.dispatch_hooks('response') + assert rsp == response + + def test_path_is_not_double_encoded1(make_request): req = make_request('get', "http://0.0.0.0/get/test case") assert req.url.raw_path == "/get/test%20case" diff --git a/tests/test_client_session.py b/tests/test_client_session.py index 6802f5a2dab..de7c1b6aa7e 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -12,7 +12,7 @@ import aiohttp from aiohttp import hdrs, web from aiohttp.client import ClientSession -from aiohttp.client_reqrep import ClientRequest +from aiohttp.client_reqrep import ClientRequest, ClientResponse from aiohttp.connector import BaseConnector, TCPConnector @@ -560,3 +560,35 @@ async def new_headers( await session.get('http://example.com') assert MyClientRequest.headers['foo'] == 'bar' + + +async def test_dispatch_response_hooks(loop): + + with mock.patch("aiohttp.client.TCPConnector.connect") as connect_patched: + request = mock.Mock(ClientRequest) + request_class = mock.MagicMock(return_value=request) + request_class.return_value = request + response = mock.MagicMock() + request.send.return_value = response + + f = loop.create_future() + f.set_result(None) + response.start.return_value = f + response.status = 200 + + f = loop.create_future() + f.set_result(response) + request.dispatch_hooks.return_value = f + + f = loop.create_future() + f.set_result(mock.MagicMock()) + connect_patched.return_value = f + + session = aiohttp.ClientSession( + loop=loop, + request_class=request_class, + cookie_jar=mock.MagicMock() + ) + await session.get('http://test.example.com') + await session.close() + request.dispatch_hooks.assert_called_with('response') From 9a2955f71491c50a1bfad4410cce9fe39ea63be6 Mon Sep 17 00:00:00 2001 From: Al Date: Wed, 6 Dec 2017 16:34:49 -0500 Subject: [PATCH 30/34] per flake8 recomendations --- aiohttp/client_reqrep.py | 11 +++++++---- tests/test_client_session.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 030b59d8a11..65e172b530d 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -1,13 +1,13 @@ import asyncio import codecs import collections +import inspect import io import json import ssl import sys import traceback import warnings -import inspect from collections import namedtuple from hashlib import md5, sha1, sha256 from http.cookies import CookieError, Morsel, SimpleCookie @@ -61,9 +61,13 @@ class HooksMixin: def register_hook(self, event, hook): """Register an event hook.""" if event not in self._hooks: - raise ValueError('Unsupported event specified, with event name {0}'.format(event)) + raise ValueError( + "Unsupported event specified, " + "with event name {0}".format(event)) - assert inspect.iscoroutinefunction(hook), "hook {0} must be a coroutine".format(getattr(hook, '__name__', '')) + assert inspect.iscoroutinefunction(hook), \ + "hook {0} must be a coroutine".format( + getattr(hook, '__name__', '')) self._hooks[event].append(hook) @@ -864,4 +868,3 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): # for exceptions, response object can closes connection # is state is broken self.release() - diff --git a/tests/test_client_session.py b/tests/test_client_session.py index de7c1b6aa7e..b64554518df 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -12,7 +12,7 @@ import aiohttp from aiohttp import hdrs, web from aiohttp.client import ClientSession -from aiohttp.client_reqrep import ClientRequest, ClientResponse +from aiohttp.client_reqrep import ClientRequest from aiohttp.connector import BaseConnector, TCPConnector From 7b9e56f79cbeb4944874f618b0c19902dece66ea Mon Sep 17 00:00:00 2001 From: Al Date: Tue, 12 Dec 2017 08:39:57 -0500 Subject: [PATCH 31/34] - Update docs - retrofit BasicAuth --- CHANGES/434.feature | 1 + aiohttp/client_reqrep.py | 10 +++------- aiohttp/helpers.py | 3 +++ docs/client_reference.rst | 9 +++++++-- 4 files changed, 14 insertions(+), 9 deletions(-) create mode 100644 CHANGES/434.feature diff --git a/CHANGES/434.feature b/CHANGES/434.feature new file mode 100644 index 00000000000..09b3a4647cf --- /dev/null +++ b/CHANGES/434.feature @@ -0,0 +1 @@ +Allow Custom Authentication handlers to be passed into ClientSession \ No newline at end of file diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 65e172b530d..10e89bfc129 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -312,18 +312,14 @@ def update_transfer_encoding(self): def update_auth(self, auth): """Set basic auth. or custom if callable""" - if callable(auth): - auth(self) - return if auth is None: auth = self.auth if auth is None: return - if not isinstance(auth, helpers.BasicAuth): - raise TypeError('BasicAuth() tuple is required instead') - - self.headers[hdrs.AUTHORIZATION] = auth.encode() + if not callable(auth): + raise TypeError('Auth must be a callable') + auth(self) def update_body_from_data(self, body): if not body: diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 23ddeaa0d70..ebff4d607ac 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -105,6 +105,9 @@ def encode(self): creds = ('%s:%s' % (self.login, self.password)).encode(self.encoding) return 'Basic %s' % base64.b64encode(creds).decode(self.encoding) + def __call__(self, r): + r.headers[hdrs.AUTHORIZATION] = self.encode() + def strip_auth_from_url(url): auth = BasicAuth.from_url(url) diff --git a/docs/client_reference.rst b/docs/client_reference.rst index 9d9b45424f4..5b5f519e8b5 100644 --- a/docs/client_reference.rst +++ b/docs/client_reference.rst @@ -86,8 +86,9 @@ The client session supports the context manager protocol for self closing. Iterable of :class:`str` or :class:`~aiohttp.istr` (optional) - :param aiohttp.BasicAuth auth: an object that represents HTTP Basic - Authorization (optional) + :param auth: a callable that represents an authentication scheme. + the object will be called passing in the current request object + during the initialization of a request (optional) :param version: supported HTTP version, ``HTTP 1.1`` by default. @@ -1430,6 +1431,10 @@ BasicAuth :return: encoded authentication data, :class:`str`. + .. method:: __call__() + + Used to attach the ``Authorization`` header on the request + CookieJar ^^^^^^^^^ From 7f7ede0a8720dd6bd4fc621858881d6dd369b858 Mon Sep 17 00:00:00 2001 From: Al Date: Tue, 12 Dec 2017 17:38:23 -0500 Subject: [PATCH 32/34] - add myself to contributors.txt --- CONTRIBUTORS.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index f60411588f9..5533a63d02d 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -212,3 +212,4 @@ Yury Selivanov Yusuke Tsutsumi Марк Коренберг Семён Марьясин +Alberto Zuniga \ No newline at end of file From 9fde198a9e54aa8fa52f487511465154495b3894 Mon Sep 17 00:00:00 2001 From: Al Date: Wed, 13 Dec 2017 09:10:31 -0500 Subject: [PATCH 33/34] increase coverage --- tests/test_client_request.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_client_request.py b/tests/test_client_request.py index 5c04b7f5879..fdaef0333d2 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -364,6 +364,24 @@ def test_custom_auth_register_hook(make_request): auth.assert_called_once_with(req) +def test_unsupportd_hook_raises(make_request): + with pytest.raises(ValueError): + req = make_request('get', 'http://0.0.0.0/') + req.register_hook('request', mock.MagicMock()) + + +def test_deregister_non_existing_hook(make_request): + req = make_request('get', 'http://0.0.0.0/') + result = req.deregister_hook('response', mock.MagicMock()) + assert result is False + + +def test_custom_auth_non_callable_raises(make_request): + with pytest.raises(TypeError): + auth = 'no call' + req = make_request('get', 'http://0.0.0.0/', auth=auth) + + def test_hook_registration(make_request): async def hook(): await asyncio.sleep(0) From f1fec4409108adbc489f24940029f5a37aef97d6 Mon Sep 17 00:00:00 2001 From: Al Date: Thu, 21 Dec 2017 11:05:40 -0500 Subject: [PATCH 34/34] fix test case --- tests/test_client_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_client_request.py b/tests/test_client_request.py index fdaef0333d2..20f90097333 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -379,7 +379,7 @@ def test_deregister_non_existing_hook(make_request): def test_custom_auth_non_callable_raises(make_request): with pytest.raises(TypeError): auth = 'no call' - req = make_request('get', 'http://0.0.0.0/', auth=auth) + make_request('get', 'http://0.0.0.0/', auth=auth) def test_hook_registration(make_request):