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

Fixing #699 java.lang.SecurityException #700

Merged
merged 3 commits into from
Nov 3, 2023
Merged

Conversation

danpe
Copy link
Contributor

@danpe danpe commented Oct 31, 2023

Fixes #699

@@ -204,6 +205,11 @@ internal abstract class MavericksPrintStateBroadcastReceiver : BroadcastReceiver
fun register(context: Context) {
check(!isRegistered) { "Already registered" }
isRegistered = true
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
context.registerReceiver(networkInfoReceiver, filter, Context.RECEIVER_NOT_EXPORTED)
Copy link
Contributor

Choose a reason for hiding this comment

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

@danpe thanks for fixing, but this doesn't seem to compile

MavericksMockPrinter.kt:209:38 Unresolved reference: networkInfoReceiver

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to do what context.registerReceiver(this, IntentFilter(ACTION_COPY_MAVERICKS_STATE)) does below? and that line should be replaced with this new code

Copy link
Contributor Author

@danpe danpe Nov 2, 2023

Choose a reason for hiding this comment

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

Thank you @elihart my bad, fixed it.
Samsung just released Android 14 to all its flagship devices, so I believe we'll be seeing more and more crashes due to this...

Copy link
Contributor

Choose a reason for hiding this comment

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

Good to know, but since this is the mocking library it shouldn't be running in production. Do you need to change your configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is weird as even if I disable mocking completely it still crashes,

        MockableMavericks.initialize(
            mocksEnabled =false
            debugMode = false,
            applicationContext = this,
            //TODO maybe use other dispatchers
            viewModelCoroutineContext = Dispatchers.Default,
            stateStoreCoroutineContext = Dispatchers.Default
        )

Still crashes on startup...

By adding Context.RECEIVER_NOT_EXPORTED
@danpe danpe requested a review from elihart November 3, 2023 11:38
@elihart elihart merged commit a5b728c into airbnb:main Nov 3, 2023
3 checks passed
@danpe
Copy link
Contributor Author

danpe commented Nov 3, 2023 via email

@elihart
Copy link
Contributor

elihart commented Nov 3, 2023

I can make a release next week. I'm still curious though, if you are seeing these crashes in production, if you should instead be changing your config to not run the mock printer code in prod builds

@danpe
Copy link
Contributor Author

danpe commented Nov 7, 2023

I can make a release next week. I'm still curious though, if you are seeing these crashes in production, if you should instead be changing your config to not run the mock printer code in prod builds

It is weird as even if I disable mocking completely it still crashes,

        MockableMavericks.initialize(
            mocksEnabled =false
            debugMode = false,
            applicationContext = this,
            //TODO maybe use other dispatchers
            viewModelCoroutineContext = Dispatchers.Default,
            stateStoreCoroutineContext = Dispatchers.Default
        )

Still crashes on startup...

@elihart
Copy link
Contributor

elihart commented Nov 7, 2023

That doesn't control the mock printer configuration. You must be calling registerMockPrinter somewhere in your app. Don't call it in production builds...

@danpe
Copy link
Contributor Author

danpe commented Nov 8, 2023

That doesn't control the mock printer configuration. You must be calling registerMockPrinter somewhere in your app. Don't call it in production builds...

Tbh, we don't call registerMockPrinter anywhere in our app, that why it is strange.

@elihart
Copy link
Contributor

elihart commented Nov 13, 2023

It seems like you'll want to figure out the root cause of why the mock printer is registering in your production app. Even if you apply this fix you still don't want the broadcast receiver starting in your prod app.

Look at how you might be using MockableMavericks.enableMockPrinterBroadcastReceiver, or manual instantiation of MavericksMockPrinter

@danpe
Copy link
Contributor Author

danpe commented Nov 14, 2023

Hi @elihart , thank you for responding.
TBH, I did Find All.. (Cmd+Shift+F) and tried looking for both enableMockPrinterBroadcastReceiver and MavericksMockPrinter in all my files and modules.

Nothing found.

@mlengy
Copy link

mlengy commented Nov 28, 2023

Hi @elihart, thanks for the fix. Currently running into this issue when attempting to target API 34 as well. Curious when a new release would be possible. Thanks!

@elihart
Copy link
Contributor

elihart commented Nov 28, 2023

I'll get a release out this week

@ArunYogeshwaran
Copy link

Thanks for this fix. It helped us when we upgraded our target SDK version to 34

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.

java.lang.SecurityException: One of RECEIVER_EXPORTED or RECEIVER_NOT_EXPORTED should be specified
4 participants