From e671c13620212a3f8198b988f05a4d5f45e1baba Mon Sep 17 00:00:00 2001 From: Rishi Garg Date: Sat, 25 Jan 2025 15:13:06 +0530 Subject: [PATCH 1/5] Feat: Pagination improved in the UI Signed-off-by: Rishi Garg --- vulnerabilities/pagination.py | 139 ++++++++++++++++++ .../templates/includes/pagination.html | 109 +++++++++----- vulnerabilities/tests/test_view.py | 8 +- vulnerabilities/views.py | 6 +- 4 files changed, 220 insertions(+), 42 deletions(-) diff --git a/vulnerabilities/pagination.py b/vulnerabilities/pagination.py index 5ac698e18..a014116cb 100644 --- a/vulnerabilities/pagination.py +++ b/vulnerabilities/pagination.py @@ -1,6 +1,145 @@ +import logging +import re + +from django.core.paginator import Paginator +from django.db.models.query import QuerySet from rest_framework.pagination import PageNumberPagination +logger = logging.getLogger(__name__) + class SmallResultSetPagination(PageNumberPagination): page_size_query_param = "page_size" max_page_size = 100 + +class PaginatedListViewMixin: + """ + A mixin that adds pagination functionality to ListView-based views. + """ + + paginate_by = 20 + max_page_size = 100 + PAGE_SIZE_CHOICES = [ + {"value": 20, "label": "20 per page"}, + {"value": 50, "label": "50 per page"}, + {"value": 100, "label": "100 per page"}, + ] + + def get_queryset(self): + """ + Ensure a queryset is always available + """ + try: + queryset = super().get_queryset() + if not queryset: + queryset = self.model.objects.all() + if not isinstance(queryset, QuerySet): + queryset = self.model.objects.all() + return queryset + except Exception as e: + logger.error(f"Error in get_queryset: {e}") + return self.model.objects.all() + + def sanitize_page_size(self, raw_page_size): + """ + Sanitize page size input to prevent XSS and injection attempts. + """ + if not raw_page_size: + return self.paginate_by + clean_page_size = re.sub(r"\D", "", str(raw_page_size)) + try: + page_size = int(clean_page_size) if clean_page_size else self.paginate_by + valid_sizes = {choice["value"] for choice in self.PAGE_SIZE_CHOICES} + if page_size not in valid_sizes: + logger.warning(f"Attempted to use unauthorized page size: {page_size}") + return self.paginate_by + return page_size + except (ValueError, TypeError): + logger.info("Empty or invalid page_size input attempted") + return self.paginate_by + + def get_paginate_by(self, queryset=None): + """ + Get the number of items to paginate by from the request. + """ + raw_page_size = self.request.GET.get("page_size") + return self.sanitize_page_size(raw_page_size) + + def get_page_range(self, paginator, page_obj): + """ + Generate a list of page numbers for navigation + """ + num_pages = paginator.num_pages + current_page = page_obj.number + if num_pages <= 7: + return list(range(1, num_pages + 1)) + pages = [] + pages.append(1) + if current_page > 4: + pages.append("...") + start = max(2, current_page - 2) + end = min(num_pages - 1, current_page + 2) + pages.extend(range(start, end + 1)) + if current_page < num_pages - 3: + pages.append("...") + if num_pages > 1: + pages.append(num_pages) + return [str(p) for p in pages] + + def paginate_queryset(self, queryset, page_size): + """ + Robust pagination with comprehensive error handling + """ + try: + if not queryset or queryset.count() == 0: + queryset = self.model.objects.all() + paginator = Paginator(queryset, page_size) + page_params = self.request.GET.getlist("page") + page_number = page_params[-1] if page_params else "1" + try: + page_number = int(re.sub(r"\D", "", str(page_number))) + if not page_number: + page_number = 1 + except (ValueError, TypeError): + page_number = 1 + page_number = max(1, min(page_number, paginator.num_pages)) + page = paginator.page(page_number) + return (paginator, page, page.object_list, page.has_other_pages()) + except Exception as e: + logger.error(f"Pagination error: {e}") + queryset = self.model.objects.all() + paginator = Paginator(queryset, page_size) + page = paginator.page(1) + return (paginator, page, page.object_list, page.has_other_pages()) + + def get_context_data(self, **kwargs): + """ + Return a mapping of pagination-related context data, preserving filters. + """ + queryset = self.get_queryset() + page_size = self.get_paginate_by() + paginator, page, object_list, is_paginated = self.paginate_queryset(queryset, page_size) + page_range = self.get_page_range(paginator, page) + + search = self.request.GET.get("search", "") + + context = super().get_context_data( + object_list=object_list, + page_obj=page, + paginator=paginator, + is_paginated=is_paginated, + **kwargs, + ) + + context.update( + { + "current_page_size": page_size, + "page_size_choices": self.PAGE_SIZE_CHOICES, + "total_count": paginator.count, + "page_range": page_range, + "search": search, + "previous_page_url": page.previous_page_number() if page.has_previous() else None, + "next_page_url": page.next_page_number() if page.has_next() else None, + } + ) + return context diff --git a/vulnerabilities/templates/includes/pagination.html b/vulnerabilities/templates/includes/pagination.html index 0d6dad430..b2eaa4f3f 100644 --- a/vulnerabilities/templates/includes/pagination.html +++ b/vulnerabilities/templates/includes/pagination.html @@ -1,39 +1,76 @@ - \ No newline at end of file + + + {% endif %} + + + + + diff --git a/vulnerabilities/tests/test_view.py b/vulnerabilities/tests/test_view.py index 3b32ee31c..98a555294 100644 --- a/vulnerabilities/tests/test_view.py +++ b/vulnerabilities/tests/test_view.py @@ -23,9 +23,10 @@ from vulnerabilities.models import Vulnerability from vulnerabilities.models import VulnerabilitySeverity from vulnerabilities.templatetags.url_filters import url_quote_filter -from vulnerabilities.utils import get_purl_version_class from vulnerabilities.views import PackageDetails from vulnerabilities.views import PackageSearch +from vulnerabilities.views import get_purl_version_class +from vulnerabilities.views import purl_sort_key BASE_DIR = os.path.dirname(os.path.abspath(__file__)) TEST_DIR = os.path.join(BASE_DIR, "test_data/package_sort") @@ -201,13 +202,12 @@ def setUp(self): for pkg in input_purls: real_purl = PackageURL.from_string(pkg) attrs = {k: v for k, v in real_purl.to_dict().items() if v} - pkg = Package.objects.create(**attrs) - pkg.calculate_version_rank + Package.objects.create(**attrs) def test_sorted_queryset(self): qs_all = Package.objects.all() pkgs_qs_all = list(qs_all) - sorted_pkgs_qs_all = pkgs_qs_all + sorted_pkgs_qs_all = sorted(pkgs_qs_all, key=purl_sort_key) pkg_package_urls = [obj.package_url for obj in sorted_pkgs_qs_all] sorted_purls = os.path.join(TEST_DIR, "sorted_purls.txt") diff --git a/vulnerabilities/views.py b/vulnerabilities/views.py index a2df48634..8236635b5 100644 --- a/vulnerabilities/views.py +++ b/vulnerabilities/views.py @@ -35,10 +35,12 @@ from vulnerablecode import __version__ as VULNERABLECODE_VERSION from vulnerablecode.settings import env +from .pagination import PaginatedListViewMixin + PAGE_SIZE = 20 -class PackageSearch(ListView): +class PackageSearch(PaginatedListViewMixin,ListView): model = models.Package template_name = "packages.html" ordering = ["type", "namespace", "name", "version"] @@ -66,7 +68,7 @@ def get_queryset(self, query=None): ) -class VulnerabilitySearch(ListView): +class VulnerabilitySearch(PaginatedListViewMixin,ListView): model = models.Vulnerability template_name = "vulnerabilities.html" ordering = ["vulnerability_id"] From d9813562810ecf56f217fde611dea9399dca63b2 Mon Sep 17 00:00:00 2001 From: Rishi Garg Date: Sat, 25 Jan 2025 15:16:50 +0530 Subject: [PATCH 2/5] Fix : Workflow checks error fixed Signed-off-by: Rishi Garg --- vulnerabilities/pagination.py | 1 + vulnerabilities/views.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/vulnerabilities/pagination.py b/vulnerabilities/pagination.py index a014116cb..80007c0e8 100644 --- a/vulnerabilities/pagination.py +++ b/vulnerabilities/pagination.py @@ -12,6 +12,7 @@ class SmallResultSetPagination(PageNumberPagination): page_size_query_param = "page_size" max_page_size = 100 + class PaginatedListViewMixin: """ A mixin that adds pagination functionality to ListView-based views. diff --git a/vulnerabilities/views.py b/vulnerabilities/views.py index 8236635b5..e21be83e8 100644 --- a/vulnerabilities/views.py +++ b/vulnerabilities/views.py @@ -40,7 +40,7 @@ PAGE_SIZE = 20 -class PackageSearch(PaginatedListViewMixin,ListView): +class PackageSearch(PaginatedListViewMixin, ListView): model = models.Package template_name = "packages.html" ordering = ["type", "namespace", "name", "version"] @@ -68,7 +68,7 @@ def get_queryset(self, query=None): ) -class VulnerabilitySearch(PaginatedListViewMixin,ListView): +class VulnerabilitySearch(PaginatedListViewMixin, ListView): model = models.Vulnerability template_name = "vulnerabilities.html" ordering = ["vulnerability_id"] From c75a13432d61275854221425f4d8c7c7c175e7ae Mon Sep 17 00:00:00 2001 From: RISHI GARG <134256793+Rishi-source@users.noreply.github.com> Date: Tue, 18 Mar 2025 10:56:55 +0530 Subject: [PATCH 3/5] Pagination Mixin optimisation and JS code removed Signed-off-by: RISHI GARG <134256793+Rishi-source@users.noreply.github.com> --- vulnerabilities/pagination.py | 123 ++++++++-------- .../templates/includes/pagination.html | 132 ++++++++---------- 2 files changed, 122 insertions(+), 133 deletions(-) diff --git a/vulnerabilities/pagination.py b/vulnerabilities/pagination.py index 80007c0e8..5584fbc25 100644 --- a/vulnerabilities/pagination.py +++ b/vulnerabilities/pagination.py @@ -1,7 +1,7 @@ import logging import re -from django.core.paginator import Paginator +from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator from django.db.models.query import QuerySet from rest_framework.pagination import PageNumberPagination @@ -18,13 +18,18 @@ class PaginatedListViewMixin: A mixin that adds pagination functionality to ListView-based views. """ - paginate_by = 20 + paginate_default = 20 max_page_size = 100 - PAGE_SIZE_CHOICES = [ + page_size_choices = [ {"value": 20, "label": "20 per page"}, {"value": 50, "label": "50 per page"}, {"value": 100, "label": "100 per page"}, ] + + max_pages_without_truncation = 5 # it is a value for number of pages without truncation like is total number of pages are less than this number the pagination will show all pages. + pages_around_current = 2 # number of pages to be shown around current page + truncation_threshold_start = 4 # it is a threshold for start of truncation + truncation_threshold_end = 3 # it is a threshold for end of truncation def get_queryset(self): """ @@ -32,32 +37,37 @@ def get_queryset(self): """ try: queryset = super().get_queryset() - if not queryset: - queryset = self.model.objects.all() - if not isinstance(queryset, QuerySet): - queryset = self.model.objects.all() - return queryset except Exception as e: logger.error(f"Error in get_queryset: {e}") - return self.model.objects.all() + return self.model.objects.none() + + if not queryset or not isinstance(queryset, QuerySet): + queryset = self.model.objects.none() + return queryset def sanitize_page_size(self, raw_page_size): """ Sanitize page size input to prevent XSS and injection attempts. """ if not raw_page_size: - return self.paginate_by - clean_page_size = re.sub(r"\D", "", str(raw_page_size)) + return self.paginate_default + + clean_page_size = re.sub(r"\D", "", str(raw_page_size)) # it remove all non-digit characters like if 50abcd is their then it takes out 50 + if not clean_page_size: + return self.paginate_default + try: - page_size = int(clean_page_size) if clean_page_size else self.paginate_by - valid_sizes = {choice["value"] for choice in self.PAGE_SIZE_CHOICES} - if page_size not in valid_sizes: - logger.warning(f"Attempted to use unauthorized page size: {page_size}") - return self.paginate_by - return page_size + page_size = int(clean_page_size) except (ValueError, TypeError): - logger.info("Empty or invalid page_size input attempted") - return self.paginate_by + logger.info("Invalid page_size input attempted") + return self.paginate_default + + valid_sizes = {choice["value"] for choice in self.page_size_choices} + if page_size not in valid_sizes: + logger.warning(f"Attempted to use unauthorized page size: {page_size}") + return self.paginate_default + + return page_size def get_paginate_by(self, queryset=None): """ @@ -72,57 +82,47 @@ def get_page_range(self, paginator, page_obj): """ num_pages = paginator.num_pages current_page = page_obj.number - if num_pages <= 7: - return list(range(1, num_pages + 1)) - pages = [] - pages.append(1) - if current_page > 4: + if num_pages <= self.max_pages_without_truncation: + return list(map(str, range(1, num_pages + 1))) + pages = [1] + + if current_page > self.truncation_threshold_start: pages.append("...") - start = max(2, current_page - 2) - end = min(num_pages - 1, current_page + 2) + start = max(2, current_page - self.pages_around_current) + end = min(num_pages - 1, current_page + self.pages_around_current) pages.extend(range(start, end + 1)) - if current_page < num_pages - 3: + if current_page < num_pages - self.truncation_threshold_end: pages.append("...") if num_pages > 1: pages.append(num_pages) - return [str(p) for p in pages] + return list(map(str, pages)) def paginate_queryset(self, queryset, page_size): - """ - Robust pagination with comprehensive error handling - """ + if not queryset: + queryset = self.model.objects.none() + paginator = Paginator(queryset, page_size) + try: + page_number = int(self.request.GET.get("page", "1")) + except (ValueError, TypeError): + logger.error("Invalid page number input") + page_number = 1 + page_number = max(1, min(page_number, paginator.num_pages)) try: - if not queryset or queryset.count() == 0: - queryset = self.model.objects.all() - paginator = Paginator(queryset, page_size) - page_params = self.request.GET.getlist("page") - page_number = page_params[-1] if page_params else "1" - try: - page_number = int(re.sub(r"\D", "", str(page_number))) - if not page_number: - page_number = 1 - except (ValueError, TypeError): - page_number = 1 - page_number = max(1, min(page_number, paginator.num_pages)) page = paginator.page(page_number) - return (paginator, page, page.object_list, page.has_other_pages()) - except Exception as e: - logger.error(f"Pagination error: {e}") - queryset = self.model.objects.all() - paginator = Paginator(queryset, page_size) + except (EmptyPage, PageNotAnInteger) as e: + logger.error(f"Specific pagination error: {e}") page = paginator.page(1) - return (paginator, page, page.object_list, page.has_other_pages()) + return (paginator, page, page.object_list, page.has_other_pages()) def get_context_data(self, **kwargs): """ Return a mapping of pagination-related context data, preserving filters. """ - queryset = self.get_queryset() + queryset = kwargs.pop('queryset', None) or self.get_queryset() page_size = self.get_paginate_by() paginator, page, object_list, is_paginated = self.paginate_queryset(queryset, page_size) page_range = self.get_page_range(paginator, page) - search = self.request.GET.get("search", "") context = super().get_context_data( object_list=object_list, @@ -132,15 +132,16 @@ def get_context_data(self, **kwargs): **kwargs, ) - context.update( - { - "current_page_size": page_size, - "page_size_choices": self.PAGE_SIZE_CHOICES, - "total_count": paginator.count, - "page_range": page_range, - "search": search, - "previous_page_url": page.previous_page_number() if page.has_previous() else None, - "next_page_url": page.next_page_number() if page.has_next() else None, - } + previous_page_url = page.previous_page_number() if page.has_previous() else None + next_page_url = page.next_page_number() if page.has_next() else None + context.update({ + "current_page_size": page_size, + "page_size_choices": self.page_size_choices, + "total_count": paginator.count, + "page_range": page_range, + "search": self.request.GET.get("search", ""), + "previous_page_url": previous_page_url, + "next_page_url": next_page_url, + } ) return context diff --git a/vulnerabilities/templates/includes/pagination.html b/vulnerabilities/templates/includes/pagination.html index b2eaa4f3f..f20f19980 100644 --- a/vulnerabilities/templates/includes/pagination.html +++ b/vulnerabilities/templates/includes/pagination.html @@ -1,76 +1,64 @@
-
-
- -
+
+
+ {% if search %} + + {% endif %} +
+ +
+
+
+ + {% if page_obj.paginator.num_pages > 1 %} + + {% endif %}
- - {% if page_obj.paginator.num_pages > 1 %} - - {% endif %} -
- - - - + .select.is-disabled select { + cursor: not-allowed; + } + \ No newline at end of file From 7c2a138e07afaade63a890dcf692762dc7893fd6 Mon Sep 17 00:00:00 2001 From: RISHI GARG <134256793+Rishi-source@users.noreply.github.com> Date: Tue, 18 Mar 2025 11:15:11 +0530 Subject: [PATCH 4/5] Tests are added and code is reformatted via make valid Signed-off-by: RISHI GARG <134256793+Rishi-source@users.noreply.github.com> --- vulnerabilities/pagination.py | 54 +++++----- vulnerabilities/tests/test_pagination.py | 130 +++++++++++++++++++++++ vulnerabilities/tests/test_view.py | 27 ++--- 3 files changed, 174 insertions(+), 37 deletions(-) create mode 100644 vulnerabilities/tests/test_pagination.py diff --git a/vulnerabilities/pagination.py b/vulnerabilities/pagination.py index 5584fbc25..c30840c34 100644 --- a/vulnerabilities/pagination.py +++ b/vulnerabilities/pagination.py @@ -1,7 +1,9 @@ import logging import re -from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator +from django.core.paginator import EmptyPage +from django.core.paginator import PageNotAnInteger +from django.core.paginator import Paginator from django.db.models.query import QuerySet from rest_framework.pagination import PageNumberPagination @@ -25,11 +27,11 @@ class PaginatedListViewMixin: {"value": 50, "label": "50 per page"}, {"value": 100, "label": "100 per page"}, ] - - max_pages_without_truncation = 5 # it is a value for number of pages without truncation like is total number of pages are less than this number the pagination will show all pages. - pages_around_current = 2 # number of pages to be shown around current page - truncation_threshold_start = 4 # it is a threshold for start of truncation - truncation_threshold_end = 3 # it is a threshold for end of truncation + + max_pages_without_truncation = 5 # it is a value for number of pages without truncation like is total number of pages are less than this number the pagination will show all pages. + pages_around_current = 2 # number of pages to be shown around current page + truncation_threshold_start = 4 # it is a threshold for start of truncation + truncation_threshold_end = 3 # it is a threshold for end of truncation def get_queryset(self): """ @@ -40,7 +42,7 @@ def get_queryset(self): except Exception as e: logger.error(f"Error in get_queryset: {e}") return self.model.objects.none() - + if not queryset or not isinstance(queryset, QuerySet): queryset = self.model.objects.none() return queryset @@ -51,22 +53,24 @@ def sanitize_page_size(self, raw_page_size): """ if not raw_page_size: return self.paginate_default - - clean_page_size = re.sub(r"\D", "", str(raw_page_size)) # it remove all non-digit characters like if 50abcd is their then it takes out 50 + + clean_page_size = re.sub( + r"\D", "", str(raw_page_size) + ) # it remove all non-digit characters like if 50abcd is their then it takes out 50 if not clean_page_size: return self.paginate_default - + try: page_size = int(clean_page_size) except (ValueError, TypeError): logger.info("Invalid page_size input attempted") return self.paginate_default - + valid_sizes = {choice["value"] for choice in self.page_size_choices} if page_size not in valid_sizes: logger.warning(f"Attempted to use unauthorized page size: {page_size}") return self.paginate_default - + return page_size def get_paginate_by(self, queryset=None): @@ -85,7 +89,7 @@ def get_page_range(self, paginator, page_obj): if num_pages <= self.max_pages_without_truncation: return list(map(str, range(1, num_pages + 1))) pages = [1] - + if current_page > self.truncation_threshold_start: pages.append("...") start = max(2, current_page - self.pages_around_current) @@ -102,7 +106,7 @@ def paginate_queryset(self, queryset, page_size): queryset = self.model.objects.none() paginator = Paginator(queryset, page_size) try: - page_number = int(self.request.GET.get("page", "1")) + page_number = int(self.request.GET.get("page", "1")) except (ValueError, TypeError): logger.error("Invalid page number input") page_number = 1 @@ -118,12 +122,11 @@ def get_context_data(self, **kwargs): """ Return a mapping of pagination-related context data, preserving filters. """ - queryset = kwargs.pop('queryset', None) or self.get_queryset() + queryset = kwargs.pop("queryset", None) or self.get_queryset() page_size = self.get_paginate_by() paginator, page, object_list, is_paginated = self.paginate_queryset(queryset, page_size) page_range = self.get_page_range(paginator, page) - context = super().get_context_data( object_list=object_list, page_obj=page, @@ -134,14 +137,15 @@ def get_context_data(self, **kwargs): previous_page_url = page.previous_page_number() if page.has_previous() else None next_page_url = page.next_page_number() if page.has_next() else None - context.update({ - "current_page_size": page_size, - "page_size_choices": self.page_size_choices, - "total_count": paginator.count, - "page_range": page_range, - "search": self.request.GET.get("search", ""), - "previous_page_url": previous_page_url, - "next_page_url": next_page_url, - } + context.update( + { + "current_page_size": page_size, + "page_size_choices": self.page_size_choices, + "total_count": paginator.count, + "page_range": page_range, + "search": self.request.GET.get("search", ""), + "previous_page_url": previous_page_url, + "next_page_url": next_page_url, + } ) return context diff --git a/vulnerabilities/tests/test_pagination.py b/vulnerabilities/tests/test_pagination.py new file mode 100644 index 000000000..940635da8 --- /dev/null +++ b/vulnerabilities/tests/test_pagination.py @@ -0,0 +1,130 @@ +# +# Copyright (c) nexB Inc. and others. All rights reserved. +# VulnerableCode is a trademark of nexB Inc. +# SPDX-License-Identifier: Apache-2.0 +# See http://www.apache.org/licenses/LICENSE-2.0 for the license text. +# See https://github.com/aboutcode-org/vulnerablecode for support or download. +# See https://aboutcode.org for more information about nexB OSS projects. +# + +from django.test import TestCase +from django.urls import reverse + +from vulnerabilities.models import Package + + +class PaginationFunctionalityTests(TestCase): + @classmethod + def setUpTestData(cls): + for i in range(150): + Package.objects.create( + type="test", + namespace="test", + name=f"package{i}", + version=str(i), + qualifiers={}, + subpath="", + ) + + def test_default_pagination(self): + response = self.client.get(reverse("package_search")) + self.assertEqual(response.status_code, 200) + page_obj = response.context["page_obj"] + self.assertIsNotNone(page_obj) + self.assertEqual(len(page_obj.object_list), 20) + self.assertEqual(response.context["total_count"], 150) + self.assertEqual(response.context["current_page_size"], 20) + + def test_page_size_variations(self): + valid_page_sizes = [20, 50, 100] + for size in valid_page_sizes: + url = f"{reverse('package_search')}?page_size={size}" + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertIn(response.context["current_page_size"], [20, size]) + + def test_page_navigation(self): + response = self.client.get(reverse("package_search")) + first_page = response.context["page_obj"] + self.assertEqual(first_page.number, 1) + self.assertTrue(first_page.has_next()) + self.assertFalse(first_page.has_previous()) + self.assertGreater(first_page.paginator.num_pages, 1) + + +class PaginationSecurityTests(TestCase): + @classmethod + def setUpTestData(cls): + for i in range(50): + Package.objects.create( + type="test", + namespace="test", + name=f"package{i}", + version=str(i), + qualifiers={}, + subpath="", + ) + + def test_invalid_page_size_inputs(self): + malicious_inputs = [ + "abc", + "-10", + "0", + "9999999999", + "11", + "