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

Android 14 (compile SDK 34) and BroadcastReceiver changes #582

Merged
merged 8 commits into from
Nov 15, 2023

Conversation

QuantumRand
Copy link
Contributor

@QuantumRand QuantumRand commented Oct 4, 2023

Upgrades compile-sdk to Android 14 (34) and makes required BroadcastReceiver changes (must include export flag when registering).

Waiting on Tuulbox 7.0.0 release.

Internal ticket: CA-2225

@twyatt twyatt added android patch Changes that should bump the PATCH version number labels Oct 5, 2023
@twyatt twyatt changed the title CA-2225: Android 14 (compile SDK 34) and BroadcastReceiver changes Android 14 (compile SDK 34) and BroadcastReceiver changes Oct 5, 2023
gradle/libs.versions.toml Show resolved Hide resolved
core/build.gradle.kts Outdated Show resolved Hide resolved
applicationContext.unregisterReceiver(receiver)
}
}.shareIn(GlobalScope, started = WhileSubscribed(replayExpirationMillis = 0))
internal val bluetoothState = broadcastReceiverFlow(intentFilter, RECEIVER_NOT_EXPORTED)
Copy link
Member

Choose a reason for hiding this comment

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

nit: RECEIVER_NOT_EXPORTED is default, right?

Suggested change
internal val bluetoothState = broadcastReceiverFlow(intentFilter, RECEIVER_NOT_EXPORTED)
internal val bluetoothState = broadcastReceiverFlow(intentFilter)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tons of conflicting information about this.
ContextCompat documentation says system BroadcastReceivers need to be exported:

flags – If this receiver is listening for broadcasts sent from the system or from other apps—even other apps that you own—use the RECEIVER_EXPORTED flag. If instead this receiver is listening only for broadcasts sent by your app, use the RECEIVER_NOT_EXPORTED flag.

But Google says that's a security vulnerability and isn't allowed in Android 14 (system receivers should have no flag at all):
https://developer.android.com/about/versions/14/behavior-changes-14#system-broadcasts

Documentation in Context seems to be most up to date, so I'm sticking with that:

For apps targeting Build.VERSION_CODES.UPSIDE_DOWN_CAKE, either RECEIVER_EXPORTED or RECEIVER_NOT_EXPORTED must be specified if the receiver is not being registered for system broadcasts or a SecurityException will be thrown. See registerReceiver(BroadcastReceiver, IntentFilter, int) to register a receiver with flags.

Copy link
Member

@twyatt twyatt Nov 15, 2023

Choose a reason for hiding this comment

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

Maybe you misunderstood my comment?

I was saying that we don't really need to specify the parameter because RECEIVER_NOT_EXPORTED is the default in Tuulbox: https://github.com/JuulLabs/tuulbox/blob/b592470845918d99e5d6c128243a5038e753fc38/coroutines/src/androidMain/kotlin/flow/BroadcastReceiverFlow.kt#L18

So, I wasn't saying we should not use RECEIVER_NOT_EXPORTED, rather: that flag will be used (by Tuulbox) if we omit the parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understood the comment, but it got me looking closer at the documentation since I realized it didn't jive with what Google's Android 14 migration docs said.

@QuantumRand QuantumRand marked this pull request as ready for review November 15, 2023 18:13
@QuantumRand QuantumRand requested review from davertay-j and a team as code owners November 15, 2023 18:13
@QuantumRand QuantumRand requested a review from sdonn3 November 15, 2023 18:13
@QuantumRand QuantumRand requested a review from twyatt November 15, 2023 19:04
core/build.gradle.kts Outdated Show resolved Hide resolved
@QuantumRand QuantumRand merged commit bc61551 into main Nov 15, 2023
@QuantumRand QuantumRand deleted the paul/ca-2225-android-14 branch November 15, 2023 22:24
@twyatt twyatt added this to the 0.28.0 milestone Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android patch Changes that should bump the PATCH version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants