From c2191153cf856da85fcc5f6bc1ce9b15a233ac39 Mon Sep 17 00:00:00 2001 From: Raphael Deem Date: Mon, 23 Oct 2017 21:01:44 -0700 Subject: [PATCH 01/13] remove port from ip --- docs/sanic/request_data.md | 4 ++++ sanic/request.py | 25 +++++++++++++++++++++---- tests/test_requests.py | 10 ++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/docs/sanic/request_data.md b/docs/sanic/request_data.md index e778faf6..4f6bc970 100644 --- a/docs/sanic/request_data.md +++ b/docs/sanic/request_data.md @@ -75,6 +75,10 @@ The following variables are accessible as properties on `Request` objects: - `ip` (str) - IP address of the requester. +- `port` (str) - Port address of the requester. + +- `socket` (tuple) - (IP, port) of the requester. + - `app` - a reference to the Sanic application object that is handling this request. This is useful when inside blueprints or other handlers in modules that do not have access to the global `app` object. ```python diff --git a/sanic/request.py b/sanic/request.py index 26f19baf..d4f6dc6f 100644 --- a/sanic/request.py +++ b/sanic/request.py @@ -46,7 +46,8 @@ class Request(dict): __slots__ = ( 'app', 'headers', 'version', 'method', '_cookies', 'transport', 'body', 'parsed_json', 'parsed_args', 'parsed_form', 'parsed_files', - '_ip', '_parsed_url', 'uri_template', 'stream', '_remote_addr' + '_ip', '_parsed_url', 'uri_template', 'stream', '_remote_addr', + '_socket', '_port' ) def __init__(self, url_bytes, headers, version, method, transport): @@ -167,11 +168,27 @@ class Request(dict): @property def ip(self): - if not hasattr(self, '_ip'): - self._ip = (self.transport.get_extra_info('peername') or - (None, None)) + if not hasattr(self, '_socket'): + self._get_address() return self._ip + @property + def port(self): + if not hasattr(self, '_socket'): + self._get_address() + return self._port + + @property + def socket(self): + if not hasattr(self, '_socket'): + self._get_socket() + return self._socket + + def _get_address(self): + self._socket = (self.transport.get_extra_info('peername') or + (None, None)) + self._ip, self._port = self._socket + @property def remote_addr(self): """Attempt to return the original client ip based on X-Forwarded-For. diff --git a/tests/test_requests.py b/tests/test_requests.py index f0696c7f..e47520c4 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -27,6 +27,16 @@ def test_sync(): assert response.text == 'Hello' +def test_remote_address(): + app = Sanic('test_text') + + @app.route('/') + def handler(request): + return text("{}".format(request.ip)) + + request, response = app.test_client.get('/') + + assert response.text == '127.0.0.1' def test_text(): app = Sanic('test_text') From 5bf722c7ae1eab214cc54db2d48b3d47b89ec3de Mon Sep 17 00:00:00 2001 From: Raphael Deem Date: Wed, 25 Oct 2017 21:58:31 -0700 Subject: [PATCH 02/13] remove bare exceptions --- sanic/app.py | 4 ++-- sanic/response.py | 2 +- sanic/server.py | 2 +- sanic/testing.py | 4 ++-- sanic/worker.py | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 8f70b6e7..0f2141c2 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -574,7 +574,7 @@ class Sanic: try: response = await self._run_response_middleware(request, response) - except: + except BaseException: error_logger.exception( 'Exception occured in one of response middleware handlers' ) @@ -642,7 +642,7 @@ class Sanic: serve(**server_settings) else: serve_multiple(server_settings, workers) - except: + except BaseException: error_logger.exception( 'Experienced exception while trying to serve') raise diff --git a/sanic/response.py b/sanic/response.py index 86f50038..3873d90f 100644 --- a/sanic/response.py +++ b/sanic/response.py @@ -3,7 +3,7 @@ from os import path try: from ujson import dumps as json_dumps -except: +except BaseException: from json import dumps as json_dumps from aiofiles import open as open_async diff --git a/sanic/server.py b/sanic/server.py index 049440dd..a475fd98 100644 --- a/sanic/server.py +++ b/sanic/server.py @@ -588,7 +588,7 @@ def serve(host, port, request_handler, error_handler, before_start=None, try: http_server = loop.run_until_complete(server_coroutine) - except: + except BaseException: logger.exception("Unable to start server") return diff --git a/sanic/testing.py b/sanic/testing.py index 5d233d7b..ef54537b 100644 --- a/sanic/testing.py +++ b/sanic/testing.py @@ -76,14 +76,14 @@ class SanicTestClient: try: request, response = results return request, response - except: + except BaseException: raise ValueError( "Request and response object expected, got ({})".format( results)) else: try: return results[-1] - except: + except BaseException: raise ValueError( "Request object expected, got ({})".format(results)) diff --git a/sanic/worker.py b/sanic/worker.py index a102fb72..79c0a17d 100644 --- a/sanic/worker.py +++ b/sanic/worker.py @@ -74,13 +74,13 @@ class GunicornWorker(base.Worker): trigger_events(self._server_settings.get('before_stop', []), self.loop) self.loop.run_until_complete(self.close()) - except: + except BaseException: traceback.print_exc() finally: try: trigger_events(self._server_settings.get('after_stop', []), self.loop) - except: + except BaseException: traceback.print_exc() finally: self.loop.close() From a9c7d95e9bcc2273136eff238c2f4a6d4298b8db Mon Sep 17 00:00:00 2001 From: Igor Gnatenko Date: Tue, 31 Oct 2017 09:39:09 +0100 Subject: [PATCH 03/13] tests: do not assume that locahost == 127.0.0.1 Signed-off-by: Igor Gnatenko --- tests/test_url_building.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_url_building.py b/tests/test_url_building.py index fe31f658..f3ed534f 100644 --- a/tests/test_url_building.py +++ b/tests/test_url_building.py @@ -5,7 +5,7 @@ from sanic import Sanic from sanic.response import text from sanic.views import HTTPMethodView from sanic.blueprints import Blueprint -from sanic.testing import PORT as test_port +from sanic.testing import PORT as test_port, HOST as test_host from sanic.exceptions import URLBuildError import string @@ -15,11 +15,11 @@ URL_FOR_VALUE1 = '/myurl?arg1=v1&arg1=v2' URL_FOR_ARGS2 = dict(arg1=['v1', 'v2'], _anchor='anchor') URL_FOR_VALUE2 = '/myurl?arg1=v1&arg1=v2#anchor' URL_FOR_ARGS3 = dict(arg1='v1', _anchor='anchor', _scheme='http', - _server='localhost:{}'.format(test_port), _external=True) -URL_FOR_VALUE3 = 'http://localhost:{}/myurl?arg1=v1#anchor'.format(test_port) + _server='{}:{}'.format(test_host, test_port), _external=True) +URL_FOR_VALUE3 = 'http://{}:{}/myurl?arg1=v1#anchor'.format(test_host, test_port) URL_FOR_ARGS4 = dict(arg1='v1', _anchor='anchor', _external=True, - _server='http://localhost:{}'.format(test_port),) -URL_FOR_VALUE4 = 'http://localhost:{}/myurl?arg1=v1#anchor'.format(test_port) + _server='http://{}:{}'.format(test_host, test_port),) +URL_FOR_VALUE4 = 'http://{}:{}/myurl?arg1=v1#anchor'.format(test_host, test_port) def _generate_handlers_from_names(app, l): From ca596c8ecd11dbbcc53f74f03cc3729aea2c09f5 Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Thu, 2 Nov 2017 15:44:36 +0100 Subject: [PATCH 04/13] Add strict_slashes to {Sanic, Blueprint}().static() --- sanic/app.py | 5 +++-- sanic/blueprints.py | 6 +++++- sanic/static.py | 5 +++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 0f2141c2..121ea306 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -345,13 +345,14 @@ class Sanic: # Static Files def static(self, uri, file_or_directory, pattern=r'/?.+', use_modified_since=True, use_content_range=False, - stream_large_files=False, name='static', host=None): + stream_large_files=False, name='static', host=None, + strict_slashes=None): """Register a root to serve files from. The input can either be a file or a directory. See """ static_register(self, uri, file_or_directory, pattern, use_modified_since, use_content_range, - stream_large_files, name, host) + stream_large_files, name, host, strict_slashes) def blueprint(self, blueprint, **options): """Register a blueprint on the application. diff --git a/sanic/blueprints.py b/sanic/blueprints.py index e8018326..28329abe 100644 --- a/sanic/blueprints.py +++ b/sanic/blueprints.py @@ -221,8 +221,12 @@ class Blueprint: name = kwargs.pop('name', 'static') if not name.startswith(self.name + '.'): name = '{}.{}'.format(self.name, name) - kwargs.update(name=name) + + strict_slashes = kwargs.pop('strict_slashes', None) + if strict_slashes is None and self.strict_slashes is not None: + kwargs.update(strict_slashes=self.strict_slashes) + static = FutureStatic(uri, file_or_directory, args, kwargs) self.statics.append(static) diff --git a/sanic/static.py b/sanic/static.py index 1ebd7291..46649fd8 100644 --- a/sanic/static.py +++ b/sanic/static.py @@ -18,7 +18,7 @@ from sanic.response import file, file_stream, HTTPResponse def register(app, uri, file_or_directory, pattern, use_modified_since, use_content_range, - stream_large_files, name='static', host=None): + stream_large_files, name='static', host=None, strict_slashes=None): # TODO: Though sanic is not a file server, I feel like we should at least # make a good effort here. Modified-since is nice, but we could # also look into etags, expires, and caching @@ -122,4 +122,5 @@ def register(app, uri, file_or_directory, pattern, if not name.startswith('_static_'): name = '_static_{}'.format(name) - app.route(uri, methods=['GET', 'HEAD'], name=name, host=host)(_handler) + app.route(uri, methods=['GET', 'HEAD'], name=name, host=host, + strict_slashes=strict_slashes)(_handler) From ff5786d61bcefd4f3f09bcc5ebcc7a3387d713c8 Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Fri, 3 Nov 2017 14:33:24 +0100 Subject: [PATCH 05/13] pep8 --- sanic/static.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sanic/static.py b/sanic/static.py index 46649fd8..0798e2fe 100644 --- a/sanic/static.py +++ b/sanic/static.py @@ -18,7 +18,8 @@ from sanic.response import file, file_stream, HTTPResponse def register(app, uri, file_or_directory, pattern, use_modified_since, use_content_range, - stream_large_files, name='static', host=None, strict_slashes=None): + stream_large_files, name='static', host=None, + strict_slashes=None): # TODO: Though sanic is not a file server, I feel like we should at least # make a good effort here. Modified-since is nice, but we could # also look into etags, expires, and caching @@ -103,7 +104,7 @@ def register(app, uri, file_or_directory, pattern, if isinstance(stream_large_files, int): threshold = stream_large_files else: - threshold = 1024*1000 + threshold = 1024 * 1000 if not stats: stats = await stat(file_path) From f128ed5b1fc1472e5939a78099ebecd892deef1f Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Fri, 3 Nov 2017 14:37:01 +0100 Subject: [PATCH 06/13] Set threshold to 1MiB instead of 0.97MiB Reference: https://en.wikipedia.org/wiki/Mebibyte#Definition --- sanic/static.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sanic/static.py b/sanic/static.py index 0798e2fe..f2d02ab0 100644 --- a/sanic/static.py +++ b/sanic/static.py @@ -104,7 +104,7 @@ def register(app, uri, file_or_directory, pattern, if isinstance(stream_large_files, int): threshold = stream_large_files else: - threshold = 1024 * 1000 + threshold = 1024 * 1024 if not stats: stats = await stat(file_path) From bb8e9c64387a0e1287565c23e41f0ab7f5a1dabf Mon Sep 17 00:00:00 2001 From: Raphael Deem Date: Fri, 3 Nov 2017 18:30:01 -0700 Subject: [PATCH 07/13] check if method is added in strict slash logic --- sanic/router.py | 9 ++++++++- tests/test_routes.py | 21 ++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/sanic/router.py b/sanic/router.py index 21c98766..b601622c 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -130,8 +130,15 @@ class Router: return # Add versions with and without trailing / + slashed_methods = self.routes_all.get(uri + '/', frozenset({})) + if isinstance(methods, Iterable): + _slash_is_missing = all(method in slashed_methods for + method in methods) + else: + _slash_is_missing = methods in slashed_methods + slash_is_missing = ( - not uri[-1] == '/' and not self.routes_all.get(uri + '/', False) + not uri[-1] == '/' and not _slash_is_missing ) without_slash_is_missing = ( uri[-1] == '/' and not diff --git a/tests/test_routes.py b/tests/test_routes.py index b4ed7cf3..b6b62283 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -44,6 +44,24 @@ def test_shorthand_routes_get(): request, response = app.test_client.post('/get') assert response.status == 405 +def test_shorthand_routes_multiple(): + app = Sanic('test_shorthand_routes_multiple') + + @app.get('/get') + def get_handler(request): + return text('OK') + + @app.options('/get') + def options_handler(request): + return text('') + + request, response = app.test_client.get('/get/') + assert response.status == 200 + assert response.text == 'OK' + + request, response = app.test_client.options('/get/') + assert response.status == 200 + def test_route_strict_slash(): app = Sanic('test_route_strict_slash') @@ -431,7 +449,7 @@ def test_websocket_route_with_subprotocols(): 'Sec-WebSocket-Key': 'dGhlIHNhbXBsZSBub25jZQ==', 'Sec-WebSocket-Version': '13'}) assert response.status == 101 - + assert results == ['bar', 'bar', None, None] @@ -754,6 +772,7 @@ def test_remove_route_without_clean_cache(): assert response.status == 200 app.remove_route('/test', clean_cache=True) + app.remove_route('/test/', clean_cache=True) request, response = app.test_client.get('/test') assert response.status == 404 From bca1e084116335fd939c2ee226070f0428cd5de8 Mon Sep 17 00:00:00 2001 From: Luke Hodkinson Date: Sat, 4 Nov 2017 22:04:59 +1100 Subject: [PATCH 08/13] Call connection_open after websocket handshake It seems that due to (recent?) changes in the websocket library, we now need to call "connection_open" to flag that the websocket is now ready to use. I've added that call just after the call to "connection_made". --- sanic/websocket.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sanic/websocket.py b/sanic/websocket.py index 37b13f3c..bc78c76f 100644 --- a/sanic/websocket.py +++ b/sanic/websocket.py @@ -90,4 +90,5 @@ class WebSocketProtocol(HttpProtocol): ) self.websocket.subprotocol = subprotocol self.websocket.connection_made(request.transport) + self.websocket.connection_open() return self.websocket From 49b1d667f1e2ada51674ea2aaf697e7beb5bc8b4 Mon Sep 17 00:00:00 2001 From: Gaetan Semet Date: Thu, 2 Nov 2017 14:31:48 +0100 Subject: [PATCH 09/13] Optionalize app.run dictConfig (fix #1000) Signed-off-by: Gaetan Semet --- sanic/app.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 0f2141c2..91179af6 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -28,7 +28,8 @@ class Sanic: def __init__(self, name=None, router=None, error_handler=None, load_env=True, request_class=None, - strict_slashes=False, log_config=None): + strict_slashes=False, log_config=None, + configure_logging=True): # Get name from previous stack frame if name is None: @@ -36,7 +37,8 @@ class Sanic: name = getmodulename(frame_records[1]) # logging - logging.config.dictConfig(log_config or LOGGING_CONFIG_DEFAULTS) + if configure_logging: + logging.config.dictConfig(log_config or LOGGING_CONFIG_DEFAULTS) self.name = name self.router = router or Router() @@ -47,6 +49,7 @@ class Sanic: self.response_middleware = deque() self.blueprints = {} self._blueprint_order = [] + self.configure_logging = configure_logging self.debug = None self.sock = None self.strict_slashes = strict_slashes @@ -793,7 +796,7 @@ class Sanic: listeners = [partial(listener, self) for listener in listeners] server_settings[settings_name] = listeners - if debug: + if self.configure_logging and debug: logger.setLevel(logging.DEBUG) if self.config.LOGO is not None: logger.debug(self.config.LOGO) From ed8725bf6c6e9392b9344da59e077c16c3123d02 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 6 Nov 2017 17:26:11 +0900 Subject: [PATCH 10/13] Let SanicTestClient has its own port For parallel test running, the servers must have different ports. See examples/pytest_xdist.py for example. --- examples/pytest_xdist.py | 49 ++++++++++++++++++++++++++++++++++++++++ sanic/testing.py | 7 +++--- 2 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 examples/pytest_xdist.py diff --git a/examples/pytest_xdist.py b/examples/pytest_xdist.py new file mode 100644 index 00000000..06730016 --- /dev/null +++ b/examples/pytest_xdist.py @@ -0,0 +1,49 @@ +"""pytest-xdist example for sanic server + +Install testing tools: + + $ pip install pytest pytest-xdist + +Run with xdist params: + + $ pytest examples/pytest_xdist.py -n 8 # 8 workers +""" +import re +from sanic import Sanic +from sanic.response import text +from sanic.testing import PORT as PORT_BASE, SanicTestClient +import pytest + + +@pytest.fixture(scope="session") +def test_port(worker_id): + m = re.search(r'[0-9]+', worker_id) + if m: + num_id = m.group(0) + else: + num_id = 0 + port = PORT_BASE + int(num_id) + return port + + +@pytest.fixture(scope="session") +def app(): + app = Sanic() + + @app.route('/') + async def index(request): + return text('OK') + + return app + + +@pytest.fixture(scope="session") +def client(app, test_port): + return SanicTestClient(app, test_port) + + +@pytest.mark.parametrize('run_id', range(100)) +def test_index(client, run_id): + request, response = client._sanic_endpoint_test('get', '/') + assert response.status == 200 + assert response.text == 'OK' diff --git a/sanic/testing.py b/sanic/testing.py index ef54537b..873e43f7 100644 --- a/sanic/testing.py +++ b/sanic/testing.py @@ -8,8 +8,9 @@ PORT = 42101 class SanicTestClient: - def __init__(self, app): + def __init__(self, app, port=PORT): self.app = app + self.port = port async def _local_request(self, method, uri, cookies=None, *args, **kwargs): import aiohttp @@ -17,7 +18,7 @@ class SanicTestClient: url = uri else: url = 'http://{host}:{port}{uri}'.format( - host=HOST, port=PORT, uri=uri) + host=HOST, port=self.port, uri=uri) logger.info(url) conn = aiohttp.TCPConnector(verify_ssl=False) @@ -66,7 +67,7 @@ class SanicTestClient: exceptions.append(e) self.app.stop() - self.app.run(host=HOST, debug=debug, port=PORT, **server_kwargs) + self.app.run(host=HOST, debug=debug, port=self.port, **server_kwargs) self.app.listeners['after_server_start'].pop() if exceptions: From e70535e8d70b7965a3ac3c2a149e61b6961d5e76 Mon Sep 17 00:00:00 2001 From: Joar Wandborg Date: Tue, 7 Nov 2017 10:34:17 +0100 Subject: [PATCH 11/13] Use .get instead of .pop --- sanic/blueprints.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sanic/blueprints.py b/sanic/blueprints.py index 28329abe..f9159168 100644 --- a/sanic/blueprints.py +++ b/sanic/blueprints.py @@ -223,7 +223,7 @@ class Blueprint: name = '{}.{}'.format(self.name, name) kwargs.update(name=name) - strict_slashes = kwargs.pop('strict_slashes', None) + strict_slashes = kwargs.get('strict_slashes') if strict_slashes is None and self.strict_slashes is not None: kwargs.update(strict_slashes=self.strict_slashes) From c9876a6c88e94c422659b6b2ae2eb49d1c37491e Mon Sep 17 00:00:00 2001 From: Yaser Amiri Date: Wed, 8 Nov 2017 14:14:57 +0330 Subject: [PATCH 12/13] Change unit tests names with repeated names. --- tests/test_exceptions.py | 6 +++--- tests/test_static.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index c535059c..16d08459 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -58,11 +58,11 @@ def exception_app(): raise InvalidUsage("OK") @app.route('/abort/401') - def handler_invalid(request): + def handler_401_error(request): abort(401) @app.route('/abort') - def handler_invalid(request): + def handler_500_error(request): abort(500) return text("OK") @@ -186,7 +186,7 @@ def test_exception_in_exception_handler_debug_off(exception_app): assert response.body == b'An error occurred while handling an error' -def test_exception_in_exception_handler_debug_off(exception_app): +def test_exception_in_exception_handler_debug_on(exception_app): """Test that an exception thrown in an error handler is handled""" request, response = exception_app.test_client.get( '/error_in_error_handler_handler', diff --git a/tests/test_static.py b/tests/test_static.py index 6252b1c1..276001cc 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -164,7 +164,7 @@ def test_static_content_range_error(file_name, static_file_directory): @pytest.mark.parametrize('file_name', ['test.file', 'decode me.txt', 'python.png']) -def test_static_file(static_file_directory, file_name): +def test_static_file_specified_host(static_file_directory, file_name): app = Sanic('test_static') app.static( '/testing.file', From cfc75b4f1a979886f6966905b7b414fe7364297c Mon Sep 17 00:00:00 2001 From: Edward Betts Date: Wed, 15 Nov 2017 15:46:39 +0000 Subject: [PATCH 13/13] Correct spelling mistakes. --- sanic/app.py | 2 +- tests/test_middleware.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index ce8857a6..05d99c08 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -580,7 +580,7 @@ class Sanic: response) except BaseException: error_logger.exception( - 'Exception occured in one of response middleware handlers' + 'Exception occurred in one of response middleware handlers' ) # pass the response to the correct callback diff --git a/tests/test_middleware.py b/tests/test_middleware.py index bc1a7eb8..4d4d6901 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -59,7 +59,7 @@ def test_middleware_response_exception(): result = {'status_code': None} @app.middleware('response') - async def process_response(reqest, response): + async def process_response(request, response): result['status_code'] = response.status return response