Compare commits

...

6 Commits

Author SHA1 Message Date
Denis Kasak
1b599ccb73 Fix newsfile. 2024-06-20 17:19:28 +02:00
Denis Kasak
0c67c41b7b Remove one more instance of dead validation code. 2024-06-20 17:00:40 +02:00
Denis Kasak
47a5799b58 Add newsfile. 2024-06-20 16:57:04 +02:00
Denis Kasak
a5f61b044b Remove dead validation code from client /hierarchy handler. 2024-06-20 16:07:22 +02:00
Denis Kasak
fd8c9aa1e2 Clarify parse_integer* docs. 2024-06-20 16:06:50 +02:00
Denis Kasak
d43f6df650 Fix documentation for parse_integer regarding negatives.
The parse_integer was previously made to reject negative values by
default, 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.
2024-06-20 16:02:57 +02:00
8 changed files with 25 additions and 34 deletions

1
changelog.d/17339.misc Normal file
View File

@@ -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).

View File

@@ -119,14 +119,15 @@ def parse_integer(
default: value to use if the parameter is absent, defaults to None. default: value to use if the parameter is absent, defaults to None.
required: whether to raise a 400 SynapseError if the parameter is absent, required: whether to raise a 400 SynapseError if the parameter is absent,
defaults to False. defaults to False.
negative: whether to allow negative integers, defaults to True. negative: whether to allow negative integers, defaults to False (disallowing
negatives).
Returns: Returns:
An int value or the default. An int value or the default.
Raises: Raises:
SynapseError: if the parameter is absent and required, if the SynapseError: if the parameter is absent and required, if the
parameter is present and not an integer, or 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 args: Mapping[bytes, Sequence[bytes]] = request.args # type: ignore
return parse_integer_from_args(args, name, default, required, negative) return parse_integer_from_args(args, name, default, required, negative)
@@ -164,7 +165,7 @@ def parse_integer_from_args(
name: str, name: str,
default: Optional[int] = None, default: Optional[int] = None,
required: bool = False, required: bool = False,
negative: bool = True, negative: bool = False,
) -> Optional[int]: ) -> Optional[int]:
"""Parse an integer parameter from the request string """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. default: value to use if the parameter is absent, defaults to None.
required: whether to raise a 400 SynapseError if the parameter is absent, required: whether to raise a 400 SynapseError if the parameter is absent,
defaults to False. defaults to False.
negative: whether to allow negative integers, defaults to True. negative: whether to allow negative integers, defaults to False (disallowing
negatives).
Returns: Returns:
An int value or the default. An int value or the default.
@@ -182,7 +184,7 @@ def parse_integer_from_args(
Raises: Raises:
SynapseError: if the parameter is absent and required, if the SynapseError: if the parameter is absent and required, if the
parameter is present and not an integer, or 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") name_bytes = name.encode("ascii")

View File

@@ -61,8 +61,8 @@ class ListDestinationsRestServlet(RestServlet):
async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
await assert_requester_is_admin(self._auth, request) await assert_requester_is_admin(self._auth, request)
start = parse_integer(request, "from", default=0, negative=False) start = parse_integer(request, "from", default=0)
limit = parse_integer(request, "limit", default=100, negative=False) limit = parse_integer(request, "limit", default=100)
destination = parse_string(request, "destination") destination = parse_string(request, "destination")
@@ -181,8 +181,8 @@ class DestinationMembershipRestServlet(RestServlet):
if not await self._store.is_destination_known(destination): if not await self._store.is_destination_known(destination):
raise NotFoundError("Unknown destination") raise NotFoundError("Unknown destination")
start = parse_integer(request, "from", default=0, negative=False) start = parse_integer(request, "from", default=0)
limit = parse_integer(request, "limit", default=100, negative=False) limit = parse_integer(request, "limit", default=100)
direction = parse_enum(request, "dir", Direction, default=Direction.FORWARDS) direction = parse_enum(request, "dir", Direction, default=Direction.FORWARDS)

View File

@@ -311,8 +311,8 @@ class DeleteMediaByDateSize(RestServlet):
) -> Tuple[int, JsonDict]: ) -> Tuple[int, JsonDict]:
await assert_requester_is_admin(self.auth, request) await assert_requester_is_admin(self.auth, request)
before_ts = parse_integer(request, "before_ts", required=True, negative=False) before_ts = parse_integer(request, "before_ts", required=True)
size_gt = parse_integer(request, "size_gt", default=0, negative=False) size_gt = parse_integer(request, "size_gt", default=0)
keep_profiles = parse_boolean(request, "keep_profiles", default=True) keep_profiles = parse_boolean(request, "keep_profiles", default=True)
if before_ts < 30000000000: # Dec 1970 in milliseconds, Aug 2920 in seconds if before_ts < 30000000000: # Dec 1970 in milliseconds, Aug 2920 in seconds
@@ -377,8 +377,8 @@ class UserMediaRestServlet(RestServlet):
if user is None: if user is None:
raise NotFoundError("Unknown user") raise NotFoundError("Unknown user")
start = parse_integer(request, "from", default=0, negative=False) start = parse_integer(request, "from", default=0)
limit = parse_integer(request, "limit", default=100, negative=False) limit = parse_integer(request, "limit", default=100)
# If neither `order_by` nor `dir` is set, set the default order # If neither `order_by` nor `dir` is set, set the default order
# to newest media is on top for backward compatibility. # to newest media is on top for backward compatibility.
@@ -421,8 +421,8 @@ class UserMediaRestServlet(RestServlet):
if user is None: if user is None:
raise NotFoundError("Unknown user") raise NotFoundError("Unknown user")
start = parse_integer(request, "from", default=0, negative=False) start = parse_integer(request, "from", default=0)
limit = parse_integer(request, "limit", default=100, negative=False) limit = parse_integer(request, "limit", default=100)
# If neither `order_by` nor `dir` is set, set the default order # If neither `order_by` nor `dir` is set, set the default order
# to newest media is on top for backward compatibility. # to newest media is on top for backward compatibility.

