Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FLAG_IMMUTABLE to PendingIntents to satisfy SDK 31+ requirements #193

Closed
wants to merge 1 commit into from
Closed

Conversation

sredman
Copy link

@sredman sredman commented Jan 21, 2023

As described in #187, post SDK 31, Android requires that any PendingIntent specify either FLAG_IMMUTABLE or FLAG_MUTABLE, with FLAG_MUTABLE being the default behavior prior to SDK 31.

This change moves relevant PendingIntents to FLAG_IMMUTABLE. This seems to be safe because the PendingIntents created by this library do not seem to require mutability.

FLAG_MUTABLE would require bumping all the way to SDK31; this is much more difficult because Android has hidden several methods of the ConnectivityManager class, such as startUsingNetworkFeature, for which there does not appear to be a drop-in replacement.

This has been tested in my use to send both SMS and MMS with attachment, where the app operates as not the default system messaging app. If the app were the default messaging app, FLAG_MUTABLE might be necessary (I do not know).

@@ -14,7 +14,7 @@ android {

defaultConfig {
applicationId "com.klinker.android.send_message.sample"
minSdkVersion 14
minSdkVersion 21
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that any app which consumes this change will have to bump the minSdkVersion to 21 (Android 5.0).

@@ -10,7 +10,7 @@ android {
compileSdkVersion 25

defaultConfig {
minSdkVersion 14
minSdkVersion 21
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PendingIntent.FLAG_IMMUTABLE is added in SDK 21 (Android 5.0), so this bump is required.

kdesysadmin pushed a commit to KDE/kdeconnect-android that referenced this pull request Jan 23, 2023
…upports SDK31

## Summary

Android apps which target SDK 31+ require specifying the mutability of any PENDING_INTENT. This is not supported in the upstream android-smsmms library: klinker41/android-smsmms#193

Until the above PR is merged, we need a solution. I have pulled the code into https://invent.kde.org/sredman/android-smsmms and published the package in the Maven repository in gitlab.

BUG: 464392

## Test Plan

### Before:
Attempting to send an SMS or MMS message using kdeconnect-sms results in no message being sent, and an error being logged:

> V/Sending message: Sending new SMS
> E/Sending message: Exception
>     java.lang.IllegalArgumentException: org.kde.kdeconnect_tp: Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent.
>     Strongly consider using FLAG_IMMUTABLE, only use FLAG_MUTABLE if some functionality depends on the PendingIntent being mutable, e.g. if it needs to be used with inline replies or bubbles.
>         at android.app.PendingIntent.checkFlags(PendingIntent.java:382)
>         at android.app.PendingIntent.getBroadcastAsUser(PendingIntent.java:673)
>         at android.app.PendingIntent.getBroadcast(PendingIntent.java:660)
>         at com.klinker.android.send_message.Transaction.sendSmsMessage(Transaction.java:267)
>         at com.klinker.android.send_message.Transaction.sendNewMessage(Transaction.java:158)
>         at com.klinker.android.send_message.Transaction.sendNewMessage(Transaction.java:172)
>         at org.kde.kdeconnect.Plugins.SMSPlugin.SmsMmsUtils.sendMessage(SmsMmsUtils.java:188)
>         at org.kde.kdeconnect.Plugins.SMSPlugin.SMSPlugin.onPacketReceived(SMSPlugin.java:414)
>         at org.kde.kdeconnect.Device.onPacketReceived(Device.java:570)
>         <snipped for brevity>

### After:
SMS and MMS sends normally.
@albertvaka
Copy link

@klinker41 any chance for a release including this fix?

@sredman sredman closed this by deleting the head repository Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants