From 26db9ea57b2261c2b5c4dcb4ea37507ad31b6dba Mon Sep 17 00:00:00 2001 From: Manish Gupta Date: Thu, 27 Aug 2026 10:48:38 +0530 Subject: [PATCH] [SECUR-236] fix: reject per_page=0 in get_per_page, not just negatives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The negative-per_page guard added earlier in this PR let per_page=0 through unchanged. A zero value still reaches OffsetPaginator.get_result(), where math.ceil(count / limit) divides by the limit and raises an unhandled ZeroDivisionError (HTTP 500) — the same unhandled-crash class this PR closes for negative values, just with a different trigger. Tighten the guard to per_page <= 0 and correct the regression test that previously asserted 0 was accepted. Co-authored-by: Plane AI --- .../plane/tests/unit/utils/test_paginator.py | 26 +++++++++---------- apps/api/plane/utils/paginator.py | 17 +++++++----- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/apps/api/plane/tests/unit/utils/test_paginator.py b/apps/api/plane/tests/unit/utils/test_paginator.py index 623a71f893..cb67b07c60 100644 --- a/apps/api/plane/tests/unit/utils/test_paginator.py +++ b/apps/api/plane/tests/unit/utils/test_paginator.py @@ -176,8 +176,9 @@ class TestCursorBounds: @pytest.mark.unit -class TestGetPerPageNegativeRejected: - """A negative per_page must be rejected, not just a too-large one. +class TestGetPerPageNonPositiveRejected: + """A non-positive per_page (negative or zero) must be rejected, not just a + too-large one. OffsetPaginator.get_result() slices the queryset with queryset[offset : offset + limit]; get_per_page() previously only checked @@ -186,16 +187,21 @@ class TestGetPerPageNegativeRejected: unhandled "Negative indexing is not supported" -> HTTP 500. This is the same crash class TestCursorBounds closes for the cursor-driven grouped paginators, reachable here on every non-grouped paginated endpoint via a plain query - param.""" + param. - @pytest.mark.parametrize("per_page", [-1, -50, -1000]) - def test_negative_per_page_rejected_by_get_per_page(self, per_page): + A per_page of exactly 0 has the same failure mode: it reaches + OffsetPaginator.get_result() with limit=0, where math.ceil(count / limit) + raises an unhandled ZeroDivisionError -> HTTP 500.""" + + @pytest.mark.parametrize("per_page", [-1, -50, -1000, 0]) + def test_non_positive_per_page_rejected_by_get_per_page(self, per_page): request = _make_request(per_page=str(per_page)) with pytest.raises(ParseError): BasePaginator().get_per_page(request, default_per_page=20, max_per_page=1000) - def test_negative_per_page_rejected_before_paginator_runs(self): - request = _make_request(per_page="-1") + @pytest.mark.parametrize("per_page", ["-1", "0"]) + def test_non_positive_per_page_rejected_before_paginator_runs(self, per_page): + request = _make_request(per_page=per_page) with pytest.raises(ParseError): BasePaginator().paginate( request=request, @@ -204,9 +210,3 @@ class TestGetPerPageNegativeRejected: default_per_page=20, max_per_page=1000, ) - - def test_zero_per_page_is_still_allowed(self): - # 0 is a valid (if degenerate) page size, not a negative one — must not - # be rejected by this guard. - request = _make_request(per_page="0") - assert BasePaginator().get_per_page(request, default_per_page=20, max_per_page=1000) == 0 diff --git a/apps/api/plane/utils/paginator.py b/apps/api/plane/utils/paginator.py index 63ae874d7a..21bb43f1a5 100644 --- a/apps/api/plane/utils/paginator.py +++ b/apps/api/plane/utils/paginator.py @@ -647,13 +647,16 @@ class BasePaginator: raise ParseError(detail="Invalid per_page parameter.") max_per_page = self._effective_max_per_page(max_per_page, default_per_page) - # A negative per_page reaches OffsetPaginator.get_result() unmodified and - # produces a negative slice bound (queryset[offset : offset + per_page]), - # which raises Django's unhandled "Negative indexing is not supported" - # (HTTP 500) — the same crash class the cursor-bound check below closes, - # just reachable on every paginated endpoint via a plain query param. - if per_page < 0: - raise ParseError(detail="Invalid per_page value. Cannot be negative.") + # A non-positive per_page reaches OffsetPaginator.get_result() unmodified. + # A negative value produces a negative slice bound + # (queryset[offset : offset + per_page]), which raises Django's unhandled + # "Negative indexing is not supported" (HTTP 500) — the same crash class + # the cursor-bound check below closes, just reachable on every paginated + # endpoint via a plain query param. A zero value reaches + # math.ceil(count / limit) with limit=0 and raises an unhandled + # ZeroDivisionError (HTTP 500) instead. + if per_page <= 0: + raise ParseError(detail="Invalid per_page value. Must be greater than zero.") if per_page > max_per_page: raise ParseError(detail=f"Invalid per_page value. Cannot exceed {max_per_page}.")