search

sk22 / megalodon

Pink modification of the official Mastodon for Android app
https://sk22.github.io/megalodon
GNU General Public License v3.0
551 stars 32 forks source link

Push Notification ID compatibility with upcoming Firefish update #994

Open blueset opened 6 months ago

blueset commented 6 months ago

Describe the bug

A clear and concise description of what the bug is.

I’m a contributor to Firefish, a Misskey fork. I’m currently working on Mastodon API compatibility layer for push notifications in Firefish.

In most Misskey forks, entity IDs are in the form of strings instead of integers. This is fine with the most part of Megalodon and official upstream. However, when processing incoming push notification payloads, the notification ID is parsed as a long instead of string.

https://github.com/sk22/megalodon/blob/f7dfebcbeacb3db182bdc3b01cf7a1d9cea720ca/mastodon/src/main/java/org/joinmastodon/android/model/PushNotification.java#L13-L16

Despite that, the value is then only used once by converting it back to string in a subsequent API call.

https://github.com/sk22/megalodon/blob/f7dfebcbeacb3db182bdc3b01cf7a1d9cea720ca/mastodon/src/main/java/org/joinmastodon/android/PushNotificationReceiver.java#L98-L99

Parsing the notification ID as long would break on Misskey forks with Mastodon API support. I would like to suggest parsing the value as a string directly.

To reproduce

Steps to reproduce the behavior:

  1. Go to an instance with dev version of Firefish fork with Mastodon push notification support (currently only s.1a23.studio, I can provide account if needed)
  2. Receive push notification
  3. Observe that parsing of JSON payload fails due to type mismatch at PushNotification.java:16.

Does this happen in the official app?

Does this issue also occur with the respective upstream release? (Please test using the respective upstream-xxxxxx.apk provided in Releases or at least using the current Mastodon version from the Play Store)

Yes

In case it does, please consider filing an upstream bug report instead. If this bug is seriously impacting your usage or you think I might want to try to fix it for Megalodon, feel free to still create this issue!

Since this is a compatibility issue with third-party server programs, it may fall out of scope of upstream project. I’m filling mastodon/mastodon-android#842 to see if upstream is willing to fix.

Screenshots and screen recordings

If applicable, add screenshots (and screen recordings, if possible) to help explain your problem.

N/A

Version

Megalodon version: [e.g. v1.1.4+fork.#]

v2.1.6+fork.110

Crash log

If you know your way around Android development tools, please consider attaching a crash log, if possible.

N/A

blueset commented 6 months ago

Upstream fixed the issue at mastodon/mastodon-android@b2c797fb464a844059b9002bc9189d3ae6874580.