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

Push Notification ID compatibility with upcoming Firefish update #994

Open
blueset opened this issue May 19, 2024 · 1 comment
Open

Push Notification ID compatibility with upcoming Firefish update #994

blueset opened this issue May 19, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@blueset
Copy link

blueset commented May 19, 2024

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.

public class PushNotification extends BaseModel{
public String accessToken;
public String preferredLocale;
public long notificationId;

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

PushNotification pn=AccountSessionManager.getInstance().getAccount(accountID).getPushSubscriptionManager().decryptNotification(k, p, s);
new GetNotificationByID(pn.notificationId+"")

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#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
Copy link
Author

blueset commented May 21, 2024

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant