From 9581d21189eb69712b20fbf9904c13a8cf95862a Mon Sep 17 00:00:00 2001 From: LTMenezes Date: Sun, 3 Feb 2019 23:06:21 +0000 Subject: [PATCH 1/4] Enforce Datetime Type for Expires on Set-Cookie --- sanic/cookies.py | 18 +++++++++--------- tests/test_cookies.py | 13 ++++++++++++- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/sanic/cookies.py b/sanic/cookies.py index c53156ee74..06cb479179 100644 --- a/sanic/cookies.py +++ b/sanic/cookies.py @@ -1,5 +1,6 @@ import re import string +import datetime DEFAULT_MAX_AGE = 0 @@ -108,6 +109,8 @@ def __setitem__(self, key, value): if key.lower() == "max-age": if not str(value).isdigit(): value = DEFAULT_MAX_AGE + elif key.lower() == "expires" and type(value) is not datetime.datetime: + raise KeyError("Cookie 'expires' property must be a datetime instance") return super().__setitem__(key, value) def encode(self, encoding): @@ -131,16 +134,13 @@ def encode(self, encoding): except TypeError: output.append("%s=%s" % (self._keys[key], value)) elif key == "expires": - try: - output.append( - "%s=%s" - % ( - self._keys[key], - value.strftime("%a, %d-%b-%Y %T GMT"), - ) + output.append( + "%s=%s" + % ( + self._keys[key], + value.strftime("%a, %d-%b-%Y %T GMT"), ) - except AttributeError: - output.append("%s=%s" % (self._keys[key], value)) + ) elif key in self._flags and self[key]: output.append(self._keys[key]) else: diff --git a/tests/test_cookies.py b/tests/test_cookies.py index 9ad635d210..fbc660fb74 100644 --- a/tests/test_cookies.py +++ b/tests/test_cookies.py @@ -162,7 +162,7 @@ def handler(request): @pytest.mark.parametrize( "expires", - [datetime.now() + timedelta(seconds=60), "Fri, 21-Dec-2018 15:30:00 GMT"], + [datetime.now() + timedelta(seconds=60)], ) def test_cookie_expires(app, expires): cookies = {"test": "wait"} @@ -183,3 +183,14 @@ def handler(request): expires = expires.strftime("%a, %d-%b-%Y %T GMT") assert response.cookies["test"]["expires"] == expires + + +@pytest.mark.parametrize( + "expires", + ["Fri, 21-Dec-2018 15:30:00 GMT"], +) +def test_cookie_expires_illegal_instance_type(expires): + c = Cookie("test_cookie", "value") + with pytest.raises(expected_exception=KeyError) as e: + c["expires"] = expires + assert e.message == "Cookie 'expires' property must be a datetime instance" From e6e1828c9437559cce6c068773b850cc685f12e2 Mon Sep 17 00:00:00 2001 From: LTMenezes Date: Sun, 3 Feb 2019 23:25:07 +0000 Subject: [PATCH 2/4] Fix lint issues --- sanic/cookies.py | 7 +++++-- tests/test_cookies.py | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/sanic/cookies.py b/sanic/cookies.py index 06cb479179..6aabc06376 100644 --- a/sanic/cookies.py +++ b/sanic/cookies.py @@ -109,8 +109,11 @@ def __setitem__(self, key, value): if key.lower() == "max-age": if not str(value).isdigit(): value = DEFAULT_MAX_AGE - elif key.lower() == "expires" and type(value) is not datetime.datetime: - raise KeyError("Cookie 'expires' property must be a datetime instance") + elif key.lower() == "expires": + if type(value) is not datetime.datetime: + raise KeyError( + "Cookie 'expires' property must be a datetime" + ) return super().__setitem__(key, value) def encode(self, encoding): diff --git a/tests/test_cookies.py b/tests/test_cookies.py index fbc660fb74..11af714516 100644 --- a/tests/test_cookies.py +++ b/tests/test_cookies.py @@ -193,4 +193,4 @@ def test_cookie_expires_illegal_instance_type(expires): c = Cookie("test_cookie", "value") with pytest.raises(expected_exception=KeyError) as e: c["expires"] = expires - assert e.message == "Cookie 'expires' property must be a datetime instance" + assert e.message == "Cookie 'expires' property must be a datetime" From 597f3ad494ede9f2a1c2ef803d7d1055edcd56b6 Mon Sep 17 00:00:00 2001 From: LTMenezes Date: Wed, 6 Feb 2019 00:15:36 -0200 Subject: [PATCH 3/4] Format code and improve error type --- sanic/cookies.py | 11 ++++------- tests/test_app.py | 19 ++++++++++--------- tests/test_config.py | 24 +++++++++++++----------- tests/test_cookies.py | 12 +++--------- tests/test_keep_alive_timeout.py | 12 ++++-------- tests/test_logo.py | 12 ++++-------- tests/test_request_timeout.py | 4 +--- tests/test_server_events.py | 3 +-- tests/test_worker.py | 31 +++++++++++++++++-------------- 9 files changed, 57 insertions(+), 71 deletions(-) diff --git a/sanic/cookies.py b/sanic/cookies.py index 6aabc06376..d7337655ab 100644 --- a/sanic/cookies.py +++ b/sanic/cookies.py @@ -1,6 +1,6 @@ import re import string -import datetime +from datetime import datetime DEFAULT_MAX_AGE = 0 @@ -110,8 +110,8 @@ def __setitem__(self, key, value): if not str(value).isdigit(): value = DEFAULT_MAX_AGE elif key.lower() == "expires": - if type(value) is not datetime.datetime: - raise KeyError( + if not isinstance(value, datetime): + raise TypeError( "Cookie 'expires' property must be a datetime" ) return super().__setitem__(key, value) @@ -139,10 +139,7 @@ def encode(self, encoding): elif key == "expires": output.append( "%s=%s" - % ( - self._keys[key], - value.strftime("%a, %d-%b-%Y %T GMT"), - ) + % (self._keys[key], value.strftime("%a, %d-%b-%Y %T GMT")) ) elif key in self._flags and self[key]: output.append(self._keys[key]) diff --git a/tests/test_app.py b/tests/test_app.py index ba21238c4c..1003d727fa 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -12,6 +12,7 @@ def uvloop_installed(): try: import uvloop + return True except ImportError: return False @@ -27,28 +28,28 @@ async def handler(request): assert response.text == "pass" -@pytest.mark.skipif(sys.version_info < (3, 7), - reason="requires python3.7 or higher") +@pytest.mark.skipif( + sys.version_info < (3, 7), reason="requires python3.7 or higher" +) def test_create_asyncio_server(app): if not uvloop_installed(): loop = asyncio.get_event_loop() - asyncio_srv_coro = app.create_server( - return_asyncio_server=True) + asyncio_srv_coro = app.create_server(return_asyncio_server=True) assert isawaitable(asyncio_srv_coro) srv = loop.run_until_complete(asyncio_srv_coro) assert srv.is_serving() is True -@pytest.mark.skipif(sys.version_info < (3, 7), - reason="requires python3.7 or higher") +@pytest.mark.skipif( + sys.version_info < (3, 7), reason="requires python3.7 or higher" +) def test_asyncio_server_start_serving(app): if not uvloop_installed(): loop = asyncio.get_event_loop() asyncio_srv_coro = app.create_server( return_asyncio_server=True, - asyncio_server_kwargs=dict( - start_serving=False - )) + asyncio_server_kwargs=dict(start_serving=False), + ) srv = loop.run_until_complete(asyncio_srv_coro) assert srv.is_serving() is False diff --git a/tests/test_config.py b/tests/test_config.py index 241a2566d7..0bd5007be9 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -166,7 +166,7 @@ def test_config_custom_defaults(): custom_defaults = { "REQUEST_MAX_SIZE": 1, "KEEP_ALIVE": False, - "ACCESS_LOG": False + "ACCESS_LOG": False, } conf = Config(defaults=custom_defaults) for key, value in DEFAULT_CONFIG.items(): @@ -182,13 +182,13 @@ def test_config_custom_defaults_with_env(): custom_defaults = { "REQUEST_MAX_SIZE123": 1, "KEEP_ALIVE123": False, - "ACCESS_LOG123": False + "ACCESS_LOG123": False, } environ_defaults = { "SANIC_REQUEST_MAX_SIZE123": "2", "SANIC_KEEP_ALIVE123": "True", - "SANIC_ACCESS_LOG123": "False" + "SANIC_ACCESS_LOG123": "False", } for key, value in environ_defaults.items(): @@ -201,8 +201,8 @@ def test_config_custom_defaults_with_env(): try: value = int(value) except ValueError: - if value in ['True', 'False']: - value = value == 'True' + if value in ["True", "False"]: + value = value == "True" assert getattr(conf, key) == value @@ -213,7 +213,7 @@ def test_config_custom_defaults_with_env(): def test_config_access_log_passing_in_run(app): assert app.config.ACCESS_LOG == True - @app.listener('after_server_start') + @app.listener("after_server_start") async def _request(sanic, loop): app.stop() @@ -227,16 +227,18 @@ async def _request(sanic, loop): async def test_config_access_log_passing_in_create_server(app): assert app.config.ACCESS_LOG == True - @app.listener('after_server_start') + @app.listener("after_server_start") async def _request(sanic, loop): app.stop() - await app.create_server(port=1341, access_log=False, - return_asyncio_server=True) + await app.create_server( + port=1341, access_log=False, return_asyncio_server=True + ) assert app.config.ACCESS_LOG == False - await app.create_server(port=1342, access_log=True, - return_asyncio_server=True) + await app.create_server( + port=1342, access_log=True, return_asyncio_server=True + ) assert app.config.ACCESS_LOG == True diff --git a/tests/test_cookies.py b/tests/test_cookies.py index 11af714516..af7858a87b 100644 --- a/tests/test_cookies.py +++ b/tests/test_cookies.py @@ -160,10 +160,7 @@ def handler(request): assert response.cookies["test"]["max-age"] == str(DEFAULT_MAX_AGE) -@pytest.mark.parametrize( - "expires", - [datetime.now() + timedelta(seconds=60)], -) +@pytest.mark.parametrize("expires", [datetime.now() + timedelta(seconds=60)]) def test_cookie_expires(app, expires): cookies = {"test": "wait"} @@ -185,12 +182,9 @@ def handler(request): assert response.cookies["test"]["expires"] == expires -@pytest.mark.parametrize( - "expires", - ["Fri, 21-Dec-2018 15:30:00 GMT"], -) +@pytest.mark.parametrize("expires", ["Fri, 21-Dec-2018 15:30:00 GMT"]) def test_cookie_expires_illegal_instance_type(expires): c = Cookie("test_cookie", "value") - with pytest.raises(expected_exception=KeyError) as e: + with pytest.raises(expected_exception=TypeError) as e: c["expires"] = expires assert e.message == "Cookie 'expires' property must be a datetime" diff --git a/tests/test_keep_alive_timeout.py b/tests/test_keep_alive_timeout.py index af2317188e..566edae0bf 100644 --- a/tests/test_keep_alive_timeout.py +++ b/tests/test_keep_alive_timeout.py @@ -9,10 +9,8 @@ from sanic.testing import SanicTestClient, HOST, PORT -CONFIG_FOR_TESTS = { - "KEEP_ALIVE_TIMEOUT": 2, - "KEEP_ALIVE": True -} +CONFIG_FOR_TESTS = {"KEEP_ALIVE_TIMEOUT": 2, "KEEP_ALIVE": True} + class ReuseableTCPConnector(TCPConnector): def __init__(self, *args, **kwargs): @@ -51,9 +49,7 @@ def _sanic_endpoint_test( uri="/", gather_request=True, debug=False, - server_kwargs={ - "return_asyncio_server": True, - }, + server_kwargs={"return_asyncio_server": True}, *request_args, **request_kwargs ): @@ -147,7 +143,7 @@ async def _collect_response(loop): # loop, so the changes above are required too. async def _local_request(self, method, uri, cookies=None, *args, **kwargs): request_keepalive = kwargs.pop( - "request_keepalive", CONFIG_FOR_TESTS['KEEP_ALIVE_TIMEOUT'] + "request_keepalive", CONFIG_FOR_TESTS["KEEP_ALIVE_TIMEOUT"] ) if uri.startswith(("http:", "https:", "ftp:", "ftps://" "//")): url = uri diff --git a/tests/test_logo.py b/tests/test_logo.py index 901d7b8dce..eb54bccfa3 100644 --- a/tests/test_logo.py +++ b/tests/test_logo.py @@ -12,8 +12,7 @@ def test_logo_base(app, caplog): - server = app.create_server( - debug=True, return_asyncio_server=True) + server = app.create_server(debug=True, return_asyncio_server=True) loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) loop._stopping = False @@ -32,8 +31,7 @@ def test_logo_base(app, caplog): def test_logo_false(app, caplog): app.config.LOGO = False - server = app.create_server( - debug=True, return_asyncio_server=True) + server = app.create_server(debug=True, return_asyncio_server=True) loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) loop._stopping = False @@ -52,8 +50,7 @@ def test_logo_false(app, caplog): def test_logo_true(app, caplog): app.config.LOGO = True - server = app.create_server( - debug=True, return_asyncio_server=True) + server = app.create_server(debug=True, return_asyncio_server=True) loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) loop._stopping = False @@ -72,8 +69,7 @@ def test_logo_true(app, caplog): def test_logo_custom(app, caplog): app.config.LOGO = "My Custom Logo" - server = app.create_server( - debug=True, return_asyncio_server=True) + server = app.create_server(debug=True, return_asyncio_server=True) loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) loop._stopping = False diff --git a/tests/test_request_timeout.py b/tests/test_request_timeout.py index f3460ed2ca..1e7db05d44 100644 --- a/tests/test_request_timeout.py +++ b/tests/test_request_timeout.py @@ -151,9 +151,7 @@ async def _local_request(self, method, uri, cookies=None, *args, **kwargs): host=HOST, port=self.port, uri=uri ) conn = DelayableTCPConnector( - pre_request_delay=self._request_delay, - ssl=False, - loop=self._loop, + pre_request_delay=self._request_delay, ssl=False, loop=self._loop ) async with aiohttp.ClientSession( cookies=cookies, connector=conn, loop=self._loop diff --git a/tests/test_server_events.py b/tests/test_server_events.py index 2e2cf513a9..c15f9ed96a 100644 --- a/tests/test_server_events.py +++ b/tests/test_server_events.py @@ -83,8 +83,7 @@ class MySanicDb: async def init_db(app, loop): app.db = MySanicDb() - await app.create_server( - debug=True, return_asyncio_server=True) + await app.create_server(debug=True, return_asyncio_server=True) assert hasattr(app, "db") assert isinstance(app.db, MySanicDb) diff --git a/tests/test_worker.py b/tests/test_worker.py index 564f9f670d..7000cccf9d 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -24,28 +24,28 @@ def gunicorn_worker(): worker.kill() -@pytest.fixture(scope='module') +@pytest.fixture(scope="module") def gunicorn_worker_with_access_logs(): command = ( - 'gunicorn ' - '--bind 127.0.0.1:1338 ' - '--worker-class sanic.worker.GunicornWorker ' - 'examples.simple_server:app' + "gunicorn " + "--bind 127.0.0.1:1338 " + "--worker-class sanic.worker.GunicornWorker " + "examples.simple_server:app" ) worker = subprocess.Popen(shlex.split(command), stdout=subprocess.PIPE) time.sleep(2) return worker -@pytest.fixture(scope='module') +@pytest.fixture(scope="module") def gunicorn_worker_with_env_var(): command = ( 'env SANIC_ACCESS_LOG="False" ' - 'gunicorn ' - '--bind 127.0.0.1:1339 ' - '--worker-class sanic.worker.GunicornWorker ' - '--log-level info ' - 'examples.simple_server:app' + "gunicorn " + "--bind 127.0.0.1:1339 " + "--worker-class sanic.worker.GunicornWorker " + "--log-level info " + "examples.simple_server:app" ) worker = subprocess.Popen(shlex.split(command), stdout=subprocess.PIPE) time.sleep(2) @@ -62,7 +62,7 @@ def test_gunicorn_worker_no_logs(gunicorn_worker_with_env_var): """ if SANIC_ACCESS_LOG was set to False do not show access logs """ - with urllib.request.urlopen('http://localhost:1339/') as _: + with urllib.request.urlopen("http://localhost:1339/") as _: gunicorn_worker_with_env_var.kill() assert not gunicorn_worker_with_env_var.stdout.read() @@ -71,9 +71,12 @@ def test_gunicorn_worker_with_logs(gunicorn_worker_with_access_logs): """ default - show access logs """ - with urllib.request.urlopen('http://localhost:1338/') as _: + with urllib.request.urlopen("http://localhost:1338/") as _: gunicorn_worker_with_access_logs.kill() - assert b"(sanic.access)[INFO][127.0.0.1" in gunicorn_worker_with_access_logs.stdout.read() + assert ( + b"(sanic.access)[INFO][127.0.0.1" + in gunicorn_worker_with_access_logs.stdout.read() + ) class GunicornTestWorker(GunicornWorker): From 501900733939766858afbf3948d75dad2d1e9540 Mon Sep 17 00:00:00 2001 From: LTMenezes Date: Wed, 6 Feb 2019 00:26:57 -0200 Subject: [PATCH 4/4] Fix import order --- sanic/cookies.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sanic/cookies.py b/sanic/cookies.py index d7337655ab..19907945e1 100644 --- a/sanic/cookies.py +++ b/sanic/cookies.py @@ -1,5 +1,6 @@ import re import string + from datetime import datetime