diff --git a/changelog.d/20350.misc b/changelog.d/20350.misc new file mode 100644 index 0000000000..5eb9ec959c --- /dev/null +++ b/changelog.d/20350.misc @@ -0,0 +1 @@ +Require a strictly positive stickiness duration for [MSC4354 Sticky Events](https://github.com/matrix-org/matrix-spec-proposals/pull/4354). \ No newline at end of file diff --git a/rust/src/events/mod.rs b/rust/src/events/mod.rs index 2f519d8dcb..ad45e29414 100644 --- a/rust/src/events/mod.rs +++ b/rust/src/events/mod.rs @@ -434,9 +434,13 @@ impl Event { }; // Check for a valid duration field. The MSC requires `duration_ms` to - // be a non-negative integer. If it's missing or invalid, we treat the + // be a strictly positive integer. If it's missing or invalid, we treat the // event as non-sticky by returning `None`. - let Some(duration_ms) = sticky_obj.get("duration_ms").and_then(|v| v.as_u64()) else { + let Some(duration_ms) = sticky_obj + .get("duration_ms") + .and_then(|v| v.as_u64()) + .filter(|duration| *duration > 0) + else { return Ok(None); }; diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 160628ef16..b5dcc8902e 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -445,6 +445,12 @@ class RoomSendEventRestServlet(TransactionRestServlet): sticky_duration_ms: int | None = None if self._msc4354_enabled: sticky_duration_ms = parse_integer(request, StickyEvent.REQUEST_PARAM_NAME) + if sticky_duration_ms is not None and sticky_duration_ms <= 0: + raise SynapseError( + HTTPStatus.BAD_REQUEST, + f"{StickyEvent.REQUEST_PARAM_NAME} must be strictly positive", + Codes.INVALID_PARAM, + ) # FIXME(MSC4140): remove by 2026-12-31, once RoomDelayedEventRestServlet has been stable for a suitable amount of time. delay = _parse_request_for_delayed_event_delay(request, self._msc4140_enabled) @@ -598,6 +604,13 @@ class RoomDelayedEventRestServlet(TransactionRestServlet): Codes.INVALID_PARAM, ) + if sticky_duration_ms is not None and sticky_duration_ms <= 0: + raise SynapseError( + HTTPStatus.BAD_REQUEST, + f"{StickyEvent.REQUEST_PARAM_NAME} must be strictly positive", + Codes.INVALID_PARAM, + ) + if request_body.state_key is not Absent: if sticky_duration_ms is not None: raise SynapseError( diff --git a/tests/rest/client/test_delayed_events.py b/tests/rest/client/test_delayed_events.py index 9bb7e22f7f..5261863827 100644 --- a/tests/rest/client/test_delayed_events.py +++ b/tests/rest/client/test_delayed_events.py @@ -1397,20 +1397,12 @@ class DelayedStickyEventsTestCase(DelayedEventsTestCaseBase): self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) @parameterized.expand( - ( - (sticky_duration, param_in_body) - for sticky_duration in ( - Duration(), - Duration(seconds=-2000), - Duration(days=1), - ) - for param_in_body in (False, True) - ) + ((Duration(days=1), param_in_body) for param_in_body in (False, True)) ) def test_delayed_sticky_event_with_extreme_duration( self, sticky_duration: Duration, param_in_body: bool ) -> None: - """Test that delayed event requests permit a zero, negative, or very long sticky duration. + """Test that delayed event requests permit a very long sticky duration. (The actual sticky duration is clamped at event send time.)""" path = self.helper.build_delayed_event_request_path( room_id=self.room_id, @@ -1429,6 +1421,36 @@ class DelayedStickyEventsTestCase(DelayedEventsTestCaseBase): delay_id = channel.json_body.get("delay_id") assert delay_id is not None + @parameterized.expand( + ( + (sticky_duration, param_in_body) + for sticky_duration in ( + Duration(), + Duration(seconds=-2000), + ) + for param_in_body in (False, True) + ) + ) + def test_delayed_sticky_event_require_positive_duration( + self, sticky_duration: Duration, param_in_body: bool + ) -> None: + """Test that delayed event requests forbid a zero or negative sticky duration.""" + path = self.helper.build_delayed_event_request_path( + room_id=self.room_id, + event_type=_EVENT_TYPE, + ) + body = self.helper.build_delayed_event_request_body( + delay=Duration(milliseconds=900), + content={"body": "sticky"}, + ) + if param_in_body: + body[StickyEvent.REQUEST_PARAM_NAME] = sticky_duration.as_millis() + else: + path += f"?{StickyEvent.REQUEST_PARAM_NAME}={sticky_duration.as_millis()}" + channel = self.make_request("PUT", path, body, self.user1_access_token) + self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result) + self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM) + def test_delayed_sticky_event_with_conflicting_sticky_durations(self) -> None: """Test that delayed event requests reject having a different sticky duration in the query parameter & request body."""