From bf50d6538922031f53a7058a3a62e9baca636b6e Mon Sep 17 00:00:00 2001 From: s3lph Date: Tue, 21 Aug 2018 01:13:35 +0200 Subject: [PATCH 1/8] Full branch coverage in currency parsing. --- matemat/util/test/test_currency_format.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/matemat/util/test/test_currency_format.py b/matemat/util/test/test_currency_format.py index 882ddac..b2f6eb5 100644 --- a/matemat/util/test/test_currency_format.py +++ b/matemat/util/test/test_currency_format.py @@ -131,3 +131,9 @@ class TestCurrencyFormat(unittest.TestCase): parse_chf('13,37') with self.assertRaises(ValueError): parse_chf('CHF 13,37') + + def test_parse_frac_negative(self): + with self.assertRaises(ValueError): + parse_chf('13.-7') + with self.assertRaises(ValueError): + parse_chf('CHF 13.-7') From 4c2c454f1206dd73441c69f56d9abf25db7dfef4 Mon Sep 17 00:00:00 2001 From: s3lph Date: Tue, 21 Aug 2018 02:34:05 +0200 Subject: [PATCH 2/8] Full branch coverage in database facade. Also removed a redundant consistency check. --- matemat/db/facade.py | 4 ---- matemat/db/test/test_facade.py | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/matemat/db/facade.py b/matemat/db/facade.py index b7f4291..5e8fffc 100644 --- a/matemat/db/facade.py +++ b/matemat/db/facade.py @@ -300,10 +300,6 @@ class MatematDatabase(object): 'is_admin': is_admin, 'is_member': is_member }) - affected = c.execute('SELECT changes()').fetchone()[0] - if affected != 1: - raise DatabaseConsistencyError( - f'change_user should affect 1 users row, but affected {affected}') # Only update the actual user object after the changes in the database succeeded user.name = name user.email = email diff --git a/matemat/db/test/test_facade.py b/matemat/db/test/test_facade.py index 5843a29..d4ab500 100644 --- a/matemat/db/test/test_facade.py +++ b/matemat/db/test/test_facade.py @@ -84,6 +84,12 @@ class DatabaseTest(unittest.TestCase): with self.db as db: with db.transaction() as c: u = db.create_user('testuser', 'supersecurepassword', 'testuser@example.com') + # Attempt touchkey login without a set touchkey + try: + db.login('testuser', touchkey='0123') + self.fail() + except AuthenticationError as e: + self.assertEqual('Touchkey not set', e.msg) # Add a touchkey without using the provided function c.execute('''UPDATE users SET touchkey = :tkhash WHERE user_id = :user_id''', { 'tkhash': crypt.crypt('0123', crypt.mksalt()), @@ -172,6 +178,9 @@ class DatabaseTest(unittest.TestCase): self.assertFalse(checkuser.is_admin) self.assertFalse(checkuser.is_member) self.assertEqual(4200, checkuser.balance) + # Balance change without an agent must fail + with self.assertRaises(ValueError): + db.change_user(user, None, balance=0) user.id = -1 with self.assertRaises(DatabaseConsistencyError): db.change_user(user, agent, is_member='True') @@ -360,3 +369,10 @@ class DatabaseTest(unittest.TestCase): self.assertEqual(60, c.fetchone()[0]) c.execute('''SELECT stock FROM products WHERE product_id = ?''', [fritzmate.id]) self.assertEqual(10, c.fetchone()[0]) + + user1.id = -1 + clubmate.id = -1 + with self.assertRaises(DatabaseConsistencyError): + db.increment_consumption(user1, florapowermate) + with self.assertRaises(DatabaseConsistencyError): + db.increment_consumption(user2, clubmate) From e71817cf8e68b526af3e3df03e722ad6b9bc7153 Mon Sep 17 00:00:00 2001 From: s3lph Date: Tue, 28 Aug 2018 21:07:32 +0200 Subject: [PATCH 3/8] Increased coverage in httpd, also some code deduplication. --- matemat/webserver/httpd.py | 46 ++++++------ matemat/webserver/test/test_httpd.py | 108 +++++++++++++++++++++++++++ matemat/webserver/test/test_serve.py | 42 +++++++++++ 3 files changed, 171 insertions(+), 25 deletions(-) create mode 100644 matemat/webserver/test/test_httpd.py diff --git a/matemat/webserver/httpd.py b/matemat/webserver/httpd.py index c0a351d..f38005c 100644 --- a/matemat/webserver/httpd.py +++ b/matemat/webserver/httpd.py @@ -455,15 +455,9 @@ class HttpHandler(BaseHTTPRequestHandler): self.send_response(404) self.end_headers() - # noinspection PyPep8Naming - def do_GET(self) -> None: - """ - Called by BasicHTTPRequestHandler for GET requests. - """ + def _handle_request(self, method: str, path: str, args: RequestArguments): try: - # Parse the request and hand it to the handle function - path, args = parse_args(self.path) - self._handle('GET', path, args) + self._handle(method, path, args) # Special handling for some errors except HttpException as e: self.send_error(e.status, e.title, e.message) @@ -483,6 +477,19 @@ class HttpHandler(BaseHTTPRequestHandler): self.send_error(500, 'Internal Server Error') self.server.logger.exception('', e.args, e) + # noinspection PyPep8Naming + def do_GET(self) -> None: + """ + Called by BasicHTTPRequestHandler for GET requests. + """ + # Parse the request and hand it to the handle function + try: + path, args = parse_args(self.path) + self._handle_request('GET', path, args) + except ValueError as e: + self.send_error(400, 'Bad Request') + self.server.logger.debug('', exc_info=e) + # noinspection PyPep8Naming def do_POST(self) -> None: """ @@ -492,29 +499,18 @@ class HttpHandler(BaseHTTPRequestHandler): # Read the POST body, if it exists, and its MIME type is application/x-www-form-urlencoded clen: int = int(str(self.headers.get('Content-Length', failobj='0'))) if clen > _MAX_POST: - raise ValueError('Request too big') + # Return a 413 error page if the request size exceeds boundaries + self.send_error(413, 'Payload Too Large') + self.server.logger.debug('', exc_info=HttpException(413, 'Payload Too Large')) + return ctype: str = self.headers.get('Content-Type', failobj='application/octet-stream') post: bytes = self.rfile.read(clen) - path, args = parse_args(self.path, postbody=post, enctype=ctype) # Parse the request and hand it to the handle function - self._handle('POST', path, args) - # Special handling for some errors - except HttpException as e: - if 500 <= e.status < 600: - self.send_error(500, 'Internal Server Error') - self.server.logger.exception('', exc_info=e) - else: - self.server.logger.debug('', exc_info=e) - except PermissionError as e: - self.send_error(403, 'Forbidden') - self.server.logger.debug('', exc_info=e) + path, args = parse_args(self.path, postbody=post, enctype=ctype) + self._handle_request('POST', path, args) except ValueError as e: self.send_error(400, 'Bad Request') self.server.logger.debug('', exc_info=e) - except BaseException as e: - # Generic error handling - self.send_error(500, 'Internal Server Error') - self.server.logger.exception('', e.args, e) @property def session_vars(self) -> Dict[str, Any]: diff --git a/matemat/webserver/test/test_httpd.py b/matemat/webserver/test/test_httpd.py new file mode 100644 index 0000000..0194770 --- /dev/null +++ b/matemat/webserver/test/test_httpd.py @@ -0,0 +1,108 @@ +from typing import Any, Dict, Union + +from matemat.exceptions import HttpException +from matemat.webserver import HttpHandler, RequestArguments, PageletResponse +from matemat.webserver.test.abstract_httpd_test import AbstractHttpdTest, test_pagelet + + +@test_pagelet('/just/testing/http_exception') +def test_pagelet_http_exception(method: str, + path: str, + args: RequestArguments, + session_vars: Dict[str, Any], + headers: Dict[str, str], + pagelet_variables: Dict[str, str]) -> Union[bytes, str, PageletResponse]: + raise HttpException(int(str(args.exc)), 'Test Exception') + + +@test_pagelet('/just/testing/value_error') +def test_pagelet_value_error(method: str, + path: str, + args: RequestArguments, + session_vars: Dict[str, Any], + headers: Dict[str, str], + pagelet_variables: Dict[str, str]) -> Union[bytes, str, PageletResponse]: + raise ValueError('test') + + +@test_pagelet('/just/testing/permission_error') +def test_pagelet_permission_error(method: str, + path: str, + args: RequestArguments, + session_vars: Dict[str, Any], + headers: Dict[str, str], + pagelet_variables: Dict[str, str]) -> Union[bytes, str, PageletResponse]: + raise PermissionError('test') + + +@test_pagelet('/just/testing/other_error') +def test_pagelet_other_error(method: str, + path: str, + args: RequestArguments, + session_vars: Dict[str, Any], + headers: Dict[str, str], + pagelet_variables: Dict[str, str]) -> Union[bytes, str, PageletResponse]: + raise TypeError('test') + + +class TestHttpd(AbstractHttpdTest): + + def test_httpd_get_illegal_path(self): + self.client_sock.set_request(b'GET /foo?bar?baz HTTP/1.1\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + self.assertEqual(400, packet.statuscode) + + def test_httpd_post_illegal_path(self): + self.client_sock.set_request(b'POST /foo?bar?baz HTTP/1.1\r\n' + b'Content-Length: 0\r\n' + b'Content-Type: application/x-www-form-urlencoded\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + self.assertEqual(400, packet.statuscode) + + def test_httpd_post_illegal_header(self): + self.client_sock.set_request(b'POST /foo?bar=baz HTTP/1.1\r\n' + b'Content-Length: 0\r\n' + b'Content-Type: application/octet-stream\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + self.assertEqual(400, packet.statuscode) + + def test_httpd_post_request_too_big(self): + self.client_sock.set_request(b'POST /foo?bar=baz HTTP/1.1\r\n' + b'Content-Length: 1000001\r\n' + b'Content-Type: application/octet-stream\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + self.assertEqual(413, packet.statuscode) + + def test_httpd_exception_http_400(self): + self.client_sock.set_request(b'GET /just/testing/http_exception?exc=400 HTTP/1.1\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + self.assertEqual(400, packet.statuscode) + + def test_httpd_exception_http_500(self): + self.client_sock.set_request(b'GET /just/testing/http_exception?exc=500 HTTP/1.1\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + self.assertEqual(500, packet.statuscode) + + def test_httpd_exception_value_error(self): + self.client_sock.set_request(b'GET /just/testing/value_error HTTP/1.1\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + self.assertEqual(400, packet.statuscode) + + def test_httpd_exception_permission_error(self): + self.client_sock.set_request(b'GET /just/testing/permission_error HTTP/1.1\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + self.assertEqual(403, packet.statuscode) + + def test_httpd_exception_other_error(self): + self.client_sock.set_request(b'GET /just/testing/other_error HTTP/1.1\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + self.assertEqual(500, packet.statuscode) diff --git a/matemat/webserver/test/test_serve.py b/matemat/webserver/test/test_serve.py index bff9a6b..e0caaa3 100644 --- a/matemat/webserver/test/test_serve.py +++ b/matemat/webserver/test/test_serve.py @@ -63,6 +63,28 @@ def serve_test_pagelet_fail(method: str, raise HttpException(599, 'Error expected during unit testing') +# noinspection PyTypeChecker +@test_pagelet('/just/testing/serve_pagelet_empty') +def serve_test_pagelet_empty(method: str, + path: str, + args: RequestArguments, + session_vars: Dict[str, Any], + headers: Dict[str, str], + pagelet_variables: Dict[str, str]) -> Union[bytes, str, PageletResponse]: + return PageletResponse() + + +# noinspection PyTypeChecker +@test_pagelet('/just/testing/serve_pagelet_type_error') +def serve_test_pagelet_fail(method: str, + path: str, + args: RequestArguments, + session_vars: Dict[str, Any], + headers: Dict[str, str], + pagelet_variables: Dict[str, str]) -> Union[bytes, str, PageletResponse]: + return 42 + + class TestServe(AbstractHttpdTest): """ Test cases for the content serving of the web server. @@ -147,6 +169,18 @@ class TestServe(AbstractHttpdTest): # Make sure the response body was rendered correctly by the templating engine self.assertEqual(b'Hello, World!', packet.body) + def test_serve_pagelet_empty(self): + # Call the test pagelet that redirects to another path + self.client_sock.set_request(b'GET /just/testing/serve_pagelet_empty HTTP/1.1\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + + # Make sure the correct pagelet was called + self.assertEqual('serve_test_pagelet_empty', packet.pagelet) + self.assertEqual(200, packet.statuscode) + # Make sure the response body was rendered correctly by the templating engine + self.assertEqual(b'', packet.body) + def test_serve_static_ok(self): # Request a static resource self.client_sock.set_request(b'GET /static_resource.txt HTTP/1.1\r\n\r\n') @@ -249,6 +283,14 @@ class TestServe(AbstractHttpdTest): packet = self.client_sock.get_response() self.assertEqual('application/x-foo-bar', packet.headers['Content-Type']) + def test_serve_pagelet_type_error(self): + # A 500 error should be returned if a pagelet returns an invalid type + self.client_sock.set_request(b'GET /just/testing/serve_pagelet_type_error HTTP/1.1\r\n\r\n') + HttpHandler(self.client_sock, ('::1', 45678), self.server) + packet = self.client_sock.get_response() + # Make sure a 500 header is served + self.assertEqual(500, packet.statuscode) + def test_serve_static_mime_extension(self): # The correct Content-Type should be guessed by file extension primarily self.client_sock.set_request(b'GET /teststyle.css HTTP/1.1\r\n\r\n') From 594f9049802ad8c11015ff9183b38dfc84a045cb Mon Sep 17 00:00:00 2001 From: s3lph Date: Tue, 28 Aug 2018 21:09:23 +0200 Subject: [PATCH 4/8] Fixed codestyle --- matemat/webserver/test/test_httpd.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/matemat/webserver/test/test_httpd.py b/matemat/webserver/test/test_httpd.py index 0194770..dbb8cad 100644 --- a/matemat/webserver/test/test_httpd.py +++ b/matemat/webserver/test/test_httpd.py @@ -27,11 +27,11 @@ def test_pagelet_value_error(method: str, @test_pagelet('/just/testing/permission_error') def test_pagelet_permission_error(method: str, - path: str, - args: RequestArguments, - session_vars: Dict[str, Any], - headers: Dict[str, str], - pagelet_variables: Dict[str, str]) -> Union[bytes, str, PageletResponse]: + path: str, + args: RequestArguments, + session_vars: Dict[str, Any], + headers: Dict[str, str], + pagelet_variables: Dict[str, str]) -> Union[bytes, str, PageletResponse]: raise PermissionError('test') From 39dacfcb39f259f71a6006caf9a3d4b5e460c988 Mon Sep 17 00:00:00 2001 From: s3lph Date: Tue, 28 Aug 2018 21:20:14 +0200 Subject: [PATCH 5/8] Full coverage in util. --- matemat/webserver/test/test_parse_request.py | 26 ++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/matemat/webserver/test/test_parse_request.py b/matemat/webserver/test/test_parse_request.py index d144ac1..8f3d35b 100644 --- a/matemat/webserver/test/test_parse_request.py +++ b/matemat/webserver/test/test_parse_request.py @@ -204,6 +204,32 @@ class TestParseRequest(unittest.TestCase): self.assertEqual(b'1337', args['bar'].get_bytes()) self.assertEqual('Hello, World!', args['baz'].get_str()) + def test_parse_post_multipart_names(self): + """ + Test that multipart names work both with and without quotation marks + """ + path, args = parse_args('/', + postbody=b'--testBoundary1337\r\n' + b'Content-Disposition: form-data; name="foo"\r\n' + b'Content-Type: text/plain\r\n\r\n' + b'42\r\n' + b'--testBoundary1337\r\n' + b'Content-Disposition: form-data; name=bar\r\n' + b'Content-Type: text/plain\r\n\r\n' + b'Hello, World!\r\n' + b'--testBoundary1337--\r\n', + enctype='multipart/form-data; boundary=testBoundary1337') + self.assertEqual('/', path) + self.assertEqual(2, len(args)) + self.assertIn('foo', args) + self.assertIn('bar', args) + self.assertTrue(args['foo'].is_scalar) + self.assertTrue(args['bar'].is_scalar) + self.assertEqual('text/plain', args['foo'].get_content_type()) + self.assertEqual('text/plain', args['bar'].get_content_type()) + self.assertEqual('42', args['foo'].get_str()) + self.assertEqual('Hello, World!', args['bar'].get_str()) + def test_parse_post_multipart_zero_arg(self): """ Test that a multipart POST request with an empty argument is parsed correctly. From 9cd6522b26700db6eaf120c862cb7468c37136bc Mon Sep 17 00:00:00 2001 From: s3lph Date: Tue, 28 Aug 2018 21:45:11 +0200 Subject: [PATCH 6/8] Full coverage in config. --- matemat/webserver/test/test_config.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/matemat/webserver/test/test_config.py b/matemat/webserver/test/test_config.py index 9e13374..e2ad985 100644 --- a/matemat/webserver/test/test_config.py +++ b/matemat/webserver/test/test_config.py @@ -241,3 +241,12 @@ class TestConfig(TestCase): with self.assertRaises(ValueError): # The filename is only a placeholder, file content is determined by mocking open parse_config_file('test') + + def test_parse_config_not_a_filename(self): + """ + Test type checking for the config filenames + """ + with self.assertRaises(TypeError): + parse_config_file(42) + with self.assertRaises(TypeError): + parse_config_file(['config', 42]) From e29423c32e2725aa4f50e2f4730dc87fdf107342 Mon Sep 17 00:00:00 2001 From: s3lph Date: Tue, 28 Aug 2018 22:14:03 +0200 Subject: [PATCH 7/8] Full coverage in exceptions. --- matemat/exceptions/test/__init__.py | 0 .../test/test_authentication_error.py | 15 +++++++++++++++ .../test/test_database_consistency_error.py | 15 +++++++++++++++ .../exceptions/test/test_http_exception.py | 19 +++++++++++++++++++ 4 files changed, 49 insertions(+) create mode 100644 matemat/exceptions/test/__init__.py create mode 100644 matemat/exceptions/test/test_authentication_error.py create mode 100644 matemat/exceptions/test/test_database_consistency_error.py create mode 100644 matemat/exceptions/test/test_http_exception.py diff --git a/matemat/exceptions/test/__init__.py b/matemat/exceptions/test/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/matemat/exceptions/test/test_authentication_error.py b/matemat/exceptions/test/test_authentication_error.py new file mode 100644 index 0000000..8c98692 --- /dev/null +++ b/matemat/exceptions/test/test_authentication_error.py @@ -0,0 +1,15 @@ + +import unittest + +from matemat.exceptions import AuthenticationError + + +class TestAuthenticationError(unittest.TestCase): + + def test_msg(self): + e = AuthenticationError('testmsg') + self.assertEqual('testmsg', e.msg) + + def test_str(self): + e = AuthenticationError('testmsg') + self.assertEqual('AuthenticationError: testmsg', str(e)) diff --git a/matemat/exceptions/test/test_database_consistency_error.py b/matemat/exceptions/test/test_database_consistency_error.py new file mode 100644 index 0000000..21f7522 --- /dev/null +++ b/matemat/exceptions/test/test_database_consistency_error.py @@ -0,0 +1,15 @@ + +import unittest + +from matemat.exceptions import DatabaseConsistencyError + + +class TestDatabaseConsistencyError(unittest.TestCase): + + def test_msg(self): + e = DatabaseConsistencyError('testmsg') + self.assertEqual('testmsg', e.msg) + + def test_str(self): + e = DatabaseConsistencyError('testmsg') + self.assertEqual('DatabaseConsistencyError: testmsg', str(e)) diff --git a/matemat/exceptions/test/test_http_exception.py b/matemat/exceptions/test/test_http_exception.py new file mode 100644 index 0000000..c9fa669 --- /dev/null +++ b/matemat/exceptions/test/test_http_exception.py @@ -0,0 +1,19 @@ + +import unittest + +from matemat.exceptions import HttpException + + +class TestHttpException(unittest.TestCase): + + def test_all_args(self): + e = HttpException(1337, 'Foo Bar', 'Lorem Ipsum Dolor Sit Amet') + self.assertEqual(1337, e.status) + self.assertEqual('Foo Bar', e.title) + self.assertEqual('Lorem Ipsum Dolor Sit Amet', e.message) + + def test_default_args(self): + e = HttpException() + self.assertEqual(500, e.status) + self.assertEqual('An error occurred', e.title) + self.assertIsNone(e.message) From fd2b0a4e55a903c289c55f7e49f1214e68f04ac2 Mon Sep 17 00:00:00 2001 From: s3lph Date: Tue, 28 Aug 2018 22:35:52 +0200 Subject: [PATCH 8/8] Better test coverage in database wrapper. --- matemat/db/test/test_wrapper.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/matemat/db/test/test_wrapper.py b/matemat/db/test/test_wrapper.py index 1b38f60..0b78890 100644 --- a/matemat/db/test/test_wrapper.py +++ b/matemat/db/test/test_wrapper.py @@ -103,3 +103,20 @@ class DatabaseTest(unittest.TestCase): with db.transaction(): with self.assertRaises(RuntimeError): self.db.close() + + def test_use_before_open(self): + with self.assertRaises(RuntimeError): + with self.db.transaction(): + pass + with self.assertRaises(RuntimeError): + self.db.close() + with self.assertRaises(RuntimeError): + _ = self.db._user_version + with self.assertRaises(RuntimeError): + self.db._user_version = 42 + + def test_setup_prevent_downgrade(self): + self.db._sqlite_db = sqlite3.connect(':memory:') + self.db._user_version = 1337 + with self.assertRaises(RuntimeError): + self.db._setup()