Limit page depth of searches

api/Pipfile

@@ -7,6 +7,7 @@ verify_ssl = true
 ipython = "*"
 pycodestyle = "*"
 pytest-django = ">=3.5"
+pytest-raises = "*"
 remote-pdb = "*"
 pre-commit = "*"
 locust = "*"

api/catalog/api/controllers/search_controller.py

@@ -18,6 +18,7 @@
 
 import catalog.api.models as models
 from catalog.api.utils.dead_link_mask import get_query_hash, get_query_mask
+from catalog.api.utils.pagination import MAX_TOTAL_PAGE_COUNT
 from catalog.api.utils.validate_images import validate_images
 
 
@@ -427,8 +428,7 @@ def _get_result_and_page_count(
     natural_page_count = int(result_count / page_size)
     if natural_page_count % page_size != 0:
         natural_page_count += 1
-    last_allowed_page = int((5000 + page_size / 2) / page_size)
-    page_count = min(natural_page_count, last_allowed_page)
+    page_count = min(natural_page_count, MAX_TOTAL_PAGE_COUNT)
     if len(results) < page_size and page_count == 0:
         result_count = len(results)
 

api/catalog/api/utils/pagination.py

@@ -4,6 +4,9 @@
 from catalog.api.utils.exceptions import get_api_exception
 
 
+MAX_TOTAL_PAGE_COUNT = 20
+
+
 class StandardPagination(PageNumberPagination):
     page_size_query_param = "page_size"
     page_query_param = "page"
@@ -42,6 +45,8 @@ def page(self, value):
         value = int(value)  # convert str params to int
         if value <= 0:
             raise get_api_exception("Page must be greater than 0.", 400)
+        elif value > 20:
+            raise get_api_exception("Searches are limited to 20 pages.", 400)
         self._page = value
 
     def get_paginated_response(self, data):

api/test/dead_link_filter_test.py

@@ -4,6 +4,8 @@
 import pytest
 import requests
 
+from catalog.api.utils.pagination import MAX_TOTAL_PAGE_COUNT
+
 
 def _patch_redis():
     def redis_mget(keys, *_, **__):
@@ -115,7 +117,7 @@ def test_page_consistency_removing_dead_links(search_without_dead_links):
     Test the results returned in consecutive pages are never repeated when
     filtering out dead links.
     """
-    total_pages = 30
+    total_pages = MAX_TOTAL_PAGE_COUNT
     page_size = 5
 
     page_results = []
@@ -134,3 +136,11 @@ def no_duplicates(xs):
     ids = list(map(lambda x: x["id"], page_results))
     # No results should be repeated so we should have no duplicate ids
     assert no_duplicates(ids)
+
+
+@pytest.mark.django_db
+def test_max_page_count():
+    response = requests.get(
+        f"{API_URL}/v1/images", params={"page": MAX_TOTAL_PAGE_COUNT + 1}, verify=False
+    )
+    assert response.status_code == 400

api/test/unit/controllers/test_search_controller.py

@@ -0,0 +1,51 @@
+from unittest import mock
+
+import pytest
+
+from catalog.api.controllers import search_controller
+from catalog.api.utils.pagination import MAX_TOTAL_PAGE_COUNT
+
+
+@pytest.mark.parametrize(
+    "total_hits, real_result_count, page_size, expected",
+    [
+        # No results
+        (0, 0, 10, (0, 0)),
+        # Setting page size to 0 raises an exception
+        pytest.param(
+            0, 0, 0, (0, 0), marks=pytest.mark.raises(exception=ZeroDivisionError)
+        ),
+        # Fewer results than page size leads to max of result total
+        (5, 5, 10, (5, 0)),
+        # Even if no real results exist, total result count and page count are returned
+        # (seems like an impossible case IRL)
+        (100, 0, 10, (100, 10)),
+        # If there are real results and ES reports no hits, nothing is expected
+        # (seems like an impossible case IRL)
+        (0, 100, 10, (0, 0)),
+        # Evenly divisible number of pages
+        (25, 5, 5, (25, 5)),
+        # Unevenly divisible number of pages
+        (21, 5, 5, (21, 5)),
+        # My assumption would be that this yields (20, 4), but the code is such that
+        # when the "natural" page count can't be cleanly divisible by the page size,
+        # We increment it plus one. Why would that be the case? 20 results, with 5
+        # results per-page, would seem to result in 4 pages total not 5 🤷‍♀️
+        (20, 5, 5, (20, 5)),
+        # Fewer hits than page size, but result list somehow differs, use that for count
+        (48, 20, 50, (20, 0)),
+        # Page count gets truncated always
+        (5000, 10, 10, (5000, MAX_TOTAL_PAGE_COUNT)),
+    ],
+)
+def test_get_result_and_page_count(total_hits, real_result_count, page_size, expected):
+    response_obj = mock.MagicMock()
+    response_obj.hits.total.value = total_hits
+    results = [mock.MagicMock() for _ in range(real_result_count)]
+
+    actual = search_controller._get_result_and_page_count(
+        response_obj,
+        results,
+        page_size,
+    )
+    assert actual == expected