From 10171d922860fd3cb74ed5e3931947e0ae31e7ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Thu, 3 Sep 2015 18:52:40 -0700 Subject: [PATCH] Installer: handle package parse errors, fixes #395 --- F-Droid/res/values/strings.xml | 1 + F-Droid/src/org/fdroid/fdroid/AppDetails.java | 21 ++-- .../org/fdroid/fdroid/installer/AppDiff.java | 9 ++ .../installer/InstallConfirmActivity.java | 97 ++++++++++--------- .../fdroid/fdroid/installer/Installer.java | 8 +- .../fdroid/installer/SystemInstaller.java | 34 ++++--- 6 files changed, 102 insertions(+), 68 deletions(-) diff --git a/F-Droid/res/values/strings.xml b/F-Droid/res/values/strings.xml index 5a01d7aa1..493fc8ebc 100644 --- a/F-Droid/res/values/strings.xml +++ b/F-Droid/res/values/strings.xml @@ -295,6 +295,7 @@ Update all Install error Failed to install due to an unknown error + An error occurred while parsing the package Uninstall error Failed to uninstall due to an unknown error System permissions denied diff --git a/F-Droid/src/org/fdroid/fdroid/AppDetails.java b/F-Droid/src/org/fdroid/fdroid/AppDetails.java index 9252aa6b1..ee03cb40c 100644 --- a/F-Droid/src/org/fdroid/fdroid/AppDetails.java +++ b/F-Droid/src/org/fdroid/fdroid/AppDetails.java @@ -909,15 +909,20 @@ public class AppDetails extends AppCompatActivity implements ProgressListener, A final int title, body; if (operation == InstallerCallback.OPERATION_INSTALL) { title = R.string.install_error_title; - } else { - title = R.string.uninstall_error_title; - } - switch (errorCode) { - default: - if (operation == InstallerCallback.OPERATION_INSTALL) { + switch (errorCode) { + case ERROR_CODE_CANNOT_PARSE: + body = R.string.install_error_cannot_parse; + break; + default: // ERROR_CODE_OTHER body = R.string.install_error_unknown; - } else { - body = R.string.uninstall_error_unknown; + break; + } + } else { // InstallerCallback.OPERATION_DELETE + title = R.string.uninstall_error_title; + switch (errorCode) { + default: // ERROR_CODE_OTHER + body = R.string.install_error_unknown; + break; } } runOnUiThread(new Runnable() { diff --git a/F-Droid/src/org/fdroid/fdroid/installer/AppDiff.java b/F-Droid/src/org/fdroid/fdroid/installer/AppDiff.java index 9e68b866a..2391cb096 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/AppDiff.java +++ b/F-Droid/src/org/fdroid/fdroid/installer/AppDiff.java @@ -36,6 +36,15 @@ public class AppDiff { final String pkgPath = mPackageURI.getPath(); mPkgInfo = mPm.getPackageArchiveInfo(pkgPath, PackageManager.GET_PERMISSIONS); + // We could not get the package info from the file. This means that we + // could not parse the file, which can happen if the file cannot be + // read or the minSdk is not satisfied. + // Since we can't return an error from a constructor, we refuse to + // continue. The caller must check if mPkgInfo is null to see if the + // AppDiff was initialised correctly. + if (mPkgInfo == null) { + return; + } mPkgInfo.applicationInfo.sourceDir = pkgPath; mPkgInfo.applicationInfo.publicSourceDir = pkgPath; diff --git a/F-Droid/src/org/fdroid/fdroid/installer/InstallConfirmActivity.java b/F-Droid/src/org/fdroid/fdroid/installer/InstallConfirmActivity.java index 298913284..aeaf16e8b 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/InstallConfirmActivity.java +++ b/F-Droid/src/org/fdroid/fdroid/installer/InstallConfirmActivity.java @@ -43,6 +43,8 @@ import org.fdroid.fdroid.R; public class InstallConfirmActivity extends Activity implements OnCancelListener, OnClickListener { + public static final int RESULT_CANNOT_PARSE = RESULT_FIRST_USER + 1; + private Intent intent; PackageManager mPm; @@ -83,55 +85,54 @@ public class InstallConfirmActivity extends Activity implements OnCancelListener mScrollView = null; mOkCanInstall = false; int msg = 0; - if (mAppDiff.mPkgInfo != null) { - AppSecurityPermissions perms = new AppSecurityPermissions(this, mAppDiff.mPkgInfo); - final int NP = perms.getPermissionCount(AppSecurityPermissions.WHICH_PERSONAL); - final int ND = perms.getPermissionCount(AppSecurityPermissions.WHICH_DEVICE); - if (mAppDiff.mInstalledAppInfo != null) { - msg = (mAppDiff.mInstalledAppInfo.flags & ApplicationInfo.FLAG_SYSTEM) != 0 - ? R.string.install_confirm_update_system - : R.string.install_confirm_update; - mScrollView = new CaffeinatedScrollView(this); - mScrollView.setFillViewport(true); - final boolean newPermissionsFound = - (perms.getPermissionCount(AppSecurityPermissions.WHICH_NEW) > 0); - if (newPermissionsFound) { - permVisible = true; - mScrollView.addView(perms.getPermissionsView( - AppSecurityPermissions.WHICH_NEW)); - } else { - throw new RuntimeException("This should not happen. No new permissions were found but InstallConfirmActivity has been started!"); - } - adapter.addTab(tabHost.newTabSpec(TAB_ID_NEW).setIndicator( - getText(R.string.newPerms)), mScrollView); - } else { - findViewById(R.id.tabscontainer).setVisibility(View.GONE); - findViewById(R.id.divider).setVisibility(View.VISIBLE); - } - if (NP > 0 || ND > 0) { + AppSecurityPermissions perms = new AppSecurityPermissions(this, mAppDiff.mPkgInfo); + final int NP = perms.getPermissionCount(AppSecurityPermissions.WHICH_PERSONAL); + final int ND = perms.getPermissionCount(AppSecurityPermissions.WHICH_DEVICE); + if (mAppDiff.mInstalledAppInfo != null) { + msg = (mAppDiff.mInstalledAppInfo.flags & ApplicationInfo.FLAG_SYSTEM) != 0 + ? R.string.install_confirm_update_system + : R.string.install_confirm_update; + mScrollView = new CaffeinatedScrollView(this); + mScrollView.setFillViewport(true); + final boolean newPermissionsFound = + (perms.getPermissionCount(AppSecurityPermissions.WHICH_NEW) > 0); + if (newPermissionsFound) { permVisible = true; - LayoutInflater inflater = (LayoutInflater) getSystemService( - Context.LAYOUT_INFLATER_SERVICE); - View root = inflater.inflate(R.layout.permissions_list, null); - if (mScrollView == null) { - mScrollView = (CaffeinatedScrollView) root.findViewById(R.id.scrollview); - } - final ViewGroup privacyList = (ViewGroup) root.findViewById(R.id.privacylist); - if (NP > 0) { - privacyList.addView(perms.getPermissionsView(AppSecurityPermissions.WHICH_PERSONAL)); - } else { - privacyList.setVisibility(View.GONE); - } - final ViewGroup deviceList = (ViewGroup) root.findViewById(R.id.devicelist); - if (ND > 0) { - deviceList.addView(perms.getPermissionsView(AppSecurityPermissions.WHICH_DEVICE)); - } else { - root.findViewById(R.id.devicelist).setVisibility(View.GONE); - } - adapter.addTab(tabHost.newTabSpec(TAB_ID_ALL).setIndicator( - getText(R.string.allPerms)), root); + mScrollView.addView(perms.getPermissionsView( + AppSecurityPermissions.WHICH_NEW)); + } else { + throw new RuntimeException("This should not happen. No new permissions were found but InstallConfirmActivity has been started!"); } + adapter.addTab(tabHost.newTabSpec(TAB_ID_NEW).setIndicator( + getText(R.string.newPerms)), mScrollView); + } else { + findViewById(R.id.tabscontainer).setVisibility(View.GONE); + findViewById(R.id.divider).setVisibility(View.VISIBLE); } + if (NP > 0 || ND > 0) { + permVisible = true; + LayoutInflater inflater = (LayoutInflater) getSystemService( + Context.LAYOUT_INFLATER_SERVICE); + View root = inflater.inflate(R.layout.permissions_list, null); + if (mScrollView == null) { + mScrollView = (CaffeinatedScrollView) root.findViewById(R.id.scrollview); + } + final ViewGroup privacyList = (ViewGroup) root.findViewById(R.id.privacylist); + if (NP > 0) { + privacyList.addView(perms.getPermissionsView(AppSecurityPermissions.WHICH_PERSONAL)); + } else { + privacyList.setVisibility(View.GONE); + } + final ViewGroup deviceList = (ViewGroup) root.findViewById(R.id.devicelist); + if (ND > 0) { + deviceList.addView(perms.getPermissionsView(AppSecurityPermissions.WHICH_DEVICE)); + } else { + root.findViewById(R.id.devicelist).setVisibility(View.GONE); + } + adapter.addTab(tabHost.newTabSpec(TAB_ID_ALL).setIndicator( + getText(R.string.allPerms)), root); + } + if (!permVisible) { if (mAppDiff.mInstalledAppInfo != null) { // This is an update to an application, but there are no @@ -184,6 +185,10 @@ public class InstallConfirmActivity extends Activity implements OnCancelListener Uri packageURI = intent.getData(); mAppDiff = new AppDiff(mPm, packageURI); + if (mAppDiff.mPkgInfo == null) { + setResult(RESULT_CANNOT_PARSE, intent); + finish(); + } setContentView(R.layout.install_start); mInstallConfirm = findViewById(R.id.install_confirm_panel); diff --git a/F-Droid/src/org/fdroid/fdroid/installer/Installer.java b/F-Droid/src/org/fdroid/fdroid/installer/Installer.java index f3ff93788..7c24135a7 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/Installer.java +++ b/F-Droid/src/org/fdroid/fdroid/installer/Installer.java @@ -76,10 +76,12 @@ abstract public class Installer { public interface InstallerCallback { int OPERATION_INSTALL = 1; - int OPERATION_DELETE = 2; + int OPERATION_DELETE = 2; - int ERROR_CODE_CANCELED = 1; - int ERROR_CODE_OTHER = 2; + // Avoid using [-1,1] as they may conflict with Activity.RESULT_* + int ERROR_CODE_CANCELED = 2; + int ERROR_CODE_OTHER = 3; + int ERROR_CODE_CANNOT_PARSE = 4; void onSuccess(int operation); diff --git a/F-Droid/src/org/fdroid/fdroid/installer/SystemInstaller.java b/F-Droid/src/org/fdroid/fdroid/installer/SystemInstaller.java index 46dcc5814..da779c52a 100644 --- a/F-Droid/src/org/fdroid/fdroid/installer/SystemInstaller.java +++ b/F-Droid/src/org/fdroid/fdroid/installer/SystemInstaller.java @@ -140,7 +140,13 @@ public class SystemInstaller extends Installer { @Override protected void installPackageInternal(File apkFile) throws AndroidNotCompatibleException { Uri packageUri = Uri.fromFile(apkFile); - if (hasNewPermissions(packageUri)) { + int count = newPermissionCount(packageUri); + if (count < 0) { + mCallback.onError(InstallerCallback.OPERATION_INSTALL, + InstallerCallback.ERROR_CODE_CANNOT_PARSE); + return; + } + if (count > 0) { Intent intent = new Intent(mContext, InstallConfirmActivity.class); intent.setData(packageUri); mActivity.startActivityForResult(intent, REQUEST_CONFIRM_PERMS); @@ -154,17 +160,20 @@ public class SystemInstaller extends Installer { } } - private boolean hasNewPermissions(Uri packageUri) { + private int newPermissionCount(Uri packageUri) { AppDiff appDiff = new AppDiff(mContext.getPackageManager(), packageUri); - if (appDiff.mPkgInfo != null) { - AppSecurityPermissions perms = new AppSecurityPermissions(mContext, appDiff.mPkgInfo); - if (appDiff.mInstalledAppInfo != null) { // it is an update to an existing app - // return false if there are no new permissions - return (perms.getPermissionCount(AppSecurityPermissions.WHICH_NEW) > 0); - } + if (appDiff.mPkgInfo == null) { + // could not get diff because we couldn't parse the package + return -1; } - // default: show install confirm activity - return true; + AppSecurityPermissions perms = new AppSecurityPermissions(mContext, appDiff.mPkgInfo); + if (appDiff.mInstalledAppInfo != null) { + // update to an existing app + return perms.getPermissionCount(AppSecurityPermissions.WHICH_NEW); + } + // default: even if there aren't any permissions, we want to make the + // user always confirm installing new apps + return 1; } private void doInstallPackageInternal(Uri packageURI) throws AndroidNotCompatibleException { @@ -260,7 +269,10 @@ public class SystemInstaller extends Installer { mCallback.onError(InstallerCallback.OPERATION_INSTALL, InstallerCallback.ERROR_CODE_OTHER); } - } else { + } else if (resultCode == InstallConfirmActivity.RESULT_CANNOT_PARSE) { + mCallback.onError(InstallerCallback.OPERATION_INSTALL, + InstallerCallback.ERROR_CODE_CANNOT_PARSE); + } else { // Activity.RESULT_CANCELED mCallback.onError(InstallerCallback.OPERATION_INSTALL, InstallerCallback.ERROR_CODE_CANCELED); }