Update UnifiedPush library #4358
Conversation
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4358 +/- ##
===========================================
- Coverage 80.40% 80.40% -0.01%
===========================================
Files 2142 2142
Lines 56642 56642
Branches 7081 7081
===========================================
- Hits 45545 45544 -1
Misses 8679 8679
- Partials 2418 2419 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // Duplicate classes between | ||
| // tink-1.16.0.jar -> tink-1.16.0 (com.google.crypto.tink:tink:1.16.0) | ||
| // tink-android-1.8.0.jar tink-android-1.8.0 (com.google.crypto.tink:tink-android:1.8.0) | ||
| exclude(group = "com.google.crypto.tink", module = "tink") |
There was a problem hiding this comment.
Hello @p1gp1g , do you know why I need to add this line?
It seems to be related to https://github.com/UnifiedPush/android-connector/blob/cf347ae931ea8b6e6c773cd566abea940d1297f1/connector/build.gradle#L58 and the related PR tink-crypto/tink-java-apps#5, but maybe I miss something?
There was a problem hiding this comment.
Since version 3.0.0, the lib is able to decrypt webpush messages:
- When a new endpoint comes, you receive a PushEndpoint that contains p256dh and auth secrets that can be send to the server, so the server can encrypt your push notifications following RFC8291.
- When a new message comes, you receive a PushMessage, if the library has correctly decrypted the push message with the webpush keys,
contentis the cleartext anddecryptedis true.
At this moment, matrix doesn't officially support webpush messages. So for the moment, decrypted will always be false. There is a MSC to add webpush pusher. As mentioned in the MSC, some web clients have support for webpush with a gateway by hacking around the specifications. Once this MSC supported, you can even push to FCM servers without any gateway, because FCM servers can be requested with webpush requests (so Android won't need sygnal with servers supporting this MSC, this will probably help in some cases).
The library uses tink library to decrypt the push messages, and I think you have another library using it too. So you end up with duplicate classes. Maybe you should explicitly add tink as a dependency and exclude it from other library using it as well
There was a problem hiding this comment.
Thanks for the fast and complete response. Maybe the unified push library should use tink-android and not tink and that would fix the conflicts? (even if I agree that we have another dependency (not checked which one yet) which has a dependency on an outdated version 1.8.0).
There was a problem hiding this comment.
You would have the same issue if 2 dependencies rely on tink-android, isn't it ?
I have opened an issue to migrate to tink-android, there are some pro/con to do the migration there
There was a problem hiding this comment.
(even if I agree that we have another dependency (not checked which one yet) which has a dependency on an outdated version 1.8.0)
I think it's this one:
androidx.security:security-crypto:1.1.0-alpha06
+--- androidx.annotation:annotation:1.1.0 -> 1.9.1 (*)
+--- androidx.annotation:annotation:1.2.0 -> 1.9.1 (*)
+--- androidx.collection:collection:1.1.0 -> 1.4.4 (*)
\--- com.google.crypto.tink:tink-android:1.8.0
... which is deprecated but we use to encrypted the session data. Yay 🥲 .
There was a problem hiding this comment.
Related: https://stackoverflow.com/questions/78362124 ...
|
I am testing that the upgrade do not break anything, and sadly I do not receive anymore push from ntfy, even when not using a matrix.org account. The troubleshoot fails at the "Test push loopback": 
Logs: Seems related: binwiederhier/ntfy#649 |
|
This may be because your ntfy didn't have any registration when it has received elemen-x request. And ntfy is a bit buggy during this state. This would be fixed with binwiederhier/ntfy-android#97 You can try Sunup, it will use the default gateway (matrix.gateway.unifiedpush.org) or create a first registration on ntfy before doing the tests |
|
Testing again this morning, now the troubleshoot tests are OK, but I still do not receive push from the homeserver. When switching to Firebase it's working. |
|
MSC4173 would probably help here. The server may have rejected the push key due to the previous 507 error You can try:
|
|
I selfhost both ntfy and synapse I built ntfy-android with binwiederhier/ntfy-android#97 and binwiederhier/ntfy-android#98
|
|
Is there anything else missing or that I can test ? Been running the patch for a month without issues on notification receipts so on my clueless knowledge it seems ready to go. |
| // Exclude package com.google.crypto.tink | ||
| // Duplicate classes between | ||
| // tink-1.16.0.jar -> tink-1.16.0 (com.google.crypto.tink:tink:1.16.0) | ||
| // tink-android-1.8.0.jar tink-android-1.8.0 (com.google.crypto.tink:tink-android:1.8.0) | ||
| exclude(group = "com.google.crypto.tink", module = "tink") |
There was a problem hiding this comment.
I think this has to be removed now the dependency with the transitivity dependency to tink is gone.
There was a problem hiding this comment.
it seems to be still necessary
There was a problem hiding this comment.
Weird, it wouldn't let me build the app locally with this exclusion 🫤 .
|
The current changes don't seem to work reliably on my end, sadly (element.io account).
This device is charging and with the screen on, so Doze should not apply. The server is The logs display a couple of timeouts, but otherwise just something like: When a timeout happens, the service reconnects but no new notifications arrive. I might try with the changes proposed by @Eskuero above. |
Can you try with Sunup, or with another ntfy server (eg. https://ntfy.adminforge.de/), and double check that you have allowed background usage without restrictions for ntfy |
|
With Sunup the notifications arrive, but they're severely delayed (30s) and for some reason they appear twice in the list of notifications 😓 . With NTFY + the alternative server, I had the same problemas as before: some notifications being delayed, most not arriving at all. I also couldn't build a custom version of NTFY with the patches mentioned above, the versions are quite outdated and every time I fix a problem another one appears... |
|
I have the app already built with the patches in my fdroid repo If you would rather give another try at building the clean patchfile to apply is also here: And also is true that probably the ntfy.sh instance is putting some ratelimits which might harm testing. If you want I can give you access to my ntfy instance. Ping me at @Eskuero:matrix.fromshado.ws |
This is probably not the best place to debug setups. There are a lot of reasons why a notification could be delayed (depending on the load on the server, if the client have history to fetch, if it can get the decryption keys, etc.). It also depends on your OS configuration, if it is in doze mode etc. When doing tests, you may figure out that sometimes the web client get its notification after the android client, sometimes the opposite. You 'd see the same thing with FCM too |
This reverts commit f431ebe.
bmarty
force-pushed
the
feature/bma/upUpdate
branch
from
60a982b to
57283a8
Compare
|
Can you try with version 3.0.9 ? |
Sure. Is there any bug fixed on the new version that may help with the encountered problem? |
|
There was a bug that doesn't apply here, sorry for the ping. For the details, PushService can be used to replace the MessagingReceiver, but isn't used here and it didn't work correctly with SDK<33 |
|
By the way, what is the encountered problem ? Is it the 507 response ? |
|
It may be better to use a resolutionStrategy than to exclude the transient dependency. something like configurations.all {
resolutionStrategy {
force(libs.tink)
dependencySubstitution {
substitute module('com.google.crypto.tink:tink-android') using module(libs.tink)
}
}
} |
|
Just for the record, I was able to get reliable push notifications working in NTFY by doing:
I have battery optimizations disabled on EXA, btw. With it enabled, notifications sometimes take some time to arrive. |
|
Thanks for the commit @jmartinesp ! We have decided to merge this PR and let users play with the nightly (that can be joined at https://appdistribution.firebase.dev/i/7de2dbc61e7fb2a6) and report any issue with UnifiedPush / ntfy. If we got issues, we will revert this PR before the next release. |
|
Content
Update UnifiedPush library
Motivation and context
Be up to date and let Renovate handle this in the future.
We will also benefit from improvement from the library, in particular the support for RAISE_TO_FOREGROUND added in UnifiedPush/android-connector@7438c52 .
Screenshots / GIFs
Tests
Tested devices
Checklist