Skip to content

Commit bc47484

Browse files
jmartinespanoadragon453
jmartinesp
and
anoadragon453
authored
Prevent sending empty badge-only notifications in FCM (#424)
Co-authored-by: Andrew Morgan <[email protected]>
1 parent 54826e8 commit bc47484

File tree

3 files changed

+219
-15
lines changed

3 files changed

+219
-15
lines changed

‎changelog.d/424.misc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Make sure no empty badge-only notifications are sent if there are no badge counts in FCM.

‎sygnal/gcmpushkin.py‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,9 @@ async def _dispatch_notification_unlimited(
551551
"please ensure that default_payload is a dict."
552552
)
553553
return pushkeys
554+
elif len(data) == 0:
555+
log.warning("Discarding notification since it contains no data.")
556+
return []
554557

555558
headers = {
556559
"User-Agent": ["sygnal"],
@@ -667,7 +670,9 @@ def _build_data(
667670
send_badge_counts: If set to `True`, will send the unread and missed_call counts.
668671
669672
Returns:
670-
JSON-compatible dict or None if the default_payload is misconfigured
673+
JSON-compatible dict. The dict will be empty if the payload was
674+
discarded. In this case, the notification should be ignored. Can
675+
instead be `None` if the `default_payload` is misconfigured.
671676
"""
672677
data = {}
673678
overflow_fields = 0
@@ -694,17 +699,19 @@ def _build_data(
694699
"room_id",
695700
]:
696701
if hasattr(n, attr):
697-
data[attr] = getattr(n, attr)
702+
current = getattr(n, attr)
703+
if current is None:
704+
continue
698705
# Truncate fields to a sensible maximum length. If the whole
699706
# body is too long, GCM will reject it.
700-
if data[attr] is not None and isinstance(data[attr], str):
707+
if current is not None and isinstance(current, str):
701708
# The only `attr` that shouldn't be of type `str` is `content`,
702709
# which is handled explicitly later on.
703-
data[attr], truncated = truncate_str(
704-
data[attr], MAX_BYTES_PER_FIELD
705-
)
710+
data[attr], truncated = truncate_str(current, MAX_BYTES_PER_FIELD)
706711
if truncated:
707712
overflow_fields += 1
713+
elif attr == "content" and current is not None:
714+
data["content"] = current
708715

709716
if api_version is APIVersion.V1:
710717
if isinstance(data.get("content"), dict):
@@ -721,15 +728,28 @@ def _build_data(
721728
if n.prio == "low":
722729
data["prio"] = "normal"
723730

731+
counts: Dict[str, Any] = {}
724732
if send_badge_counts and getattr(n, "counts", None):
725733
if api_version is APIVersion.Legacy:
726-
data["unread"] = n.counts.unread
727-
data["missed_calls"] = n.counts.missed_calls
734+
if n.counts.unread:
735+
counts["unread"] = n.counts.unread
736+
if n.counts.missed_calls:
737+
counts["missed_calls"] = n.counts.missed_calls
728738
elif api_version is APIVersion.V1:
729739
if n.counts.unread:
730-
data["unread"] = str(n.counts.unread)
740+
counts["unread"] = str(n.counts.unread)
731741
if n.counts.missed_calls:
732-
data["missed_calls"] = str(n.counts.missed_calls)
742+
counts["missed_calls"] = str(n.counts.missed_calls)
743+
744+
if not data.get("room_id") and not data.get("event_id") and not counts:
745+
logger.debug(
746+
"Notification is badge-only but contains no badge count values. Discarding data. "
747+
+ "If this is not intended, check the `send_badge_counts` parameter in the config."
748+
)
749+
return {}
750+
751+
# Add count data to the payload data
752+
data.update(counts)
733753

734754
if overflow_fields > MAX_NOTIFICATION_OVERFLOW_FIELDS:
735755
logger.warning(

‎tests/test_gcm.py‎

Lines changed: 188 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,32 @@ def config_setup(self, config: Dict[str, Any]) -> None:
196196
},
197197
}
198198

199+
config["apps"]["com.example.gcm.apiv1.disabled-badges"] = {
200+
"type": "tests.test_gcm.TestGcmPushkin",
201+
"api_version": "v1",
202+
"project_id": "example_project",
203+
"service_account_file": self.service_account_file.name,
204+
"send_badge_counts": False,
205+
"fcm_options": {
206+
"android": {
207+
"notification": {
208+
"body": {
209+
"test body",
210+
},
211+
},
212+
},
213+
"apns": {
214+
"payload": {
215+
"aps": {
216+
"content-available": 1,
217+
"mutable-content": 1,
218+
"alert": "",
219+
},
220+
},
221+
},
222+
},
223+
}
224+
199225
def tearDown(self) -> None:
200226
self.service_account_file.close()
201227

@@ -235,7 +261,6 @@ def test_expected(self) -> None:
235261
"sender": "@exampleuser:matrix.org",
236262
"room_name": "Mission Control",
237263
"room_alias": "#exampleroom:matrix.org",
238-
"membership": None,
239264
"sender_display_name": "Major Tom",
240265
"content": {
241266
"msgtype": "m.text",
@@ -284,7 +309,6 @@ def test_expected_api_v1(self) -> None:
284309
"sender": "@exampleuser:matrix.org",
285310
"room_name": "Mission Control",
286311
"room_alias": "#exampleroom:matrix.org",
287-
"membership": None,
288312
"sender_display_name": "Major Tom",
289313
"content_msgtype": "m.text",
290314
"content_body": "I'm floating in a most peculiar way.",
@@ -500,7 +524,6 @@ async def side_effect(*_args: Any, **_kwargs: Any) -> None:
500524
"sender": "@exampleuser:matrix.org",
501525
"room_name": "Mission Control",
502526
"room_alias": "#exampleroom:matrix.org",
503-
"membership": None,
504527
"sender_display_name": "Major Tom",
505528
"content_msgtype": "m.text",
506529
"content_body": "I'm floating in a most peculiar way.",
@@ -584,7 +607,6 @@ def test_send_badge_counts(self) -> None:
584607
"other": 1,
585608
},
586609
"event_id": "$qTOWWTEL48yPm3uT-gdNhFcoHxfKbZuqRVnnWWSkGBs",
587-
"membership": None,
588610
"prio": "high",
589611
"room_alias": "#exampleroom:matrix.org",
590612
"room_id": "!slw48wfj34rtnrf:example.com",
@@ -595,6 +617,168 @@ def test_send_badge_counts(self) -> None:
595617
},
596618
)
597619

620+
def test_send_badge_counts_api_v1(self) -> None:
621+
"""
622+
Tests that the config option `send_badge_counts` being disabled in API v1
623+
actually removes the unread and missed call count from notifications
624+
"""
625+
gcm = self.get_test_pushkin("com.example.gcm.apiv1.disabled-badges")
626+
gcm.preload_with_response(
627+
200, {"results": [{"registration_id": "spqr_new", "message_id": "msg42"}]}
628+
)
629+
630+
resp = self._request(
631+
self._make_dummy_notification(
632+
[
633+
{
634+
"app_id": "com.example.gcm.apiv1.disabled-badges",
635+
"pushkey": "spqr",
636+
"pushkey_ts": 42,
637+
}
638+
]
639+
)
640+
)
641+
642+
self.assertEqual(resp, {"rejected": []})
643+
assert gcm.last_request_body is not None
644+
645+
# No 'unread' or 'missed_call' fields are present
646+
self.assertEqual(
647+
gcm.last_request_body["message"]["data"],
648+
{
649+
"content_body": "I'm floating in a most peculiar way.",
650+
"content_msgtype": "m.text",
651+
"event_id": "$qTOWWTEL48yPm3uT-gdNhFcoHxfKbZuqRVnnWWSkGBs",
652+
"prio": "high",
653+
"room_alias": "#exampleroom:matrix.org",
654+
"room_id": "!slw48wfj34rtnrf:example.com",
655+
"room_name": "Mission Control",
656+
"sender": "@exampleuser:matrix.org",
657+
"sender_display_name": "Major Tom",
658+
"type": "m.room.message",
659+
},
660+
)
661+
662+
def test_send_badge_counts_with_badge_only_notification(self) -> None:
663+
"""
664+
Tests that the config option `send_badge_counts` being disabled
665+
prevents badge only notifications from being sent at all.
666+
"""
667+
gcm = self.get_test_pushkin("fcm-with-disabled-badge-count")
668+
gcm.preload_with_response(
669+
200, {"results": [{"registration_id": "spqr_new", "message_id": "msg42"}]}
670+
)
671+
672+
resp = self._request(
673+
self._make_dummy_notification_badge_only(
674+
[
675+
{
676+
"app_id": "fcm-with-disabled-badge-count",
677+
"pushkey": "spqr",
678+
"pushkey_ts": 42,
679+
}
680+
]
681+
)
682+
)
683+
684+
self.assertEqual(resp, {"rejected": []})
685+
self.assertEqual(gcm.num_requests, 0)
686+
687+
def test_send_badge_counts_with_badge_only_notification_api_v1(self) -> None:
688+
"""
689+
Tests that the config option `send_badge_counts` being disabled
690+
in API v1 prevents badge only notifications from being sent at all.
691+
"""
692+
gcm = self.get_test_pushkin("com.example.gcm.apiv1.disabled-badges")
693+
gcm.preload_with_response(
694+
200, {"results": [{"registration_id": "spqr_new", "message_id": "msg42"}]}
695+
)
696+
697+
resp = self._request(
698+
self._make_dummy_notification_badge_only(
699+
[
700+
{
701+
"app_id": "com.example.gcm.apiv1.disabled-badges",
702+
"pushkey": "spqr",
703+
"pushkey_ts": 42,
704+
}
705+
]
706+
)
707+
)
708+
709+
self.assertEqual(resp, {"rejected": []})
710+
self.assertEqual(gcm.num_requests, 0)
711+
712+
def test_send_badge_counts_with_event_id_only_notification(self) -> None:
713+
"""
714+
Tests that the config option `send_badge_counts` being disabled
715+
actually removes the unread and missed call count from notifications
716+
"""
717+
gcm = self.get_test_pushkin("fcm-with-disabled-badge-count")
718+
gcm.preload_with_response(
719+
200, {"results": [{"registration_id": "spqr_new", "message_id": "msg42"}]}
720+
)
721+
722+
resp = self._request(
723+
self._make_dummy_notification_event_id_only(
724+
[
725+
{
726+
"app_id": "fcm-with-disabled-badge-count",
727+
"pushkey": "spqr",
728+
"pushkey_ts": 42,
729+
}
730+
]
731+
)
732+
)
733+
734+
self.assertEqual(resp, {"rejected": []})
735+
assert gcm.last_request_body is not None
736+
737+
# No 'unread' or 'missed_call' fields are present
738+
self.assertEqual(
739+
gcm.last_request_body["data"],
740+
{
741+
"event_id": "$qTOWWTEL48yPm3uT-gdNhFcoHxfKbZuqRVnnWWSkGBs",
742+
"prio": "high",
743+
"room_id": "!slw48wfj34rtnrf:example.com",
744+
},
745+
)
746+
747+
def test_send_badge_counts_with_event_id_only_notification_api_v1(self) -> None:
748+
"""
749+
Tests that the config option `send_badge_counts` being disabled in API v1
750+
actually removes the unread and missed call count from notifications
751+
"""
752+
gcm = self.get_test_pushkin("com.example.gcm.apiv1.disabled-badges")
753+
gcm.preload_with_response(
754+
200, {"results": [{"registration_id": "spqr_new", "message_id": "msg42"}]}
755+
)
756+
757+
resp = self._request(
758+
self._make_dummy_notification_event_id_only(
759+
[
760+
{
761+
"app_id": "com.example.gcm.apiv1.disabled-badges",
762+
"pushkey": "spqr",
763+
"pushkey_ts": 42,
764+
}
765+
]
766+
)
767+
)
768+
769+
self.assertEqual(resp, {"rejected": []})
770+
assert gcm.last_request_body is not None
771+
772+
# No 'unread' or 'missed_call' fields are present
773+
self.assertEqual(
774+
gcm.last_request_body["message"]["data"],
775+
{
776+
"event_id": "$qTOWWTEL48yPm3uT-gdNhFcoHxfKbZuqRVnnWWSkGBs",
777+
"prio": "high",
778+
"room_id": "!slw48wfj34rtnrf:example.com",
779+
},
780+
)
781+
598782
def test_api_v1_large_fields(self) -> None:
599783
"""
600784
Tests the gcm pushkin truncates unusually large fields. Includes large
@@ -644,7 +828,6 @@ def test_api_v1_large_fields(self) -> None:
644828
xxxxxxxxxxooooooooooxxxxxxxxxxooooooooooxxxxxxxxxxooooooooooxxxxxxxxxx\
645829
ooooooooooxxxxxxxxxxooooooooooxxxxxxxxxxoooooooooox…",
646830
"room_alias": "#exampleroom:matrix.org",
647-
"membership": None,
648831
"sender_display_name": "Major Tom",
649832
"content_msgtype": "m.text",
650833
"content_body": "I'm floating in a most peculiar way.",

0 commit comments

Comments
 (0)