Crash java.lang.SecurityException on app start cause by BrowserUtils.openBrowser about appcenter-sdk-android

[luboganev]

Hi @guperrot ,
Thanks for the suggestion. I'll try this definitely.
I don't have access to that device over the weekend, so I'll be able to test again on Monday. I'll post my findings here after that.

[luboganev]

Hi @guperrot , there is unfortunately more to it than just the default browser.

Test setup (FAIL)

The phone has the system Xiaomi Browser, Chrome and Opera installed. Here's the behavior of opening a link from the mail app:

As you can see, there is no issue in opening the browser from the email client. This is probably due to the fact the Email client does not care about not showing a picker and just tries to open the link like the second snippet you have provided previously.

I have done testing of the first snippet again after making sure there is a default browser. The problem is even if you have selected a default browser, the code you have provided will still crash. This is cause by the following part of the code:

String selectedPackageName = null;
String selectedClassName = null;
for (ResolveInfo browser : browsers) {
  ActivityInfo activityInfo = browser.activityInfo;
  if (activityInfo.packageName.equals(defaultBrowserPackageName) && activityInfo.name.equals(defaultBrowserClassName)) {
    selectedPackageName = defaultBrowserPackageName;
    selectedClassName = defaultBrowserClassName;
    AppCenterLog.debug("test", "And its not the picker.");
    break;
  }
}
if (defaultBrowser != null && selectedPackageName == null) {
  AppCenterLog.debug("test", "Default browser is actually a picker...");
}

The assumption that the default browser is a picker is simply not true in the case of the Xiaomi Redmi 5A. You make a very strong assumption in this code, which is that the default browser has to be contained in the list of browsers. However, this does not hold true for some reason on this device, since the reported default browser package is the correct one, and it is simply not listed in the query. The query doesn't even contain the Chrome browser or the system browser, which is yet another problem.

Please take in account, this is a normal user phone belonging to one of my colleagues and not a test device, so the issue is not an edge case only for developers testing on new (or freshly reset) phones as you have pointed out.

I have recorded the whole step by step debugging so that you can also observer what I see and understand better the situation. Please find the videos here:
MOV file (higher res): https://github.com/luboganev/appcenter-sdk-android/blob/develop/debugging_mov.mov
MP4 file (smaller size): https://github.com/luboganev/appcenter-sdk-android/blob/develop/debugging.mp4

I hope this helps.

[guperrot]

Resolution suggestion

I would like to propose that the code from the first snippet actually contains a try {} catch {} safeguard and use the second snippet as a fallback in case the first one crashes. Of course, you might think of better solution that matches better the SDK achitecture. In any case, I think it is critical to get this right, since this causes an app to crash so early, that not even the AppCenter crash reporting can send anything to the backend. If this would happen in the wild, noone will be able to see any crashes except when they start appearing in the Google Play console.

Ok I missed the fact that the list returned only invalid browsers, looks like try/catch the entire existing code and fallback to direct startActivity is the only way to work around the issue.

[annakocheshkova]

Hi @luboganev thanks for reporting this. I had Opera installed on my personal device for a long time and I never had this problem when testing App Center - on Xiaomi and Huawei.
Which Opera version do you use? It has several app versions (mini, touch etc.). We will try to reproduce.

[luboganev]

Hello @annakocheshkova ,

Thanks for the quick reply. The Opera version is this one:
https://play.google.com/store/apps/details?id=com.opera.browser

[guperrot]

Hi @luboganev, can you add this code to main activity onCreate just to test you can repro the crash every time?

/* Open a browser but we don't want a chooser U.I. to pop. */
        Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse("https://install.appcenter.ms/apps/f626dbb0-bd87-46e4-9214-14d06a2fdac5/update-setup/?release_hash=23d245d9f8701123ee196698c462ccbb5ff424cd533e746e85a7a329db79ccac&redirect_id=*******my_package_name******&redirect_scheme=appcenter&request_id=97fda018-869b-4f73-9a21-2884e41cca4e&platform=Android&enable_failure_redirect=true&install_id=44d98f13-9ddb-4367-82c5-0add4dc419fe"));
        List<ResolveInfo> browsers = getPackageManager().queryIntentActivities(intent, 0);
        if (browsers.isEmpty()) {
            AppCenterLog.error("test", "No browser found on device, abort login.");
        } else {

            /*
             * Check the default browser is not the picker,
             * last thing we want is app to start and suddenly asks user to pick
             * between 2 browsers without explaining why.
             */
            String defaultBrowserPackageName = null;
            String defaultBrowserClassName = null;
            ResolveInfo defaultBrowser = getPackageManager().resolveActivity(intent, PackageManager.MATCH_DEFAULT_ONLY);
            if (defaultBrowser != null) {
                ActivityInfo activityInfo = defaultBrowser.activityInfo;
                defaultBrowserPackageName = activityInfo.packageName;
                defaultBrowserClassName = activityInfo.name;
                AppCenterLog.debug("test", "Default browser seems to be " + defaultBrowserPackageName + "/" + defaultBrowserClassName);
            }
            String selectedPackageName = null;
            String selectedClassName = null;
            for (ResolveInfo browser : browsers) {
                ActivityInfo activityInfo = browser.activityInfo;
                if (activityInfo.packageName.equals(defaultBrowserPackageName) && activityInfo.name.equals(defaultBrowserClassName)) {
                    selectedPackageName = defaultBrowserPackageName;
                    selectedClassName = defaultBrowserClassName;
                    AppCenterLog.debug("test", "And its not the picker.");
                    break;
                }
            }
            if (defaultBrowser != null && selectedPackageName == null) {
                AppCenterLog.debug("test", "Default browser is actually a picker...");
            }

            /* If no default browser found, pick first one we can find. */
            if (selectedPackageName == null) {
                AppCenterLog.debug("test", "Picking first browser in list.");
                ResolveInfo browser = browsers.iterator().next();
                ActivityInfo activityInfo = browser.activityInfo;
                selectedPackageName = activityInfo.packageName;
                selectedClassName = activityInfo.name;
            }

            /* Launch generic browser. */
            AppCenterLog.debug("test", "Launch browser=" + selectedPackageName + "/" + selectedClassName);
            intent.setClassName(selectedPackageName, selectedClassName);
            startActivity(intent);
        }

Is the crash resolved by simply doing:

        Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse("https://install.appcenter.ms/apps/f626dbb0-bd87-46e4-9214-14d06a2fdac5/update-setup/?release_hash=23d245d9f8701123ee196698c462ccbb5ff424cd533e746e85a7a329db79ccac&redirect_id=*******my_package_name******&redirect_scheme=appcenter&request_id=97fda018-869b-4f73-9a21-2884e41cca4e&platform=Android&enable_failure_redirect=true&install_id=44d98f13-9ddb-4367-82c5-0add4dc419fe"));
        startActivity(intent);

It's important that you don't select a default browser during all the tests as it was the case when you crashed.

Thanks in advance.

[luboganev]

Hi everyone,

I have performed some testing with the code @guperrot provided and have found some interesting results.

Test setup (PASS)

• Code: Snippet 1
• Phone: Sony Xperia Z1 Compact Android 5.1 (It's different device, I know, that's intentional)
• No default browser is set
• Opera is actually the only available browser on the device (I have disabled the Chrome one)

Results

The code runs just fine and opens the Opera Browser without any issues. At second look, however, I've noticed it actually starts a completely different Activity. In the crash log I've posted above, the SDK tries to start this Activity and crashes:
Launch browser=com.opera.browser/com.opera.browser.leanplum.LeanplumCatchActivity

When I run the same thing on the Sony phone it actually starts the correct Launcher Activity of the Opera browser and runs just fine:
Launch browser=com.opera.browser/com.opera.android.MainLauncherActivity

However, if I force the code snippet to start the LeanplumCatchActivity on the Sony as well, I get the same crash SecurityException. This shows, the crash itself is not an OS issue of Xiaomi but is related to the Opera Browser itself and the Activities it reports when this line of the code is invoked:
Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse("*******the very long url******")); List<ResolveInfo> browsers = activity.getPackageManager().queryIntentActivities(intent, 0);

The list returned above contains two activities coming from the Opera Browser app as seen in the screenshot below:

As you notice, the Activity causing the crash is also in the list, but not at the first place, so the SDK actually does not invoke it, cause it picks the first available from the list. So in the case of the Sony phone, we just get lucky due to the order of Activities in the list.

Test setup (FAIL)

• Code: Snippet 1
• Phone: Xiaomi Redmi 5A
• No default browser is set

Basically running the same code results in this:

As you can see, the problematic Activity is the only one available, thus the SDK would start it and crash.

Test setup (PASS)

• Code: Snippet 2
• Phone: Xiaomi Redmi 5A
• No default browser is set
  By using the alternative code snippet by @guperrot :

Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse("*****the very long url****")); activity.startActivity(intent);

This time everything runs fine and the Opera Browser is opened successfully. To be honest, I have absolutely no idea why this actually works, since in theory if the activity.getPackageManager().queryIntentActivities does not work, this should fail as well. But that's the state of things now. I guess Xiaomi do something in the background.

Resolution suggestion

I would like to propose that the code from the first snippet actually contains a try {} catch {} safeguard and use the second snippet as a fallback in case the first one crashes. Of course, you might think of better solution that matches better the SDK achitecture. In any case, I think it is critical to get this right, since this causes an app to crash so early, that not even the AppCenter crash reporting can send anything to the backend. If this would happen in the wild, noone will be able to see any crashes except when they start appearing in the Google Play console.

[guperrot]

Hi thanks for the testing.
It's basically a permission issue that the query does not account for.

The SDK bypassed the intent picker as it would be weird for the end user to be asked what browser to use without explanation of what is going on.

Also we use this default mechanism (first in list) only if the user never ever chose a default browser on the device. It should be an edge case only for developers testing on new (or freshly reset) phones, but still need to be fixed.

The try/catch then fall back on another browser sounds like a reasonable solution.

[guperrot]

@luboganev in addition to the suggested fix, I'd like you to do 1 more test if possible, can you open any http link from the mail application (for example) and remember to use always opera when prompted to? You can use the mail from distributed release for AppCenter.

Once the default browser is registered, does AppCenter still crashes? (does the code snippet correctly detect browser is now default).

[luboganev]

@guperrot I see the fix was already merged to develop and it will probably be released with the next release. Should I close the issue or it is something you're going to do once the fix is released?

[guperrot]

Hi, we usually close the issue only after release for visibility and avoid duplicates.

[MatkovIvan]

Hey!
We've included fixes in the latest SDK release for this. I'm closing the issue but please reopen if you run into this again.