Skip to content

Commit

Permalink
Fix #767: bound provided limit
Browse files Browse the repository at this point in the history
  • Loading branch information
leplatrem committed Feb 20, 2025
1 parent d00e630 commit 15e9207
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
13 changes: 13 additions & 0 deletions kinto-remote-settings/src/kinto_remote_settings/changes/utils.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,23 @@
import hashlib
from typing import Optional
from uuid import UUID

from kinto.core import utils as core_utils
from pyramid.settings import aslist


def bound_limit(settings: dict, value: Optional[int]) -> int:
"""
``_limit`` querystring value has to be within what is configured as
pagination size and max storage fetching size (respectively defaults
as `None` and 10000 in `kinto.core.DEFAULT_SETTINGS`)
"""
max_fetch_size = settings["storage_max_fetch_size"]
paginate_by = settings["paginate_by"] or max_fetch_size
max_limit = min(paginate_by, max_fetch_size)
return min(abs(value), max_limit) if value is not None else max_limit


def monitored_collections(registry):
storage = registry.storage
resources_uri = aslist(registry.settings.get("changes.resources", ""))
Expand Down
12 changes: 9 additions & 3 deletions kinto-remote-settings/src/kinto_remote_settings/changes/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@
CHANGESET_PATH,
MONITOR_BUCKET,
)
from .utils import changes_object, monitored_collections
from .utils import bound_limit, changes_object, monitored_collections


POSTGRESQL_MAX_INTEGER_VALUE = 2**63
positive_big_integer = colander.Range(min=0, max=POSTGRESQL_MAX_INTEGER_VALUE)


class ChangesModel(object):
Expand Down Expand Up @@ -265,7 +269,9 @@ def deserialize(self, cstruct=colander.null):
class ChangeSetQuerystring(colander.MappingSchema):
_since = QuotedTimestamp(missing=colander.drop)
_expected = colander.SchemaNode(colander.String())
_limit = colander.SchemaNode(colander.Integer(), missing=colander.drop)
_limit = colander.SchemaNode(
colander.Integer(), missing=colander.drop, validator=positive_big_integer
)
# Query parameters used on monitor/changes endpoint.
bucket = colander.SchemaNode(colander.String(), missing=colander.drop)
collection = colander.SchemaNode(colander.String(), missing=colander.drop)
Expand All @@ -285,7 +291,7 @@ def get_changeset(request):
storage = request.registry.storage

queryparams = request.validated["querystring"]
limit = queryparams.get("_limit")
limit = bound_limit(request.registry.settings, queryparams.get("_limit"))
filters = []
include_deleted = False
if "_since" in queryparams:
Expand Down
15 changes: 15 additions & 0 deletions kinto-remote-settings/tests/changes/test_changeset.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,21 @@ def test_limit_is_supported(self):
resp = self.app.get(self.changeset_uri + "&_limit=1", headers=self.headers)
assert len(resp.json["changes"]) == 1

def test_limit_has_to_be_positive(self):
self.app.get(
self.changeset_uri + "&_limit=-1", headers=self.headers, status=400
)
self.app.get(
self.changeset_uri + f"&_limit={2**65}", headers=self.headers, status=400
)

def test_limit_is_bounded(self):
with mock.patch.object(self.app.app.registry.storage, "list_all") as mocked:
self.app.get(self.changeset_uri + "&_limit=42000", headers=self.headers)
mocked.assert_called()
_, kwargs = mocked.call_args
assert kwargs["limit"] == 10_000 # Default in kinto.core

def test_extra_param_is_allowed(self):
self.app.get(self.changeset_uri + "&_extra=abc", headers=self.headers)

Expand Down

0 comments on commit 15e9207

Please sign in to comment.