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

Fix android 14 crash #5

Merged
merged 6 commits into from
Feb 15, 2024
Merged

Fix android 14 crash #5

merged 6 commits into from
Feb 15, 2024

Conversation

rikner
Copy link

@rikner rikner commented Feb 15, 2024

Using PendingIntent.FLAG_ALLOW_UNSAFE_IMPLICIT_INTENT fixes following crash:

--------- beginning of crash
02-14 18:04:54.290 E/AndroidRuntime( 3110): FATAL EXCEPTION: MidiDeviceConnectionWatchThread
02-14 18:04:54.290 E/AndroidRuntime( 3110): Process: com.flowkey.testapp, PID: 3110
02-14 18:04:54.290 E/AndroidRuntime( 3110): java.lang.IllegalArgumentException: com.flowkey.testapp: Targeting U+ (version 34 and above) disallows creating or retrieving a PendingIntent with FLAG_MUTABLE, an implicit Intent within and without FLAG_NO_CREATE and FLAG_ALLOW_UNSAFE_IMPLICIT_INTENT for security reasons. To retrieve an already existing PendingIntent, use FLAG_NO_CREATE, however, to create a new PendingIntent with an implicit Intent use FLAG_IMMUTABLE.
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at android.os.Parcel.createExceptionOrNull(Parcel.java:3061)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at android.os.Parcel.createException(Parcel.java:3041)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at android.os.Parcel.readException(Parcel.java:3024)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at android.os.Parcel.readException(Parcel.java:2966)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at android.app.IActivityManager$Stub$Proxy.getIntentSenderWithFeature(IActivityManager.java:6568)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at android.app.PendingIntent.getBroadcastAsUser(PendingIntent.java:735)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at android.app.PendingIntent.getBroadcast(PendingIntent.java:718)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at jp.kshoji.driver.midi.device.MidiDeviceConnectionWatcher$MidiDeviceConnectionWatchThread.run(MidiDeviceConnectionWatcher.java:274)
02-14 18:04:54.290 E/AndroidRuntime( 3110): Caused by: android.os.RemoteException: Remote stack trace:
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at com.android.server.am.ActivityManagerService.getIntentSenderWithFeatureAsApp(ActivityManagerService.java:5277)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at com.android.server.am.ActivityManagerService.getIntentSenderWithFeature(ActivityManagerService.java:5220)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at android.app.IActivityManager$Stub.onTransact(IActivityManager.java:3251)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at com.android.server.am.ActivityManagerService.onTransact(ActivityManagerService.java:2720)
02-14 18:04:54.290 E/AndroidRuntime( 3110):     at android.os.Binder.execTransactInternal(Binder.java:1339)
02-14 18:04:54.290 E/AndroidRuntime( 3110): 

However we also need to use the RECEIVER_EXPORTED flag in order to fix a follow-up issue:

2024-02-15 13:49:37.438 23760-23932 AndroidRuntime          com.flowkey.testapp                  E  FATAL EXCEPTION: MidiDeviceConnectionWatchThread
                                                                                                    Process: com.flowkey.testapp, PID: 23760
                                                                                                    java.lang.SecurityException: com.flowkey.testapp: One of RECEIVER_EXPORTED or RECEIVER_NOT_EXPORTED should be specified when a receiver isn't being registered exclusively for system broadcasts
                                                                                                    	at android.os.Parcel.createExceptionOrNull(Parcel.java:3057)
                                                                                                    	at android.os.Parcel.createException(Parcel.java:3041)
                                                                                                    	at android.os.Parcel.readException(Parcel.java:3024)
                                                                                                    	at android.os.Parcel.readException(Parcel.java:2966)
                                                                                                    	at android.app.IActivityManager$Stub$Proxy.registerReceiverWithFeature(IActivityManager.java:5684)
                                                                                                    	at android.app.ContextImpl.registerReceiverInternal(ContextImpl.java:1852)
                                                                                                    	at android.app.ContextImpl.registerReceiver(ContextImpl.java:1792)
                                                                                                    	at android.app.ContextImpl.registerReceiver(ContextImpl.java:1780)
                                                                                                    	at android.content.ContextWrapper.registerReceiver(ContextWrapper.java:755)
                                                                                                    	at jp.kshoji.driver.midi.device.MidiDeviceConnectionWatcher$MidiDeviceConnectionWatchThread.run(MidiDeviceConnectionWatcher.java:287)
                                                                                                    Caused by: android.os.RemoteException: Remote stack trace:
                                                                                                    	at com.android.server.am.ActivityManagerService.registerReceiverWithFeature(ActivityManagerService.java:13927)
                                                                                                    	at android.app.IActivityManager$Stub.onTransact(IActivityManager.java:2570)
                                                                                                    	at com.android.server.am.ActivityManagerService.onTransact(ActivityManagerService.java:2720)
                                                                                                    	at android.os.Binder.execTransactInternal(Binder.java:1339)
                                                                                                    	at android.os.Binder.execTransact(Binder.java:1275)

Copy link
Member

@ephemer ephemer left a comment

Choose a reason for hiding this comment

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

Thanks for making the intent explicit, that would have been my first suggestion. I wonder if we can get away with using RECEIVER_NOT_EXPORTED? That would be more secure

IntentFilter filter = new IntentFilter(UsbMidiGrantedReceiver.USB_PERMISSION_GRANTED_ACTION);

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
context.registerReceiver(receiver, filter, RECEIVER_EXPORTED);
Copy link
Member

Choose a reason for hiding this comment

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

Is it really necessary to export the receiver to be available to other apps?
Is this a requirement because we're awaiting a system notification?

Copy link
Member

Choose a reason for hiding this comment

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

Have we tried this with RECEIVER_NOT_EXPORTED?

Copy link
Author

Choose a reason for hiding this comment

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

when using the unsafe flag RECEIVER_NOT_EXPORTED did not work (since no MIDI messages were received or send). BUT now with the explicit intent RECEIVER_NOT_EXPORTED it seems to work. will push the change 👍

Copy link
Member

Choose a reason for hiding this comment

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

yes implicit intents have to be exported but explicit intents do not.

it's possible midi messages were arriving but the driver assumes the permissions were rejected if it cannot access the information in the intent (which contain the message whether permissions were accepted or not)

@rikner rikner merged commit e31fac4 into develop Feb 15, 2024
1 check passed
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.

3 participants