Tidy up integer parsing (#17339)
The parse_integer function was previously made to reject negative values by default in https://github.com/element-hq/synapse/pull/16920, but the documentation stated otherwise. This fixes the documentation and also: - Removes explicit negative=False parameters from call sites. - Brings the negative default of parse_integer_from_args in alignment with parse_integer.
This commit is contained in:
parent
1e74b50dc6
commit
700d2cc4a0
|
@ -0,0 +1 @@
|
|||
Tidy up `parse_integer` docs and call sites to reflect the fact that they require non-negative integers by default, and bring `parse_integer_from_args` default in alignment. Contributed by Denis Kasak (@dkasak).
|
|
@ -119,14 +119,15 @@ def parse_integer(
|
|||
default: value to use if the parameter is absent, defaults to None.
|
||||
required: whether to raise a 400 SynapseError if the parameter is absent,
|
||||
defaults to False.
|
||||
negative: whether to allow negative integers, defaults to True.
|
||||
negative: whether to allow negative integers, defaults to False (disallowing
|
||||
negatives).
|
||||
Returns:
|
||||
An int value or the default.
|
||||
|
||||
Raises:
|
||||
SynapseError: if the parameter is absent and required, if the
|
||||
parameter is present and not an integer, or if the
|
||||
parameter is illegitimate negative.
|
||||
parameter is illegitimately negative.
|
||||
"""
|
||||
args: Mapping[bytes, Sequence[bytes]] = request.args # type: ignore
|
||||
return parse_integer_from_args(args, name, default, required, negative)
|
||||
|
@ -164,7 +165,7 @@ def parse_integer_from_args(
|
|||
name: str,
|
||||
default: Optional[int] = None,
|
||||
required: bool = False,
|
||||
negative: bool = True,
|
||||
negative: bool = False,
|
||||
) -> Optional[int]:
|
||||
"""Parse an integer parameter from the request string
|
||||
|
||||
|
@ -174,7 +175,8 @@ def parse_integer_from_args(
|
|||
default: value to use if the parameter is absent, defaults to None.
|
||||
required: whether to raise a 400 SynapseError if the parameter is absent,
|
||||
defaults to False.
|
||||
negative: whether to allow negative integers, defaults to True.
|
||||
negative: whether to allow negative integers, defaults to False (disallowing
|
||||
negatives).
|
||||
|
||||
Returns:
|
||||
An int value or the default.
|
||||
|
@ -182,7 +184,7 @@ def parse_integer_from_args(
|
|||
Raises:
|
||||
SynapseError: if the parameter is absent and required, if the
|
||||
parameter is present and not an integer, or if the
|
||||
parameter is illegitimate negative.
|
||||
parameter is illegitimately negative.
|
||||
"""
|
||||
name_bytes = name.encode("ascii")
|
||||
|
||||
|
|
|
@ -61,8 +61,8 @@ class ListDestinationsRestServlet(RestServlet):
|
|||
async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
|
||||
await assert_requester_is_admin(self._auth, request)
|
||||
|
||||
start = parse_integer(request, "from", default=0, negative=False)
|
||||
limit = parse_integer(request, "limit", default=100, negative=False)
|
||||
start = parse_integer(request, "from", default=0)
|
||||
limit = parse_integer(request, "limit", default=100)
|
||||
|
||||
destination = parse_string(request, "destination")
|
||||
|
||||
|
@ -181,8 +181,8 @@ class DestinationMembershipRestServlet(RestServlet):
|
|||
if not await self._store.is_destination_known(destination):
|
||||
raise NotFoundError("Unknown destination")
|
||||
|
||||
start = parse_integer(request, "from", default=0, negative=False)
|
||||
limit = parse_integer(request, "limit", default=100, negative=False)
|
||||
start = parse_integer(request, "from", default=0)
|
||||
limit = parse_integer(request, "limit", default=100)
|
||||
|
||||
direction = parse_enum(request, "dir", Direction, default=Direction.FORWARDS)
|
||||
|
||||
|
|
|
@ -311,8 +311,8 @@ class DeleteMediaByDateSize(RestServlet):
|
|||
) -> Tuple[int, JsonDict]:
|
||||
await assert_requester_is_admin(self.auth, request)
|
||||
|
||||
before_ts = parse_integer(request, "before_ts", required=True, negative=False)
|
||||
size_gt = parse_integer(request, "size_gt", default=0, negative=False)
|
||||
before_ts = parse_integer(request, "before_ts", required=True)
|
||||
size_gt = parse_integer(request, "size_gt", default=0)
|
||||
keep_profiles = parse_boolean(request, "keep_profiles", default=True)
|
||||
|
||||
if before_ts < 30000000000: # Dec 1970 in milliseconds, Aug 2920 in seconds
|
||||
|
@ -377,8 +377,8 @@ class UserMediaRestServlet(RestServlet):
|
|||
if user is None:
|
||||
raise NotFoundError("Unknown user")
|
||||
|
||||
start = parse_integer(request, "from", default=0, negative=False)
|
||||
limit = parse_integer(request, "limit", default=100, negative=False)
|
||||
start = parse_integer(request, "from", default=0)
|
||||
limit = parse_integer(request, "limit", default=100)
|
||||
|
||||
# If neither `order_by` nor `dir` is set, set the default order
|
||||
# to newest media is on top for backward compatibility.
|
||||
|
@ -421,8 +421,8 @@ class UserMediaRestServlet(RestServlet):
|
|||
if user is None:
|
||||
raise NotFoundError("Unknown user")
|
||||
|
||||
start = parse_integer(request, "from", default=0, negative=False)
|
||||
limit = parse_integer(request, "limit", default=100, negative=False)
|
||||
start = parse_integer(request, "from", default=0)
|
||||
limit = parse_integer(request, "limit", default=100)
|
||||
|
||||
# If neither `order_by` nor `dir` is set, set the default order
|
||||
# to newest media is on top for backward compatibility.
|
||||
|
|
|
@ -63,10 +63,10 @@ class UserMediaStatisticsRestServlet(RestServlet):
|
|||
),
|
||||
)
|
||||
|
||||
start = parse_integer(request, "from", default=0, negative=False)
|
||||
limit = parse_integer(request, "limit", default=100, negative=False)
|
||||
from_ts = parse_integer(request, "from_ts", default=0, negative=False)
|
||||
until_ts = parse_integer(request, "until_ts", negative=False)
|
||||
start = parse_integer(request, "from", default=0)
|
||||
limit = parse_integer(request, "limit", default=100)
|
||||
from_ts = parse_integer(request, "from_ts", default=0)
|
||||
until_ts = parse_integer(request, "until_ts")
|
||||
|
||||
if until_ts is not None:
|
||||
if until_ts <= from_ts:
|
||||
|
|
|
@ -90,8 +90,8 @@ class UsersRestServletV2(RestServlet):
|
|||
async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
|
||||
await assert_requester_is_admin(self.auth, request)
|
||||
|
||||
start = parse_integer(request, "from", default=0, negative=False)
|
||||
limit = parse_integer(request, "limit", default=100, negative=False)
|
||||
start = parse_integer(request, "from", default=0)
|
||||
limit = parse_integer(request, "limit", default=100)
|
||||
|
||||
user_id = parse_string(request, "user_id")
|
||||
name = parse_string(request, "name", encoding="utf-8")
|
||||
|
|
|
@ -510,7 +510,7 @@ class PublicRoomListRestServlet(RestServlet):
|
|||
if server:
|
||||
raise e
|
||||
|
||||
limit: Optional[int] = parse_integer(request, "limit", 0, negative=False)
|
||||
limit: Optional[int] = parse_integer(request, "limit", 0)
|
||||
since_token = parse_string(request, "since")
|
||||
|
||||
if limit == 0:
|
||||
|
@ -1430,16 +1430,7 @@ class RoomHierarchyRestServlet(RestServlet):
|
|||
requester = await self._auth.get_user_by_req(request, allow_guest=True)
|
||||
|
||||
max_depth = parse_integer(request, "max_depth")
|
||||
if max_depth is not None and max_depth < 0:
|
||||
raise SynapseError(
|
||||
400, "'max_depth' must be a non-negative integer", Codes.BAD_JSON
|
||||
)
|
||||
|
||||
limit = parse_integer(request, "limit")
|
||||
if limit is not None and limit <= 0:
|
||||
raise SynapseError(
|
||||
400, "'limit' must be a positive integer", Codes.BAD_JSON
|
||||
)
|
||||
|
||||
return 200, await self._room_summary_handler.get_room_hierarchy(
|
||||
requester,
|
||||
|
|
|
@ -75,9 +75,6 @@ class PaginationConfig:
|
|||
raise SynapseError(400, "'to' parameter is invalid")
|
||||
|
||||
limit = parse_integer(request, "limit", default=default_limit)
|
||||
if limit < 0:
|
||||
raise SynapseError(400, "Limit must be 0 or above")
|
||||
|
||||
limit = min(limit, MAX_LIMIT)
|
||||
|
||||
try:
|
||||
|
|
Loading…
Reference in New Issue