Skip to content

Commit f0d6f14

Browse files
TrevisGordanTrevisGordan
authored
Parse Integer negative value validation (#16920)
1 parent 3a196b3 commit f0d6f14

File tree

9 files changed

+89
-158
lines changed

9 files changed

+89
-158
lines changed

‎changelog.d/16920.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Adds validation to ensure that the `limit` parameter on `/publicRooms` is non-negative.

‎synapse/http/servlet.py

Lines changed: 65 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919
#
2020
#
2121

22-
""" This module contains base REST classes for constructing REST servlets. """
22+
"""This module contains base REST classes for constructing REST servlets."""
23+
2324
import enum
2425
import logging
2526
from http import HTTPStatus
@@ -65,17 +66,49 @@ def parse_integer(request: Request, name: str, default: int) -> int: ...
6566

6667

6768
@overload
68-
def parse_integer(request: Request, name: str, *, required: Literal[True]) -> int: ...
69+
def parse_integer(
70+
request: Request, name: str, *, default: int, negative: bool
71+
) -> int: ...
72+
73+
74+
@overload
75+
def parse_integer(
76+
request: Request, name: str, *, default: int, negative: bool = False
77+
) -> int: ...
78+
79+
80+
@overload
81+
def parse_integer(
82+
request: Request, name: str, *, required: Literal[True], negative: bool = False
83+
) -> int: ...
84+
85+
86+
@overload
87+
def parse_integer(
88+
request: Request, name: str, *, default: Literal[None], negative: bool = False
89+
) -> None: ...
90+
91+
92+
@overload
93+
def parse_integer(request: Request, name: str, *, negative: bool) -> Optional[int]: ...
6994

7095

7196
@overload
7297
def parse_integer(
73-
request: Request, name: str, default: Optional[int] = None, required: bool = False
98+
request: Request,
99+
name: str,
100+
default: Optional[int] = None,
101+
required: bool = False,
102+
negative: bool = False,
74103
) -> Optional[int]: ...
75104

76105

77106
def parse_integer(
78-
request: Request, name: str, default: Optional[int] = None, required: bool = False
107+
request: Request,
108+
name: str,
109+
default: Optional[int] = None,
110+
required: bool = False,
111+
negative: bool = False,
79112
) -> Optional[int]:
80113
"""Parse an integer parameter from the request string
81114
@@ -85,16 +118,17 @@ def parse_integer(
85118
default: value to use if the parameter is absent, defaults to None.
86119
required: whether to raise a 400 SynapseError if the parameter is absent,
87120
defaults to False.
88-
121+
negative: whether to allow negative integers, defaults to True.
89122
Returns:
90123
An int value or the default.
91124
92125
Raises:
93-
SynapseError: if the parameter is absent and required, or if the
94-
parameter is present and not an integer.
126+
SynapseError: if the parameter is absent and required, if the
127+
parameter is present and not an integer, or if the
128+
parameter is illegitimate negative.
95129
"""
96130
args: Mapping[bytes, Sequence[bytes]] = request.args # type: ignore
97-
return parse_integer_from_args(args, name, default, required)
131+
return parse_integer_from_args(args, name, default, required, negative)
98132

99133

100134
@overload
@@ -120,6 +154,7 @@ def parse_integer_from_args(
120154
name: str,
121155
default: Optional[int] = None,
122156
required: bool = False,
157+
negative: bool = False,
123158
) -> Optional[int]: ...
124159

125160

@@ -128,6 +163,7 @@ def parse_integer_from_args(
128163
name: str,
129164
default: Optional[int] = None,
130165
required: bool = False,
166+
negative: bool = True,
131167
) -> Optional[int]:
132168
"""Parse an integer parameter from the request string
133169
@@ -137,33 +173,37 @@ def parse_integer_from_args(
137173
default: value to use if the parameter is absent, defaults to None.
138174
required: whether to raise a 400 SynapseError if the parameter is absent,
139175
defaults to False.
176+
negative: whether to allow negative integers, defaults to True.
140177
141178
Returns:
142179
An int value or the default.
143180
144181
Raises:
145-
SynapseError: if the parameter is absent and required, or if the
146-
parameter is present and not an integer.
182+
SynapseError: if the parameter is absent and required, if the
183+
parameter is present and not an integer, or if the
184+
parameter is illegitimate negative.
147185
"""
148186
name_bytes = name.encode("ascii")
149187

150-
if name_bytes in args:
151-
try:
152-
return int(args[name_bytes][0])
153-
except Exception:
154-
message = "Query parameter %r must be an integer" % (name,)
155-
raise SynapseError(
156-
HTTPStatus.BAD_REQUEST, message, errcode=Codes.INVALID_PARAM
157-
)
158-
else:
159-
if required:
160-
message = "Missing integer query parameter %r" % (name,)
161-
raise SynapseError(
162-
HTTPStatus.BAD_REQUEST, message, errcode=Codes.MISSING_PARAM
163-
)
164-
else:
188+
if name_bytes not in args:
189+
if not required:
165190
return default
166191

192+
message = f"Missing required integer query parameter {name}"
193+
raise SynapseError(HTTPStatus.BAD_REQUEST, message, errcode=Codes.MISSING_PARAM)
194+
195+
try:
196+
integer = int(args[name_bytes][0])
197+
except Exception:
198+
message = f"Query parameter {name} must be an integer"
199+
raise SynapseError(HTTPStatus.BAD_REQUEST, message, errcode=Codes.INVALID_PARAM)
200+
201+
if not negative and integer < 0:
202+
message = f"Query parameter {name} must be a positive integer."
203+
raise SynapseError(HTTPStatus.BAD_REQUEST, message, errcode=Codes.INVALID_PARAM)
204+
205+
return integer
206+
167207

168208
@overload
169209
def parse_boolean(request: Request, name: str, default: bool) -> bool: ...

‎synapse/rest/admin/federation.py

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from typing import TYPE_CHECKING, Tuple
2424

2525
from synapse.api.constants import Direction
26-
from synapse.api.errors import Codes, NotFoundError, SynapseError
26+
from synapse.api.errors import NotFoundError, SynapseError
2727
from synapse.federation.transport.server import Authenticator
2828
from synapse.http.servlet import RestServlet, parse_enum, parse_integer, parse_string
2929
from synapse.http.site import SynapseRequest
@@ -61,22 +61,8 @@ def __init__(self, hs: "HomeServer"):
6161
async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
6262
await assert_requester_is_admin(self._auth, request)
6363

64-
start = parse_integer(request, "from", default=0)
65-
limit = parse_integer(request, "limit", default=100)
66-
67-
if start < 0:
68-
raise SynapseError(
69-
HTTPStatus.BAD_REQUEST,
70-
"Query parameter from must be a string representing a positive integer.",
71-
errcode=Codes.INVALID_PARAM,
72-
)
73-
74-
if limit < 0:
75-
raise SynapseError(
76-
HTTPStatus.BAD_REQUEST,
77-
"Query parameter limit must be a string representing a positive integer.",
78-
errcode=Codes.INVALID_PARAM,
79-
)
64+
start = parse_integer(request, "from", default=0, negative=False)
65+
limit = parse_integer(request, "limit", default=100, negative=False)
8066

8167
destination = parse_string(request, "destination")
8268

@@ -195,22 +181,8 @@ async def on_GET(
195181
if not await self._store.is_destination_known(destination):
196182
raise NotFoundError("Unknown destination")
197183

198-
start = parse_integer(request, "from", default=0)
199-
limit = parse_integer(request, "limit", default=100)
200-
201-
if start < 0:
202-
raise SynapseError(
203-
HTTPStatus.BAD_REQUEST,
204-
"Query parameter from must be a string representing a positive integer.",
205-
errcode=Codes.INVALID_PARAM,
206-
)
207-
208-
if limit < 0:
209-
raise SynapseError(
210-
HTTPStatus.BAD_REQUEST,
211-
"Query parameter limit must be a string representing a positive integer.",
212-
errcode=Codes.INVALID_PARAM,
213-
)
184+
start = parse_integer(request, "from", default=0, negative=False)
185+
limit = parse_integer(request, "limit", default=100, negative=False)
214186

215187
direction = parse_enum(request, "dir", Direction, default=Direction.FORWARDS)
216188

‎synapse/rest/admin/media.py

Lines changed: 7 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -311,29 +311,17 @@ async def on_POST(
311311
) -> Tuple[int, JsonDict]:
312312
await assert_requester_is_admin(self.auth, request)
313313

314-
before_ts = parse_integer(request, "before_ts", required=True)
315-
size_gt = parse_integer(request, "size_gt", default=0)
314+
before_ts = parse_integer(request, "before_ts", required=True, negative=False)
315+
size_gt = parse_integer(request, "size_gt", default=0, negative=False)
316316
keep_profiles = parse_boolean(request, "keep_profiles", default=True)
317317

318-
if before_ts < 0:
319-
raise SynapseError(
320-
HTTPStatus.BAD_REQUEST,
321-
"Query parameter before_ts must be a positive integer.",
322-
errcode=Codes.INVALID_PARAM,
323-
)
324-
elif before_ts < 30000000000: # Dec 1970 in milliseconds, Aug 2920 in seconds
318+
if before_ts < 30000000000: # Dec 1970 in milliseconds, Aug 2920 in seconds
325319
raise SynapseError(
326320
HTTPStatus.BAD_REQUEST,
327321
"Query parameter before_ts you provided is from the year 1970. "
328322
+ "Double check that you are providing a timestamp in milliseconds.",
329323
errcode=Codes.INVALID_PARAM,
330324
)
331-
if size_gt < 0:
332-
raise SynapseError(
333-
HTTPStatus.BAD_REQUEST,
334-
"Query parameter size_gt must be a string representing a positive integer.",
335-
errcode=Codes.INVALID_PARAM,
336-
)
337325

338326
# This check is useless, we keep it for the legacy endpoint only.
339327
if server_name is not None and self.server_name != server_name:
@@ -389,22 +377,8 @@ async def on_GET(
389377
if user is None:
390378
raise NotFoundError("Unknown user")
391379

392-
start = parse_integer(request, "from", default=0)
393-
limit = parse_integer(request, "limit", default=100)
394-
395-
if start < 0:
396-
raise SynapseError(
397-
HTTPStatus.BAD_REQUEST,
398-
"Query parameter from must be a string representing a positive integer.",
399-
errcode=Codes.INVALID_PARAM,
400-
)
401-
402-
if limit < 0:
403-
raise SynapseError(
404-
HTTPStatus.BAD_REQUEST,
405-
"Query parameter limit must be a string representing a positive integer.",
406-
errcode=Codes.INVALID_PARAM,
407-
)
380+
start = parse_integer(request, "from", default=0, negative=False)
381+
limit = parse_integer(request, "limit", default=100, negative=False)
408382

409383
# If neither `order_by` nor `dir` is set, set the default order
410384
# to newest media is on top for backward compatibility.
@@ -447,22 +421,8 @@ async def on_DELETE(
447421
if user is None:
448422
raise NotFoundError("Unknown user")
449423

450-
start = parse_integer(request, "from", default=0)
451-
limit = parse_integer(request, "limit", default=100)
452-
453-
if start < 0:
454-
raise SynapseError(
455-
HTTPStatus.BAD_REQUEST,
456-
"Query parameter from must be a string representing a positive integer.",
457-
errcode=Codes.INVALID_PARAM,
458-
)
459-
460-
if limit < 0:
461-
raise SynapseError(
462-
HTTPStatus.BAD_REQUEST,
463-
"Query parameter limit must be a string representing a positive integer.",
464-
errcode=Codes.INVALID_PARAM,
465-
)
424+
start = parse_integer(request, "from", default=0, negative=False)
425+
limit = parse_integer(request, "limit", default=100, negative=False)
466426

467427
# If neither `order_by` nor `dir` is set, set the default order
468428
# to newest media is on top for backward compatibility.

‎synapse/rest/admin/statistics.py

Lines changed: 4 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -63,38 +63,12 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
6363
),
6464
)
6565

66-
start = parse_integer(request, "from", default=0)
67-
if start < 0:
68-
raise SynapseError(
69-
HTTPStatus.BAD_REQUEST,
70-
"Query parameter from must be a string representing a positive integer.",
71-
errcode=Codes.INVALID_PARAM,
72-
)
73-
74-
limit = parse_integer(request, "limit", default=100)
75-
if limit < 0:
76-
raise SynapseError(
77-
HTTPStatus.BAD_REQUEST,
78-
"Query parameter limit must be a string representing a positive integer.",
79-
errcode=Codes.INVALID_PARAM,
80-
)
66+
start = parse_integer(request, "from", default=0, negative=False)
67+
limit = parse_integer(request, "limit", default=100, negative=False)
68+
from_ts = parse_integer(request, "from_ts", default=0, negative=False)
69+
until_ts = parse_integer(request, "until_ts", negative=False)
8170

82-
from_ts = parse_integer(request, "from_ts", default=0)
83-
if from_ts < 0:
84-
raise SynapseError(
85-
HTTPStatus.BAD_REQUEST,
86-
"Query parameter from_ts must be a string representing a positive integer.",
87-
errcode=Codes.INVALID_PARAM,
88-
)
89-
90-
until_ts = parse_integer(request, "until_ts")
9171
if until_ts is not None:
92-
if until_ts < 0:
93-
raise SynapseError(
94-
HTTPStatus.BAD_REQUEST,
95-
"Query parameter until_ts must be a string representing a positive integer.",
96-
errcode=Codes.INVALID_PARAM,
97-
)
9872
if until_ts <= from_ts:
9973
raise SynapseError(
10074
HTTPStatus.BAD_REQUEST,

‎synapse/rest/admin/users.py

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -90,22 +90,8 @@ def __init__(self, hs: "HomeServer"):
9090
async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
9191
await assert_requester_is_admin(self.auth, request)
9292

93-
start = parse_integer(request, "from", default=0)
94-
limit = parse_integer(request, "limit", default=100)
95-
96-
if start < 0:
97-
raise SynapseError(
98-
HTTPStatus.BAD_REQUEST,
99-
"Query parameter from must be a string representing a positive integer.",
100-
errcode=Codes.INVALID_PARAM,
101-
)
102-
103-
if limit < 0:
104-
raise SynapseError(
105-
HTTPStatus.BAD_REQUEST,
106-
"Query parameter limit must be a string representing a positive integer.",
107-
errcode=Codes.INVALID_PARAM,
108-
)
93+
start = parse_integer(request, "from", default=0, negative=False)
94+
limit = parse_integer(request, "limit", default=100, negative=False)
10995

11096
user_id = parse_string(request, "user_id")
11197
name = parse_string(request, "name", encoding="utf-8")

‎synapse/rest/client/room.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
499499
if server:
500500
raise e
501501

502-
limit: Optional[int] = parse_integer(request, "limit", 0)
502+
limit: Optional[int] = parse_integer(request, "limit", 0, negative=False)
503503
since_token = parse_string(request, "since")
504504

505505
if limit == 0:

‎synapse/rest/media/preview_url_resource.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,6 @@ async def on_GET(self, request: SynapseRequest) -> None:
7272
# XXX: if get_user_by_req fails, what should we do in an async render?
7373
requester = await self.auth.get_user_by_req(request)
7474
url = parse_string(request, "url", required=True)
75-
ts = parse_integer(request, "ts")
76-
if ts is None:
77-
ts = self.clock.time_msec()
78-
75+
ts = parse_integer(request, "ts", default=self.clock.time_msec())
7976
og = await self.url_previewer.preview(url, requester.user, ts)
8077
respond_with_json_bytes(request, 200, og, send_cors=True)

0 commit comments

Comments
 (0)