PushNotification's devices field is not compatible with Sygnal
#1947
Comments
|
You can send a fake test notification to a device with Sending the following notification body: {
"notification": {
"event_id": "$3957tyerfgewrf384",
"room_id": "!slw48wfj34rtnrf:example.org",
"type": "m.room.message",
"sender": "@exampleuser:example.org",
"sender_display_name": "Major Tom",
"room_name": "Mission Control",
"room_alias": "#exampleroom:example.org",
"prio": "high",
"content": {
"msgtype": "m.text",
"body": "I'm floating in a most peculiar way."
},
"counts": {
"unread": 2,
"missed_calls": 1
},
"devices": [
{
"app_id": "<APP ID HERE>",
"pushkey": "<PUSHKEY HERE>",
"pushkey_ts": 12345678,
"data": {},
"tweaks": {
"sound": "bing"
}
}
]
}
}Results in the following message being transmitted to FCM by Sygnal: {
"sender_display_name": "Major Tom",
"room_name": "Mission Control",
"sender": "@exampleuser:example.org",
"unread": "2",
"missed_calls": "1",
"prio": "high",
"type": "m.room.message",
"event_id": "$3957tyerfgewrf384",
"content_body": "I'm floating in a most peculiar way.",
"room_alias": "#exampleroom:example.org",
"room_id": "!slw48wfj34rtnrf:example.org",
"content_msgtype": "m.text"
}This logic is defined here: https://github.com/matrix-org/sygnal/blob/v0.15.1/sygnal/gcmpushkin.py |
vanyasem
changed the title
PushNotification object is not compatible with SygnalPushNotification's devices field is not compatible with Sygnal
|
An example of an {
"unread": "1",
"missed_calls": null,
"prio": "high",
"event_id": "$3957tyerfgewrf384",
"room_id": "!slw48wfj34rtnrf:example.org"
} |
|
Same with #1948 |
|
@krille-chan the documentation that you linked specifies the JSON model for the API endpoint You misunderstood the meaning of the model linked above. It's a server-side model for the Push Gateway itself. It's not meant to be sent back to users devices
The definition of "reference" is:
You can learn more about the meaning of a "reference implementation" here: https://en.wikipedia.org/wiki/Reference_implementation Arguing that an official reference implementation is somehow wrong, and your specific interpretation of the documentation is right is really bold. Just to be sure, the field in question has been discussed directly with the Matrix team, and they confirmed that Your implementation is the only one that does that, yet you still choose to stand by your incorrect interpretation of the docs. |
|
Btw the correct documentation to link would be https://github.com/matrix-org/sygnal/blob/main/docs/applications.md#firebase-cloud-messaging, which specifies the client-side model for the push: {
"event_id": "$3957tyerfgewrf384",
"type": "m.room.message",
"sender": "@exampleuser:example.org",
"room_name": "Mission Control",
"room_alias": "#exampleroom:example.org",
"sender_display_name": "Major Tom",
"content_msgtype": "m.text",
"content_body": "I'm floating in a most peculiar way.",
"room_id": "!slw48wfj34rtnrf:example.org",
"prio": "high",
"unread": "2",
"missed_calls": "1"
} |
Checklist
In which Project did the bug appear?
matrix-dart-sdkOn which platform did the bug appear?
Android, iOS
SDK Version
0.34.0
Describe the problem caused by this bug
PushNotificationobject inmartix-dart-sdkis not compatible with the reference implementation of a Push Gateway for Matrix called SygnalThe
PushNotificationobject in the SDK has a mandatorydevicesfield: https://github.com/famedly/matrix-dart-sdk/blob/v0.34.0/lib/src/utils/push_notification.dart#L20 which is not sent by SygnalI have only tested the GCM (FCM) implementation of Sygnal, but by reading the appropriate source code, I can confirm that this issue affects APN notifications as well
See matrix-org/sygnal#319 for comments of the maintainer of Sygnal
As far as I could tell, despite being required, it's not used by the SDK in any capacity
I believe the confusion happened because of the following text in the matrix push gateway API spec: https://spec.matrix.org/v1.12/push-gateway-api/
This field is only marked as required for server-side endpoints of the push gateway, and should not be required on the client-side
I made a PR that makes the
devicesfield optional: #1949The text was updated successfully, but these errors were encountered: