From 6e212f0e33270dcc2a7a388e8005e4d6b4e5a999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BE=9D=E7=91=AA=E8=B2=93?= Date: Sat, 4 Feb 2023 09:34:52 +0800 Subject: [PATCH] Revised the Pagination utility to handle the malformed and illegal page number and page size values. --- src/accounting/utils/pagination.py | 80 ++++++++++++++++++++++++--- tests/test_utils.py | 88 +++++++++++++++++++++++------- 2 files changed, 140 insertions(+), 28 deletions(-) diff --git a/src/accounting/utils/pagination.py b/src/accounting/utils/pagination.py index a5b133b..204b236 100644 --- a/src/accounting/utils/pagination.py +++ b/src/accounting/utils/pagination.py @@ -24,6 +24,7 @@ from urllib.parse import urlparse, parse_qsl, urlencode, urlunparse, \ ParseResult from flask import request +from werkzeug.routing import RequestRedirect from accounting.locale import gettext @@ -52,6 +53,12 @@ class PageLink: """Whether the link should be shown on mobile screens.""" +class Redirection(RequestRedirect): + """The redirection.""" + code = 302 + """The HTTP code.""" + + T = t.TypeVar("T") @@ -69,12 +76,14 @@ class Pagination(t.Generic[T]): :param is_reversed: True if the default page is the last page, or False otherwise. """ + self.__current_uri: str = request.full_path if request.query_string \ + else request.path + """The current URI.""" self.__items: list[T] = items """All the items.""" self.__is_reversed: bool = is_reversed """Whether the default page is the last page.""" - self.page_size: int = int(request.args.get("page-size", - self.DEFAULT_PAGE_SIZE)) + self.page_size: int = self.__get_page_size() """The number of items in a page.""" self.__total_pages: int = 0 if len(items) == 0 \ else int((len(items) - 1) / self.page_size) + 1 @@ -89,9 +98,6 @@ class Pagination(t.Generic[T]): """The items shown in the list""" if self.__total_pages > 0: self.__set_list() - self.__current_uri: str = request.full_path if request.query_string \ - else request.path - """The current URI.""" self.__base_uri_params: tuple[list[str], list[tuple[str, str]]] \ = self.__get_base_uri_params() """The base URI parameters.""" @@ -100,6 +106,19 @@ class Pagination(t.Generic[T]): self.page_sizes: list[PageLink] = self.__get_page_sizes() """The links to switch the number of items in a page.""" + def __get_page_size(self) -> int: + """Returns the page size. + + :return: The page size. + :raise Redirection: When the page size is malformed. + """ + if "page-size" not in request.args: + return self.DEFAULT_PAGE_SIZE + try: + return int(request.args["page-size"]) + except ValueError: + raise Redirection(self.__uri_set("page-size", None)) + def __set_list(self) -> None: """Sets the items to show in the list. @@ -107,8 +126,7 @@ class Pagination(t.Generic[T]): """ self.__default_page_no = self.__total_pages if self.__is_reversed \ else 1 - self.page_no = int(request.args.get("page-no", - self.__default_page_no)) + self.page_no = self.__get_page_no() if self.page_no < 1: self.page_no = 1 if self.page_no > self.__total_pages: @@ -119,6 +137,54 @@ class Pagination(t.Generic[T]): upper_bound = len(self.__items) self.list = self.__items[lower_bound:upper_bound] + def __get_page_no(self) -> int: + """Returns the page number. + + :return: The page number. + :raise Redirection: When the page number is malformed. + """ + if "page-no" not in request.args: + return self.__default_page_no + try: + page_no: int = int(request.args["page-no"]) + except ValueError: + raise Redirection(self.__uri_set("page-no", None)) + if page_no < 1: + if not self.__is_reversed: + raise Redirection(self.__uri_set("page-no", None)) + raise Redirection(self.__uri_set("page-no", "1")) + if page_no > self.__total_pages: + if self.__is_reversed: + raise Redirection(self.__uri_set("page-no", None)) + raise Redirection(self.__uri_set("page-no", + str(self.__total_pages))) + return page_no + + def __uri_set(self, name: str, value: str | None) -> str: + """Raises current URI with a parameter set. + + :param name: The name of the parameter. + :param value: The value, or None to remove the parameter. + :return: The URI with the parameter set. + """ + uri_p: ParseResult = urlparse(self.__current_uri) + params: list[tuple[str, str]] = parse_qsl(uri_p.query) + + # Try to keep the position of the parameter. + i: int = 0 + is_found: bool = False + while i < len(params): + if params[i][0] == name: + if is_found or value is None: + params = params[:i] + params[i + 1:] + continue + params[i] = (name, value) + i = i + 1 + + parts: list[str] = list(uri_p) + parts[4] = urlencode(params) + return urlunparse(parts) + def __get_base_uri_params(self) -> tuple[list[str], list[tuple[str, str]]]: """Returns the base URI and its parameters, with the "page-no" and "page-size" parameters removed. diff --git a/tests/test_utils.py b/tests/test_utils.py index 4d8fece..750764c 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -166,9 +166,9 @@ class PaginationTestCase(unittest.TestCase): self.client = httpx.Client(app=self.app, base_url="https://testserver") self.client.headers["Referer"] = "https://testserver" - def __test_pagination(self, query: str, items: range, - result: range, is_needed: bool = True, - is_reversed: bool | None = None) -> None: + def __test_success(self, query: str, items: range, + result: range, is_needed: bool = True, + is_reversed: bool | None = None) -> None: """Tests the pagination. :param query: The query string. @@ -186,17 +186,34 @@ class PaginationTestCase(unittest.TestCase): response: httpx.Response = self.client.get(target) self.assertEqual(response.status_code, 200) + def __test_malformed(self, query: str, items: range, redirect_to: str, + is_reversed: bool | None = None) -> None: + """Tests the pagination. + + :param query: The query string. + :param items: The original items. + :param redirect_to: The expected target query of the redirection. + :param is_reversed: Whether the list is reversed. + :return: None. + """ + target: str = "/test-pagination" + self.params = self.Params(list(items), is_reversed, [], True) + response: httpx.Response = self.client.get(f"{target}?{query}") + self.assertEqual(response.status_code, 302) + self.assertEqual(response.headers["Location"], + f"{target}?{redirect_to}") + def test_default(self) -> None: """Tests the default pagination. :return: None. """ # The default first page - self.__test_pagination("", range(1, 687), range(1, 11)) + self.__test_success("", range(1, 687), range(1, 11)) # Some page in the middle - self.__test_pagination("page-no=37", range(1, 687), range(361, 371)) + self.__test_success("page-no=37", range(1, 687), range(361, 371)) # The last page - self.__test_pagination("page-no=69", range(1, 687), range(681, 687)) + self.__test_success("page-no=69", range(1, 687), range(681, 687)) def test_page_size(self) -> None: """Tests the pagination with a different page size. @@ -204,13 +221,13 @@ class PaginationTestCase(unittest.TestCase): :return: None. """ # The default page with a different page size - self.__test_pagination("page-size=15", range(1, 687), range(1, 16)) + self.__test_success("page-size=15", range(1, 687), range(1, 16)) # Some page with a different page size - self.__test_pagination("page-no=37&page-size=15", range(1, 687), - range(541, 556)) + self.__test_success("page-no=37&page-size=15", range(1, 687), + range(541, 556)) # The last page with a different page size. - self.__test_pagination("page-no=46&page-size=15", range(1, 687), - range(676, 687)) + self.__test_success("page-no=46&page-size=15", range(1, 687), + range(676, 687)) def test_not_needed(self) -> None: """Tests the pagination that is not needed. @@ -218,12 +235,12 @@ class PaginationTestCase(unittest.TestCase): :return: None. """ # Empty list - self.__test_pagination("", range(0, 0), range(0, 0), is_needed=False) + self.__test_success("", range(0, 0), range(0, 0), is_needed=False) # A list that fits in one page - self.__test_pagination("", range(1, 4), range(1, 4), is_needed=False) + self.__test_success("", range(1, 4), range(1, 4), is_needed=False) # A large page size that fits in everything - self.__test_pagination("page-size=1000", range(1, 687), range(1, 687), - is_needed=False) + self.__test_success("page-size=1000", range(1, 687), range(1, 687), + is_needed=False) def test_reversed(self) -> None: """Tests the default page on a reversed list. @@ -231,11 +248,11 @@ class PaginationTestCase(unittest.TestCase): :return: None. """ # The default page - self.__test_pagination("", range(1, 687), range(681, 687), - is_reversed=True) + self.__test_success("", range(1, 687), range(681, 687), + is_reversed=True) # The default page with a different page size - self.__test_pagination("page-size=15", range(1, 687), range(676, 687), - is_reversed=True) + self.__test_success("page-size=15", range(1, 687), range(676, 687), + is_reversed=True) def test_last_page(self) -> None: """Tests the calculation of the items on the last page. @@ -243,6 +260,35 @@ class PaginationTestCase(unittest.TestCase): :return: None. """ # The last page that fits in one page - self.__test_pagination("page-no=69", range(1, 691), range(681, 691)) + self.__test_success("page-no=69", range(1, 691), range(681, 691)) # A danging item in the last page - self.__test_pagination("page-no=70", range(1, 692), range(691, 692)) + self.__test_success("page-no=70", range(1, 692), range(691, 692)) + + def test_malformed(self) -> None: + """Tests the malformed pagination parameters. + + :return: None. + """ + # A malformed page size + self.__test_malformed("q=word&page-size=100a&page-no=37&next=%2F", + range(1, 691), "q=word&page-no=37&next=%2F") + # A malformed page number + self.__test_malformed("q=word&page-size=15&page-no=37a&next=%2F", + range(1, 691), "q=word&page-size=15&next=%2F") + # A page number beyond the last page + self.__test_malformed("q=word&page-size=15&page-no=100&next=%2F", + range(1, 691), + "q=word&page-size=15&page-no=46&next=%2F") + # A page number beyond the last page, on a reversed list + self.__test_malformed("q=word&page-size=15&page-no=100&next=%2F", + range(1, 691), + "q=word&page-size=15&next=%2F", is_reversed=True) + # A page number before the first page + self.__test_malformed("q=word&page-size=15&page-no=0&next=%2F", + range(1, 691), + "q=word&page-size=15&next=%2F") + # A page number before the first page, on a reversed list + self.__test_malformed("q=word&page-size=15&page-no=0&next=%2F", + range(1, 691), + "q=word&page-size=15&page-no=1&next=%2F", + is_reversed=True)