UnifiedPush / android-connector

Mirror of https://codeberg.org/UnifiedPush/android-connector/
Apache License 2.0
35 stars 8 forks source link

UnifiedPush preferences can get out of sync, registering silently fails #85

Closed nikclayton closed 3 months ago

nikclayton commented 3 months ago

I don't have a reproduction recipe for this I'm afraid. I discovered this while I was experimenting with adding and removing distributors.

It's possible for the Unified Push preferences to get in to a state where there is an "instance" pref with a token (i.e., "$instance/$PREF_MASTER_TOKEN" exists), and the instance is not listed in the set PREF_MASTER_INSTANCE ("unifiedpush.instances").

This causes all attempts to register with that instance to silently fail.

While troubleshooting why some registrations weren't working I wrote this to log the UP preferences:

    val prefs = context.getSharedPreferences(PREF_MASTER, Context.MODE_PRIVATE)
    prefs.all.forEach {
        Timber.w("UP pref: ${it.key} -> ${it.value}")
    }

and that currently logs:

UP pref: unifiedpush.distributor -> io.heckel.ntfy
UP pref: unifiedpush.instances -> [3]
UP pref: 3/unifiedpush.connector -> a0590926-a962-4691-8479-0a0931b5ec52
UP pref: 4/unifiedpush.connector -> 846254b2-818d-40aa-9062-ff30d48f56af
UP pref: 5/unifiedpush.connector -> e7064b02-2ba3-4408-86e5-e7678f528b54
UP pref: 6/unifiedpush.connector -> d4e79db8-69af-4392-8d0d-3bc49cae9a14

Notice how I have 4 instances but only one of them is listed in PREF_MASTER_INSTANCE.

This then manifests as a failure to register the instance. The flow through the code looks like this (if the preferences are as above), trying to re-register instance "4" (assuming a provider is installed).

  1. App calls UnifiedPush.registerAppWithDialog(context, "4").
  2. UnifiedPush.registerAppWithDialog calls UnifiedPush.registerApp(context, "4", features, messageForDistributor)
  3. UnifiedPush.registerApp calls Store.getTokenOrNew() to get the token to include in the broadcast intent. For instance 4 this returns "846254b2-818d-40aa-9062-ff30d48f56af".
  4. The intent is broadcast.
  5. MessagingReceiver.onReceive() receives the broadcast, and extracts the token
  6. MessagingReceiver.onReceive() then runs val instance = token?.let { store.tryGetInstance(it) } ?: return

And things go wrong.

a. There's no logging when this happens. Troubleshooting this problem would have been much easier if every early return in the Unified Push code had an associated debug or warning log message (in this case, a warning, since receiving a broadcast with an unexpected token is not part of the expected normal operation).

b. Store.tryGetInstance() iterates over the instances in unifiedpush.instances, and checks to see if the instance's token matches the provided token, and returns the instance if so.

This fails, because as already established, the instance is not listed in unifiedpush.instances. getTokenOrNew() could have checked this when it called at step 3.

The end result is the registration silently fails, because MessagingReceiver.onReceive() returns early, does not log anything, and does not signal to the client code that something has gone wrong.

p1gp1g commented 3 months ago

Ok, so there are 3 issues:

p1gp1g commented 3 months ago

https://codeberg.org/UnifiedPush/android-connector/commit/6d0499961a09af011540d1f6d3c17630fbbcc4cb should fix it