From f1ff14d29cc124f2ca4e698d1ac28ff79eea97d2 Mon Sep 17 00:00:00 2001 From: s3lph Date: Sat, 7 Jul 2018 18:58:23 +0200 Subject: [PATCH 1/3] More user-friendly return value in the pagelet API. --- matemat/exceptions/AuthenticatonError.py | 2 +- matemat/exceptions/HttpException.py | 20 ++++ matemat/exceptions/__init__.py | 1 + matemat/webserver/httpd.py | 106 ++++++++++++------ matemat/webserver/pagelets/login.py | 19 ++-- matemat/webserver/pagelets/logout.py | 5 +- matemat/webserver/pagelets/main.py | 39 +++---- matemat/webserver/pagelets/touchkey.py | 19 ++-- matemat/webserver/test/abstract_httpd_test.py | 8 +- matemat/webserver/test/test_post.py | 2 +- matemat/webserver/test/test_serve.py | 10 +- matemat/webserver/test/test_session.py | 2 +- 12 files changed, 141 insertions(+), 92 deletions(-) create mode 100644 matemat/exceptions/HttpException.py diff --git a/matemat/exceptions/AuthenticatonError.py b/matemat/exceptions/AuthenticatonError.py index 0d23c2e..d332fdc 100644 --- a/matemat/exceptions/AuthenticatonError.py +++ b/matemat/exceptions/AuthenticatonError.py @@ -2,7 +2,7 @@ from typing import Optional -class AuthenticationError(BaseException): +class AuthenticationError(Exception): def __init__(self, msg: Optional[str] = None) -> None: super().__init__() diff --git a/matemat/exceptions/HttpException.py b/matemat/exceptions/HttpException.py new file mode 100644 index 0000000..4f792ce --- /dev/null +++ b/matemat/exceptions/HttpException.py @@ -0,0 +1,20 @@ + +class HttpException(Exception): + + def __init__(self, status: int = 500, title: str = 'An error occurred', message: str = None) -> None: + super().__init__() + self.__status: int = status + self.__title: str = title + self.__message: str = message + + @property + def status(self) -> int: + return self.__status + + @property + def title(self) -> str: + return self.__title + + @property + def message(self) -> str: + return self.__message diff --git a/matemat/exceptions/__init__.py b/matemat/exceptions/__init__.py index ebae437..9b10b5f 100644 --- a/matemat/exceptions/__init__.py +++ b/matemat/exceptions/__init__.py @@ -4,3 +4,4 @@ This package provides custom exception classes used in the Matemat codebase. from .AuthenticatonError import AuthenticationError from .DatabaseConsistencyError import DatabaseConsistencyError +from .HttpException import HttpException diff --git a/matemat/webserver/httpd.py b/matemat/webserver/httpd.py index b703f72..a161d42 100644 --- a/matemat/webserver/httpd.py +++ b/matemat/webserver/httpd.py @@ -1,5 +1,5 @@ -from typing import Any, Callable, Dict, Optional, Tuple, Union +from typing import Any, Callable, Dict, Tuple, Union import traceback @@ -13,10 +13,10 @@ from uuid import uuid4 from datetime import datetime, timedelta from matemat import __version__ as matemat_version +from matemat.exceptions import HttpException from matemat.webserver import RequestArguments from matemat.webserver.util import parse_args - # # Python internal class hacks # @@ -27,15 +27,16 @@ TCPServer.address_family = socket.AF_INET6 BaseHTTPRequestHandler.log_request = lambda self, code='-', size='-': None BaseHTTPRequestHandler.log_error = lambda self, fstring='', *args: None - # Dictionary to hold registered pagelet paths and their handler functions _PAGELET_PATHS: Dict[str, Callable[[str, # HTTP method (GET, POST, ...) str, # Request path RequestArguments, # HTTP Request arguments Dict[str, Any], # Session vars Dict[str, str]], # Response headers - Tuple[int, Union[bytes, str]]]] = dict() # Returns: (status code, response body) - + Union[ # Return type: either a response body, or a redirect + bytes, str, # Response body: will assign HTTP/1.0 200 OK + Tuple[int, str] # Redirect: First element must be 301, second the redirect path + ]]] = dict() # Inactivity timeout for client sessions _SESSION_TIMEOUT: int = 3600 @@ -54,15 +55,17 @@ def pagelet(path: str): args: RequestArguments, session_vars: Dict[str, Any], headers: Dict[str, str]) - -> (int, Optional[Union[str, bytes]]) + -> Union[bytes, str, Tuple[int, str]] method: The HTTP method (GET, POST) that was used. path: The path that was requested. args: The arguments that were passed with the request (as GET or POST arguments). session_vars: The session storage. May be read from and written to. headers: The dictionary of HTTP response headers. Add headers you wish to send with the response. - returns: A tuple consisting of the HTTP status code (as an int) and the response body (as str or bytes, - may be None) + returns: One of the following: + - A HTTP Response body as str or bytes + - A HTTP redirect: A tuple of 301 (an int) and the path to redirect to (a str) + raises: HttpException: If a non-200 HTTP status code should be returned :param path: The path to register the function for. """ @@ -72,11 +75,15 @@ def pagelet(path: str): RequestArguments, Dict[str, Any], Dict[str, str]], - Tuple[int, Optional[Union[bytes, str]]]]): + Union[ + bytes, str, + Tuple[int, str] + ]]): # Add the function to the dict of pagelets _PAGELET_PATHS[path] = fun # Don't change the function itself at all return fun + # Return the inner function (Python requires a "real" function annotation to not have any arguments except # the function itself) return http_handler @@ -167,7 +174,7 @@ class HttpHandler(BaseHTTPRequestHandler): if session_id not in self.server.session_vars: self.server.session_vars[session_id] = (now + timedelta(seconds=_SESSION_TIMEOUT)), dict() else: - self.server.session_vars[session_id] =\ + self.server.session_vars[session_id] = \ (now + timedelta(seconds=_SESSION_TIMEOUT), self.server.session_vars[session_id][1]) # Return the session ID and timeout return session_id, self.server.session_vars[session_id][0] @@ -181,6 +188,45 @@ class HttpHandler(BaseHTTPRequestHandler): if session_id in self.server.session_vars: del self.server.session_vars[session_id] + @staticmethod + def _parse_pagelet_result(pagelet_res: Union[bytes, str, Tuple[int, str]], headers: Dict[str, str]) \ + -> Tuple[int, bytes]: + """ + Process the return value of a pagelet function call. + + :param pagelet_res: The pagelet return value. + :param headers: The dict of HTTP response headers, needed for setting the redirect header. + :return: The HTTP Response status code (an int) and body (a bytes). + :raises TypeError: If the pagelet result was not in the expected form. + """ + # The HTTP Response Status Code, defaults to 200 OK + hsc: int = 200 + # The HTTP Response body, defaults to None + data: Union[bytes, str] = None + if isinstance(pagelet_res, tuple): + # If the return type is a tuple, the first element must be 301 (the HTTP Redirect status code) + head, tail = pagelet_res + if head == 301: + # Set the HTTP Response Status Code, and the redirect header + hsc = 301 + headers['Location'] = tail + else: + raise TypeError(f'Return value of pagelet not understood: {pagelet_res}') + elif isinstance(pagelet_res, str) or isinstance(pagelet_res, bytes): + # Return value is a response body + data = pagelet_res + else: + # Return value is not a response body or a redirect + raise TypeError(f'Return value of pagelet not understood: {pagelet_res}') + # The pagelet may return None as data as a shorthand for an empty response + if data is None: + data = bytes() + # If the pagelet returns a Python str, convert it to an UTF-8 encoded bytes object + if isinstance(data, str): + data = data.encode('utf-8') + # Return the resulting status code and body + return hsc, data + def _handle(self, method: str, path: str, args: RequestArguments) -> None: """ Handle a HTTP request by either dispatching it to the appropriate pagelet or by serving a static resource. @@ -209,13 +255,9 @@ class HttpHandler(BaseHTTPRequestHandler): 'Cache-Control': 'no-cache' } # Call the pagelet function - hsc, data = _PAGELET_PATHS[path](method, path, args, self.session_vars, headers) - # The pagelet may return None as data as a shorthand for an empty response - if data is None: - data = bytes() - # If the pagelet returns a Python str, convert it to an UTF-8 encoded bytes object - if isinstance(data, str): - data = data.encode('utf-8') + pagelet_res = _PAGELET_PATHS[path](method, path, args, self.session_vars, headers) + # Parse the pagelet's return value, vielding a HTTP status code and a response body + hsc, data = HttpHandler._parse_pagelet_result(pagelet_res, headers) # Send the HTTP status code self.send_response(hsc) # Format the session cookie timeout string and send the session cookie header @@ -272,16 +314,18 @@ class HttpHandler(BaseHTTPRequestHandler): path, args = parse_args(self.path) self._handle('GET', path, args) # Special handling for some errors + except HttpException as e: + self.send_error(e.status, e.title, e.message) except PermissionError: - self.send_response(403, 'Forbidden') - self.end_headers() + self.send_error(403, 'Forbidden') except ValueError: - self.send_response(400, 'Bad Request') - self.end_headers() - except BaseException: + self.send_error(400, 'Bad Request') + except BaseException as e: # Generic error handling - self.send_response(500, 'Internal Server Error') - self.end_headers() + self.send_error(500, 'Internal Server Error') + print(e) + + traceback.print_tb(e.__traceback__) # noinspection PyPep8Naming def do_POST(self) -> None: @@ -299,19 +343,15 @@ class HttpHandler(BaseHTTPRequestHandler): # Parse the request and hand it to the handle function self._handle('POST', path, args) # Special handling for some errors + except HttpException as e: + self.send_error(e.status, e.title, e.message) except PermissionError: - self.send_response(403, 'Forbidden') - self.end_headers() + self.send_error(403, 'Forbidden') except ValueError: - self.send_response(400, 'Bad Request') - self.end_headers() - except TypeError: - self.send_response(400, 'Bad Request') - self.end_headers() + self.send_error(400, 'Bad Request') except BaseException as e: # Generic error handling - self.send_response(500, 'Internal Server Error') - self.end_headers() + self.send_error(500, 'Internal Server Error') print(e) traceback.print_tb(e.__traceback__) diff --git a/matemat/webserver/pagelets/login.py b/matemat/webserver/pagelets/login.py index 7d0cc2d..150b174 100644 --- a/matemat/webserver/pagelets/login.py +++ b/matemat/webserver/pagelets/login.py @@ -1,7 +1,7 @@ -from typing import Any, Dict, Optional, Tuple, Union +from typing import Any, Dict, Tuple, Union -from matemat.exceptions import AuthenticationError +from matemat.exceptions import AuthenticationError, HttpException from matemat.webserver import pagelet, RequestArguments from matemat.primitives import User from matemat.db import MatematDatabase @@ -13,10 +13,9 @@ def login_page(method: str, args: RequestArguments, session_vars: Dict[str, Any], headers: Dict[str, str])\ - -> Tuple[int, Optional[Union[str, bytes]]]: + -> Union[bytes, str, Tuple[int, str]]: if 'user' in session_vars: - headers['Location'] = '/' - return 301, None + return 301, '/' if method == 'GET': data = ''' @@ -41,15 +40,13 @@ def login_page(method: str, ''' - return 200, data.format(msg=str(args.msg) if 'msg' in args else '') + return data.format(msg=str(args.msg) if 'msg' in args else '') elif method == 'POST': with MatematDatabase('test.db') as db: try: user: User = db.login(str(args.username), str(args.password)) except AuthenticationError: - headers['Location'] = '/login?msg=Username%20or%20password%20wrong.%20Please%20try%20again.' - return 301, bytes() + return 301, '/login?msg=Username%20or%20password%20wrong.%20Please%20try%20again.' session_vars['user'] = user - headers['Location'] = '/' - return 301, bytes() - return 405, None + return 301, '/' + raise HttpException(405) diff --git a/matemat/webserver/pagelets/logout.py b/matemat/webserver/pagelets/logout.py index beb86a3..05bcfe2 100644 --- a/matemat/webserver/pagelets/logout.py +++ b/matemat/webserver/pagelets/logout.py @@ -10,8 +10,7 @@ def logout(method: str, args: RequestArguments, session_vars: Dict[str, Any], headers: Dict[str, str])\ - -> Tuple[int, Optional[Union[str, bytes]]]: + -> Union[bytes, str, Tuple[int, str]]: if 'user' in session_vars: del session_vars['user'] - headers['Location'] = '/' - return 301, None + return 301, '/' diff --git a/matemat/webserver/pagelets/main.py b/matemat/webserver/pagelets/main.py index e22c872..db4c49a 100644 --- a/matemat/webserver/pagelets/main.py +++ b/matemat/webserver/pagelets/main.py @@ -12,7 +12,7 @@ def main_page(method: str, args: RequestArguments, session_vars: Dict[str, Any], headers: Dict[str, str])\ - -> Tuple[int, Optional[Union[str, bytes]]]: + -> Union[bytes, str, Tuple[int, str]]: data = ''' @@ -34,24 +34,19 @@ def main_page(method: str, ''' - try: - with MatematDatabase('test.db') as db: - if 'user' in session_vars: - user: User = session_vars['user'] - products = db.list_products() - plist = '\n'.join([f'
  • {p.name} ' + - f'{p.price_member//100 if user.is_member else p.price_non_member//100}' + - f'.{p.price_member%100 if user.is_member else p.price_non_member%100}' - for p in products]) - uname = f'{user.name} (Logout)' - data = data.format(user=uname, list=plist) - else: - users = db.list_users() - ulist = '\n'.join([f'
  • {u.name}' for u in users]) - ulist = ulist + '
  • Password login' - data = data.format(user='', list=ulist) - return 200, data - except BaseException as e: - import traceback - traceback.print_tb(e.__traceback__) - return 500, None + with MatematDatabase('test.db') as db: + if 'user' in session_vars: + user: User = session_vars['user'] + products = db.list_products() + plist = '\n'.join([f'
  • {p.name} ' + + f'{p.price_member//100 if user.is_member else p.price_non_member//100}' + + f'.{p.price_member%100 if user.is_member else p.price_non_member%100}' + for p in products]) + uname = f'{user.name} (Logout)' + data = data.format(user=uname, list=plist) + else: + users = db.list_users() + ulist = '\n'.join([f'
  • {u.name}' for u in users]) + ulist = ulist + '
  • Password login' + data = data.format(user='', list=ulist) + return data diff --git a/matemat/webserver/pagelets/touchkey.py b/matemat/webserver/pagelets/touchkey.py index 4de8009..280610e 100644 --- a/matemat/webserver/pagelets/touchkey.py +++ b/matemat/webserver/pagelets/touchkey.py @@ -1,7 +1,7 @@ -from typing import Any, Dict, Optional, Tuple, Union +from typing import Any, Dict, Tuple, Union -from matemat.exceptions import AuthenticationError +from matemat.exceptions import AuthenticationError, HttpException from matemat.webserver import pagelet, RequestArguments from matemat.primitives import User from matemat.db import MatematDatabase @@ -13,10 +13,9 @@ def touchkey_page(method: str, args: RequestArguments, session_vars: Dict[str, Any], headers: Dict[str, str])\ - -> Tuple[int, Optional[Union[str, bytes]]]: + -> Union[bytes, str, Tuple[int, str]]: if 'user' in session_vars: - headers['Location'] = '/' - return 301, bytes() + return 301, '/' if method == 'GET': data = ''' @@ -40,15 +39,13 @@ def touchkey_page(method: str, ''' - return 200, data.format(username=str(args.username) if 'username' in args else '') + return data.format(username=str(args.username) if 'username' in args else '') elif method == 'POST': with MatematDatabase('test.db') as db: try: user: User = db.login(str(args.username), touchkey=str(args.touchkey)) except AuthenticationError: - headers['Location'] = f'/touchkey?username={args["username"]}&msg=Please%20try%20again.' - return 301, bytes() + return 301, f'/touchkey?username={args["username"]}&msg=Please%20try%20again.' session_vars['user'] = user - headers['Location'] = '/' - return 301, None - return 405, None + return 301, '/' + raise HttpException(405) diff --git a/matemat/webserver/test/abstract_httpd_test.py b/matemat/webserver/test/abstract_httpd_test.py index 103979b..ff4c0c6 100644 --- a/matemat/webserver/test/abstract_httpd_test.py +++ b/matemat/webserver/test/abstract_httpd_test.py @@ -16,6 +16,8 @@ class HttpResponse: """ A really basic HTTP response container and parser class, just good enough for unit testing a HTTP server, if even. + DO NOT USE THIS OUTSIDE UNIT TESTING! + Usage: response = HttpResponse() while response.parse_phase != 'done' @@ -161,16 +163,16 @@ def test_pagelet(path: str): RequestArguments, Dict[str, Any], Dict[str, str]], - Tuple[int, Union[bytes, str]]]): + Union[bytes, str, Tuple[int, str]]]): @pagelet(path) def testing_wrapper(method: str, path: str, args: RequestArguments, session_vars: Dict[str, Any], headers: Dict[str, str]): - status, body = fun(method, path, args, session_vars, headers) headers['X-Test-Pagelet'] = fun.__name__ - return status, body + result = fun(method, path, args, session_vars, headers) + return result return testing_wrapper return with_testing_headers diff --git a/matemat/webserver/test/test_post.py b/matemat/webserver/test/test_post.py index 0bc5d16..e105354 100644 --- a/matemat/webserver/test/test_post.py +++ b/matemat/webserver/test/test_post.py @@ -24,7 +24,7 @@ def post_test_pagelet(method: str, dump += f'{a.name}: {a.get_str()}\n' else: dump += f'{a.name}: {codecs.encode(a.get_bytes(), "hex").decode("utf-8")}\n' - return 200, dump + return dump class TestPost(AbstractHttpdTest): diff --git a/matemat/webserver/test/test_serve.py b/matemat/webserver/test/test_serve.py index 722870b..b507df7 100644 --- a/matemat/webserver/test/test_serve.py +++ b/matemat/webserver/test/test_serve.py @@ -3,6 +3,7 @@ from typing import Any, Dict import os import os.path +from matemat.exceptions import HttpException from matemat.webserver import HttpHandler, RequestArguments from matemat.webserver.test.abstract_httpd_test import AbstractHttpdTest, test_pagelet @@ -14,7 +15,7 @@ def serve_test_pagelet_ok(method: str, session_vars: Dict[str, Any], headers: Dict[str, str]): headers['Content-Type'] = 'text/plain' - return 200, 'serve test pagelet ok' + return 'serve test pagelet ok' @test_pagelet('/just/testing/serve_pagelet_fail') @@ -25,7 +26,7 @@ def serve_test_pagelet_fail(method: str, headers: Dict[str, str]): session_vars['test'] = 'hello, world!' headers['Content-Type'] = 'text/plain' - return 500, 'serve test pagelet fail' + raise HttpException() class TestServe(AbstractHttpdTest): @@ -62,11 +63,8 @@ class TestServe(AbstractHttpdTest): 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_fail', packet.pagelet) - # Make sure the expected content is served + # Make sure an error is raised self.assertEqual(500, packet.statuscode) - self.assertEqual(b'serve test pagelet fail', packet.body) def test_serve_static_ok(self): # Request a static resource diff --git a/matemat/webserver/test/test_session.py b/matemat/webserver/test/test_session.py index 5cf408e..2e64ceb 100644 --- a/matemat/webserver/test/test_session.py +++ b/matemat/webserver/test/test_session.py @@ -16,7 +16,7 @@ def session_test_pagelet(method: str, headers: Dict[str, str]): session_vars['test'] = 'hello, world!' headers['Content-Type'] = 'text/plain' - return 200, 'session test' + return 'session test' class TestSession(AbstractHttpdTest): From a53144797020584fc9d2d1e5ac3bdfea79cd4031 Mon Sep 17 00:00:00 2001 From: s3lph Date: Sat, 7 Jul 2018 19:10:41 +0200 Subject: [PATCH 2/3] Added the changed pagelet API to the external documentation. --- doc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc b/doc index 14b8380..51e9404 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 14b8380090858c3bed5c3b2ee7cf1408aaa133df +Subproject commit 51e940460ddbaebb7f2ffc48d00d9ef19cf8d33f From 14f339e63052e8d05c53741b7493327aef648542 Mon Sep 17 00:00:00 2001 From: s3lph Date: Sat, 7 Jul 2018 19:24:00 +0200 Subject: [PATCH 3/3] Added unit test for redirection testing. --- matemat/webserver/httpd.py | 1 - matemat/webserver/test/test_serve.py | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/matemat/webserver/httpd.py b/matemat/webserver/httpd.py index a161d42..3892ac8 100644 --- a/matemat/webserver/httpd.py +++ b/matemat/webserver/httpd.py @@ -324,7 +324,6 @@ class HttpHandler(BaseHTTPRequestHandler): # Generic error handling self.send_error(500, 'Internal Server Error') print(e) - traceback.print_tb(e.__traceback__) # noinspection PyPep8Naming diff --git a/matemat/webserver/test/test_serve.py b/matemat/webserver/test/test_serve.py index b507df7..5bcdd4d 100644 --- a/matemat/webserver/test/test_serve.py +++ b/matemat/webserver/test/test_serve.py @@ -18,6 +18,15 @@ def serve_test_pagelet_ok(method: str, return 'serve test pagelet ok' +@test_pagelet('/just/testing/serve_pagelet_redirect') +def serve_test_pagelet_redirect(method: str, + path: str, + args: RequestArguments, + session_vars: Dict[str, Any], + headers: Dict[str, str]): + return 301, '/foo/bar' + + @test_pagelet('/just/testing/serve_pagelet_fail') def serve_test_pagelet_fail(method: str, path: str, @@ -66,6 +75,18 @@ class TestServe(AbstractHttpdTest): # Make sure an error is raised self.assertEqual(500, packet.statuscode) + def test_serve_pagelet_redirect(self): + # Call the test pagelet that redirects to another path + self.client_sock.set_request(b'GET /just/testing/serve_pagelet_redirect 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 redirect is issued + self.assertEqual(301, packet.statuscode) + self.assertEqual('/foo/bar', packet.headers['Location']) + # Make sure the response body is empty + self.assertEqual(0, len(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')