fix: clamp paging on tag listings and the page-derived recent offset
tags/stats, tags/{tag}/prompts and tags/filter parsed limit/offset with a
bare int(), so limit=999999 was passed straight to SQLite and a huge
offset raised OverflowError (500). They now share parse_page_params
(limit 1..500, offset 0..MAX_PAGE_OFFSET, 400 on non-integers), and
/recent clamps the offset it derives from ?page= the same way.
This commit is contained in:
+13
-32
@@ -245,7 +245,7 @@ class PromptRoutesMixin:
|
||||
return bad_request("limit, offset and page must be integers")
|
||||
|
||||
if page > 1 and offset == 0:
|
||||
offset = (page - 1) * limit
|
||||
offset = min((page - 1) * limit, MAX_PAGE_OFFSET)
|
||||
|
||||
# Validated against a whitelist in the database layer
|
||||
sort = request.query.get("sort") or None
|
||||
@@ -318,13 +318,9 @@ class PromptRoutesMixin:
|
||||
"""Get tags with usage counts, search, sort, and pagination."""
|
||||
try:
|
||||
try:
|
||||
limit = int(request.query.get("limit", 50))
|
||||
offset = int(request.query.get("offset", 0))
|
||||
except (ValueError, TypeError):
|
||||
return web.json_response(
|
||||
{"success": False, "error": "Invalid limit or offset parameter"},
|
||||
status=400,
|
||||
)
|
||||
limit, offset = parse_page_params(request.query)
|
||||
except ValueError:
|
||||
return bad_request("limit and offset must be integers")
|
||||
search = request.query.get("search", "").strip() or None
|
||||
sort = request.query.get("sort", "alpha_asc")
|
||||
|
||||
@@ -364,13 +360,9 @@ class PromptRoutesMixin:
|
||||
)
|
||||
|
||||
try:
|
||||
limit = int(request.query.get("limit", 20))
|
||||
offset = int(request.query.get("offset", 0))
|
||||
except (ValueError, TypeError):
|
||||
return web.json_response(
|
||||
{"success": False, "error": "Invalid limit or offset parameter"},
|
||||
status=400,
|
||||
)
|
||||
limit, offset = parse_page_params(request.query, default_limit=20)
|
||||
except ValueError:
|
||||
return bad_request("limit and offset must be integers")
|
||||
|
||||
result = await self._run_in_executor(
|
||||
self.db.get_prompts_by_tags, [tag_name], "and", limit, offset
|
||||
@@ -401,16 +393,9 @@ class PromptRoutesMixin:
|
||||
|
||||
if untagged:
|
||||
try:
|
||||
limit = int(request.query.get("limit", 20))
|
||||
offset = int(request.query.get("offset", 0))
|
||||
except (ValueError, TypeError):
|
||||
return web.json_response(
|
||||
{
|
||||
"success": False,
|
||||
"error": "Invalid limit or offset parameter",
|
||||
},
|
||||
status=400,
|
||||
)
|
||||
limit, offset = parse_page_params(request.query, default_limit=20)
|
||||
except ValueError:
|
||||
return bad_request("limit and offset must be integers")
|
||||
result = await self._run_in_executor(
|
||||
self.db.get_untagged_prompts, limit, offset
|
||||
)
|
||||
@@ -442,13 +427,9 @@ class PromptRoutesMixin:
|
||||
mode = "and"
|
||||
|
||||
try:
|
||||
limit = int(request.query.get("limit", 20))
|
||||
offset = int(request.query.get("offset", 0))
|
||||
except (ValueError, TypeError):
|
||||
return web.json_response(
|
||||
{"success": False, "error": "Invalid limit or offset parameter"},
|
||||
status=400,
|
||||
)
|
||||
limit, offset = parse_page_params(request.query, default_limit=20)
|
||||
except ValueError:
|
||||
return bad_request("limit and offset must be integers")
|
||||
|
||||
result = await self._run_in_executor(
|
||||
self.db.get_prompts_by_tags, tags_list, mode, limit, offset
|
||||
|
||||
@@ -187,6 +187,16 @@ class TestRecentBounds(PromptAPITestCase):
|
||||
self.assertEqual(data["pagination"]["offset"], 4)
|
||||
self.assertEqual(len(data["results"]), 1)
|
||||
|
||||
async def test_huge_page_number_is_clamped_not_500(self):
|
||||
resp = await self.client.request(
|
||||
"GET", f"/prompt_manager/recent?limit={MAX_PAGE_LIMIT}&page={10**30}"
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status, 200)
|
||||
data = await resp.json()
|
||||
self.assertEqual(data["pagination"]["offset"], MAX_PAGE_OFFSET)
|
||||
self.assertEqual(data["results"], [])
|
||||
|
||||
|
||||
def _raise(*_args, **_kwargs):
|
||||
raise RuntimeError("boom")
|
||||
@@ -323,6 +333,51 @@ class TestTagStatsAndFilters(RouteCoverageCase):
|
||||
self.assertEqual((bad_status, err_status), (400, 500))
|
||||
|
||||
|
||||
class TestTagRoutePagingBounds(RouteCoverageCase):
|
||||
"""Tag listings clamp limit/offset like every other list endpoint."""
|
||||
|
||||
ROUTES = (
|
||||
"/prompt_manager/tags/stats",
|
||||
"/prompt_manager/tags/sky/prompts",
|
||||
"/prompt_manager/tags/filter?tags=sky",
|
||||
"/prompt_manager/tags/filter?untagged=true",
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _with(route, query):
|
||||
return f"{route}{'&' if '?' in route else '?'}{query}"
|
||||
|
||||
async def test_limit_is_clamped_to_max_page_limit(self):
|
||||
for route in self.ROUTES:
|
||||
status, data = await self._json("GET", self._with(route, "limit=999999"))
|
||||
self.assertEqual(status, 200, route)
|
||||
self.assertEqual(data["pagination"]["limit"], MAX_PAGE_LIMIT, route)
|
||||
|
||||
async def test_zero_limit_becomes_one(self):
|
||||
for route in self.ROUTES:
|
||||
_, data = await self._json("GET", self._with(route, "limit=0"))
|
||||
self.assertEqual(data["pagination"]["limit"], 1, route)
|
||||
|
||||
async def test_negative_offset_is_clamped_to_zero(self):
|
||||
for route in self.ROUTES:
|
||||
_, data = await self._json("GET", self._with(route, "offset=-7"))
|
||||
self.assertEqual(data["pagination"]["offset"], 0, route)
|
||||
|
||||
async def test_huge_offset_is_clamped_not_500(self):
|
||||
for route in self.ROUTES:
|
||||
status, data = await self._json(
|
||||
"GET", self._with(route, f"offset={10**30}")
|
||||
)
|
||||
self.assertEqual(status, 200, route)
|
||||
self.assertEqual(data["pagination"]["offset"], MAX_PAGE_OFFSET, route)
|
||||
|
||||
async def test_non_integer_values_are_400(self):
|
||||
for route in self.ROUTES:
|
||||
status, data = await self._json("GET", self._with(route, "limit=1.5"))
|
||||
self.assertEqual(status, 400, route)
|
||||
self.assertFalse(data["success"], route)
|
||||
|
||||
|
||||
class TestTagMutationRoutes(RouteCoverageCase):
|
||||
|
||||
async def test_rename_rejects_bad_json_and_empty_name(self):
|
||||
|
||||
Reference in New Issue
Block a user