View File

@@ -63,10 +63,10 @@ class UserMediaStatisticsRestServlet(RestServlet):
), ),
) )
start = parse_integer(request, "from", default=0, negative=False) start = parse_integer(request, "from", default=0)
limit = parse_integer(request, "limit", default=100, negative=False) limit = parse_integer(request, "limit", default=100)
from_ts = parse_integer(request, "from_ts", default=0, negative=False) from_ts = parse_integer(request, "from_ts", default=0)
until_ts = parse_integer(request, "until_ts", negative=False) until_ts = parse_integer(request, "until_ts")
if until_ts is not None: if until_ts is not None:
if until_ts <= from_ts: if until_ts <= from_ts:

View File

@@ -90,8 +90,8 @@ class UsersRestServletV2(RestServlet):
async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
await assert_requester_is_admin(self.auth, request) await assert_requester_is_admin(self.auth, request)
start = parse_integer(request, "from", default=0, negative=False) start = parse_integer(request, "from", default=0)
limit = parse_integer(request, "limit", default=100, negative=False) limit = parse_integer(request, "limit", default=100)
user_id = parse_string(request, "user_id") user_id = parse_string(request, "user_id")
name = parse_string(request, "name", encoding="utf-8") name = parse_string(request, "name", encoding="utf-8")

View File

@@ -510,7 +510,7 @@ class PublicRoomListRestServlet(RestServlet):
if server: if server:
raise e 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") since_token = parse_string(request, "since")
if limit == 0: if limit == 0:
@@ -1430,16 +1430,7 @@ class RoomHierarchyRestServlet(RestServlet):
requester = await self._auth.get_user_by_req(request, allow_guest=True) requester = await self._auth.get_user_by_req(request, allow_guest=True)
max_depth = parse_integer(request, "max_depth") 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") 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( return 200, await self._room_summary_handler.get_room_hierarchy(
requester, requester,

View File

@@ -75,9 +75,6 @@ class PaginationConfig:
raise SynapseError(400, "'to' parameter is invalid") raise SynapseError(400, "'to' parameter is invalid")
limit = parse_integer(request, "limit", default=default_limit) 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) limit = min(limit, MAX_LIMIT)
try: try: