Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Fix bundling aggregations if unsigned is not a returned event field. (#…
Browse files Browse the repository at this point in the history
…12234)

An error occured if a filter was supplied with `event_fields` which did not include
`unsigned`.

In that case, bundled aggregations are still added as the spec states it is allowed
for servers to add additional fields.
  • Loading branch information
clokep authored Mar 16, 2022
1 parent 9e90d64 commit 9627456
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog.d/12234.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a long-standing bug when a `filter` argument with `event_fields` supplied but not including the `unsigned` field could result in a 500 error on `/sync`.
9 changes: 6 additions & 3 deletions synapse/events/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,9 +530,12 @@ def _inject_bundled_aggregations(

# Include the bundled aggregations in the event.
if serialized_aggregations:
serialized_event["unsigned"].setdefault("m.relations", {}).update(
serialized_aggregations
)
# There is likely already an "unsigned" field, but a filter might
# have stripped it off (via the event_fields option). The server is
# allowed to return additional fields, so add it back.
serialized_event.setdefault("unsigned", {}).setdefault(
"m.relations", {}
).update(serialized_aggregations)

def serialize_events(
self,
Expand Down
28 changes: 28 additions & 0 deletions tests/rest/client/test_relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,34 @@ def test_background_update(self) -> None:
[annotation_event_id_good, thread_event_id],
)

def test_bundled_aggregations_with_filter(self) -> None:
"""
If "unsigned" is an omitted field (due to filtering), adding the bundled
aggregations should not break.
Note that the spec allows for a server to return additional fields beyond
what is specified.
"""
self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a")

# Note that the sync filter does not include "unsigned" as a field.
filter = urllib.parse.quote_plus(
b'{"event_fields": ["content", "event_id"], "room": {"timeline": {"limit": 3}}}'
)
channel = self.make_request(
"GET", f"/sync?filter={filter}", access_token=self.user_token
)
self.assertEqual(200, channel.code, channel.json_body)

# Ensure the timeline is limited, find the parent event.
room_timeline = channel.json_body["rooms"]["join"][self.room]["timeline"]
self.assertTrue(room_timeline["limited"])
parent_event = self._find_event_in_chunk(room_timeline["events"])

# Ensure there's bundled aggregations on it.
self.assertIn("unsigned", parent_event)
self.assertIn("m.relations", parent_event["unsigned"])


class RelationRedactionTestCase(BaseRelationsTestCase):
"""
Expand Down

0 comments on commit 9627456

Please sign in to comment.