commit 09f670c39c7adbbff159a5ac2b29529043d8aea7
author Hai Zhang <zhanghai@google.com> Fri Apr 03 16:03:21 2020 -0700
committer Hai Zhang <zhanghai@google.com> Wed May 06 18:40:28 2020 -0700
tree e10c16b3b8680e74b45fe8d12e91a84367f0a67a
parent ae4db7840279e277ce05d88e3e9be514f6716a1e

Add warning about declaring the MANAGE_EXTERNAL_STORAGE permission.

The MANAGE_EXTERNAL_STORAGE app op is the app op of the permission
MANAGE_EXTERNAL_STORAGE, and cannot be granted without the app
declaring the permission.

Bug: 149765004
Test: atest AaptParserTest
Change-Id: I54f31949dca17abf25f8910d3454e4a774c5c5e5

src/com/android/tradefed/device/TestDevice.java

diff --git a/src/com/android/tradefed/device/TestDevice.java b/src/com/android/tradefed/device/TestDevice.java
index 0ff6616..73c453f 100644
--- a/src/com/android/tradefed/device/TestDevice.java
+++ b/src/com/android/tradefed/device/TestDevice.java
@@ -438,6 +438,15 @@
             if (aaptParser != null
                     && aaptParser.getTargetSdkVersion() > 29
                     && aaptParser.isRequestingLegacyStorage()) {
+                if (!aaptParser.isUsingPermissionManageExternalStorage()) {
+                    CLog.w(
+                            "App is requesting legacy storage and targets R or above, but didn't"
+                                    + " request the MANAGE_EXTERNAL_STORAGE permission so the"
+                                    + " associated app op cannot be automatically granted and the"
+                                    + " app won't have legacy external storage access: "
+                                    + aaptParser.getPackageName());
+                    continue;
+                }
                 // Set the MANAGE_EXTERNAL_STORAGE App Op to MODE_ALLOWED (Code = 0)
                 // for all users.
                 ArrayList<Integer> userIds = listUsers();

src/com/android/tradefed/util/AaptParser.java

diff --git a/src/com/android/tradefed/util/AaptParser.java b/src/com/android/tradefed/util/AaptParser.java
index b20652e..a88fb8f 100644
--- a/src/com/android/tradefed/util/AaptParser.java
+++ b/src/com/android/tradefed/util/AaptParser.java
@@ -48,6 +48,9 @@
 
     private static final Pattern ALT_NATIVE_CODE_PATTERN =
             Pattern.compile("alt-native-code: '(.*)'");
+    private static final Pattern USES_PERMISSION_MANAGE_EXTERNAL_STORAGE_PATTERN =
+            Pattern.compile(
+                    "uses-permission: name='android\\.permission\\.MANAGE_EXTERNAL_STORAGE'");
     private static final int AAPT_TIMEOUT_MS = 60000;
     private static final int INVALID_SDK = -1;
 
@@ -59,6 +62,7 @@
     private int mSdkVersion = INVALID_SDK;
     private int mTargetSdkVersion = 10000;
     private boolean mRequestLegacyStorage = false;
+    private boolean mUsesPermissionManageExternalStorage;
 
     // @VisibleForTesting
     AaptParser() {
@@ -100,6 +104,8 @@
                     }
                 }
             }
+            m = USES_PERMISSION_MANAGE_EXTERNAL_STORAGE_PATTERN.matcher(aaptOut);
+            mUsesPermissionManageExternalStorage = m.find();
             return true;
         }
         CLog.e("Failed to parse package and version info from 'aapt dump badging'. stdout: '%s'",
@@ -209,4 +215,8 @@
     public boolean isRequestingLegacyStorage() {
         return mRequestLegacyStorage;
     }
+
+    public boolean isUsingPermissionManageExternalStorage() {
+        return mUsesPermissionManageExternalStorage;
+    }
 }

tests/src/com/android/tradefed/util/AaptParserTest.java

diff --git a/tests/src/com/android/tradefed/util/AaptParserTest.java b/tests/src/com/android/tradefed/util/AaptParserTest.java
index f25871c..65bf70d 100644
--- a/tests/src/com/android/tradefed/util/AaptParserTest.java
+++ b/tests/src/com/android/tradefed/util/AaptParserTest.java
@@ -221,6 +221,31 @@
         assertFalse(p.isRequestingLegacyStorage());
     }
 
+    public void testParseXmlTree_withUsesPermissionManageExternalStorage() {
+        AaptParser p = new AaptParser();
+        p.parse(
+                "package: name='com.android.foo' versionCode='217173' versionName='1.7173' "
+                        + "platformBuildVersionName=''\n"
+                        + "install-location:'preferExternal'\n"
+                        + "sdkVersion:'10'\n"
+                        + "targetSdkVersion:'21'\n"
+                        + "uses-permission: name='android.permission.INTERNET'\n"
+                        + "uses-permission: name='android.permission.MANAGE_EXTERNAL_STORAGE'\n");
+        assertTrue(p.isUsingPermissionManageExternalStorage());
+    }
+
+    public void testParseXmlTree_withoutUsesPermissionManageExternalStorage() {
+        AaptParser p = new AaptParser();
+        p.parse(
+                "package: name='com.android.foo' versionCode='217173' versionName='1.7173' "
+                        + "platformBuildVersionName=''\n"
+                        + "install-location:'preferExternal'\n"
+                        + "sdkVersion:'10'\n"
+                        + "targetSdkVersion:'21'\n"
+                        + "uses-permission: name='android.permission.INTERNET'\n");
+        assertFalse(p.isUsingPermissionManageExternalStorage());
+    }
+
     public void testParseTargetSdkVersion() {
         AaptParser p = new AaptParser();
         p.parse(