Merge "Delete database entry for files deleted by apps that bypass restrictions"
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 424f1f2..2dafd9d 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -5578,10 +5578,6 @@
final LocalCallingIdentity token =
clearLocalCallingIdentity(LocalCallingIdentity.fromExternal(getContext(), uid));
try {
- if (shouldBypassFuseRestrictions(/*forWrite*/ true, path)) {
- return deleteFileUnchecked(path);
- }
-
// Check if app is deleting a file under an app specific directory
final String appSpecificDir = extractPathOwnerPackageName(path);
@@ -5595,9 +5591,12 @@
}
}
+ final boolean shouldBypass = shouldBypassFuseRestrictions(/*forWrite*/ true,
+ path);
+
// Legacy apps that made is this far don't have the right storage permission and hence
// are not allowed to access anything other than their external app directory
- if (isCallingPackageRequestingLegacy()) {
+ if (!shouldBypass && isCallingPackageRequestingLegacy()) {
return OsConstants.EPERM;
}
@@ -5611,6 +5610,12 @@
final String[] whereArgs = {sanitizedPath};
if (delete(contentUri, where, whereArgs) == 0) {
+ if (shouldBypass) {
+ // For apps that bypass scoped storage restrictions, delete() should only fail
+ // if the file is not in database. This shouldn't stop these apps from deleting
+ // a file, hence delete the file in the lower file system.
+ return deleteFileUnchecked(path);
+ }
return OsConstants.ENOENT;
} else if (!path.equals(sanitizedPath)) {
// delete() doesn't delete the file in lower file system if sanitized path is
diff --git a/tests/jni/FuseDaemonTest/Android.bp b/tests/jni/FuseDaemonTest/Android.bp
index 1c4afe6..35cc9df 100644
--- a/tests/jni/FuseDaemonTest/Android.bp
+++ b/tests/jni/FuseDaemonTest/Android.bp
@@ -46,7 +46,10 @@
static_libs: ["androidx.test.rules", "truth-prebuilt", "tests-fusedaemon-lib"],
test_suites: ["general-tests", "mts"],
sdk_version: "test_current",
- target_sdk_version: "29"
+ target_sdk_version: "29",
+ java_resources: [
+ ":TestAppA",
+ ]
}
java_test_host {
diff --git a/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/LegacyAccessHostTest.java b/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/LegacyAccessHostTest.java
index 33b874f..78283f7 100644
--- a/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/LegacyAccessHostTest.java
+++ b/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/LegacyAccessHostTest.java
@@ -154,8 +154,8 @@
}
@Test
- public void testCanRename_hasW() throws Exception {
- runDeviceTest("testCanRename_hasW");
+ public void testCanRename_hasRW() throws Exception {
+ runDeviceTest("testCanRename_hasRW");
}
@Test
@@ -176,4 +176,9 @@
executeShellCommand("rm " + SHELL_FILE);
}
}
+
+ @Test
+ public void testCanDeleteAllFiles_hasRW() throws Exception {
+ runDeviceTest("testCanDeleteAllFiles_hasRW");
+ }
}
diff --git a/tests/jni/FuseDaemonTest/legacy/AndroidManifest.xml b/tests/jni/FuseDaemonTest/legacy/AndroidManifest.xml
index 25530b6..0665f91 100644
--- a/tests/jni/FuseDaemonTest/legacy/AndroidManifest.xml
+++ b/tests/jni/FuseDaemonTest/legacy/AndroidManifest.xml
@@ -18,7 +18,10 @@
package="com.android.tests.fused.legacy" >
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
+ <uses-permission android:name="android.permission.QUERY_ALL_PACKAGES" />
<application android:requestLegacyExternalStorage="true" >
+ <receiver android:name="com.android.cts.install.lib.LocalIntentSender"
+ android:exported="true" />
<uses-library android:name="android.test.runner" />
</application>
diff --git a/tests/jni/FuseDaemonTest/legacy/src/com/android/tests/fused/legacy/LegacyFileAccessTest.java b/tests/jni/FuseDaemonTest/legacy/src/com/android/tests/fused/legacy/LegacyFileAccessTest.java
index 2f897de..64f67f5 100644
--- a/tests/jni/FuseDaemonTest/legacy/src/com/android/tests/fused/legacy/LegacyFileAccessTest.java
+++ b/tests/jni/FuseDaemonTest/legacy/src/com/android/tests/fused/legacy/LegacyFileAccessTest.java
@@ -16,7 +16,12 @@
package com.android.tests.fused.legacy;
+import static com.android.tests.fused.lib.TestUtils.createFileAs;
+import static com.android.tests.fused.lib.TestUtils.deleteFileAsNoThrow;
+import static com.android.tests.fused.lib.TestUtils.getFileRowIdFromDatabase;
+import static com.android.tests.fused.lib.TestUtils.installApp;
import static com.android.tests.fused.lib.TestUtils.pollForExternalStorageState;
+import static com.android.tests.fused.lib.TestUtils.uninstallApp;
import static com.android.tests.fused.lib.TestUtils.pollForPermission;
@@ -34,6 +39,9 @@
import androidx.test.InstrumentationRegistry;
import androidx.test.runner.AndroidJUnit4;
+import com.android.cts.install.lib.TestApp;
+import com.android.tests.fused.lib.ReaddirTestHelper;
+
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -57,6 +65,12 @@
private static final String TAG = "LegacyFileAccessTest";
+ static final String VIDEO_FILE_NAME = "LegacyAccessTest_file.mp4";
+ static final String NONMEDIA_FILE_NAME = "LegacyAccessTest_file.pdf";
+
+ private static final TestApp TEST_APP_A = new TestApp("TestAppA",
+ "com.android.tests.fused.testapp.A", 1, false, "TestAppA.apk");
+
@Before
public void setUp() throws Exception {
pollForExternalStorageState();
@@ -254,7 +268,7 @@
* restrictions imposed by MediaProvider
*/
@Test
- public void testCanRename_hasW() throws Exception {
+ public void testCanRename_hasRW() throws Exception {
pollForPermission(Manifest.permission.READ_EXTERNAL_STORAGE, /*granted*/ true);
pollForPermission(Manifest.permission.WRITE_EXTERNAL_STORAGE, /*granted*/ true);
@@ -359,6 +373,45 @@
}
}
+ /**
+ * Test that legacy app with WRITE_EXTERNAL_STORAGE can delete all files, and corresponding
+ * database entry is deleted on deleting the file.
+ */
+ @Test
+ public void testCanDeleteAllFiles_hasRW() throws Exception {
+ pollForPermission(Manifest.permission.READ_EXTERNAL_STORAGE, /*granted*/ true);
+ pollForPermission(Manifest.permission.WRITE_EXTERNAL_STORAGE, /*granted*/ true);
+
+ final File EXTERNAL_STORAGE_DIR = Environment.getExternalStorageDirectory();
+ final File videoFile = new File(EXTERNAL_STORAGE_DIR, VIDEO_FILE_NAME);
+ final File otherAppPdfFile = new File(EXTERNAL_STORAGE_DIR,
+ Environment.DIRECTORY_DOWNLOADS + "/" + NONMEDIA_FILE_NAME);
+
+ try {
+ assertThat(videoFile.createNewFile()).isTrue();
+ assertThat(ReaddirTestHelper.readDirectory(EXTERNAL_STORAGE_DIR))
+ .contains(VIDEO_FILE_NAME);
+
+ assertThat(getFileRowIdFromDatabase(videoFile)).isNotEqualTo(-1);
+ // Legacy app can delete its own file.
+ assertThat(videoFile.delete()).isTrue();
+ // Deleting the file will remove videoFile entry from database.
+ assertThat(getFileRowIdFromDatabase(videoFile)).isEqualTo(-1);
+
+ installApp(TEST_APP_A, false);
+ assertThat(createFileAs(TEST_APP_A, otherAppPdfFile.getAbsolutePath())).isTrue();
+ assertThat(getFileRowIdFromDatabase(otherAppPdfFile)).isNotEqualTo(-1);
+ // Legacy app with write permission can delete the pdfFile owned by TestApp.
+ assertThat(otherAppPdfFile.delete()).isTrue();
+ // Deleting the pdfFile also removes pdfFile from database.
+ assertThat(getFileRowIdFromDatabase(otherAppPdfFile)).isEqualTo(-1);
+ } finally {
+ deleteFileAsNoThrow(TEST_APP_A, otherAppPdfFile.getAbsolutePath());
+ uninstallApp(TEST_APP_A);
+ videoFile.delete();
+ }
+ }
+
private static void assertCanCreateFile(File file) throws IOException {
if (file.exists()) {
file.delete();
diff --git a/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java b/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
index ba2a2fd..8f47edd 100644
--- a/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
+++ b/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
@@ -1005,6 +1005,7 @@
// to images and video only
assertThat(imageFile.renameTo(musicFile)).isFalse();
} finally {
+ deleteFileAsNoThrow(TEST_APP_A, otherAppVideoFile.getAbsolutePath());
uninstallApp(TEST_APP_A);
imageFile.delete();
videoFile.delete();
@@ -1317,16 +1318,11 @@
assertThat(otherAppMusic.delete()).isTrue();
assertThat(otherAppMusic.exists()).isFalse();
-
- // Create the files again to allow the helper app to clean them up
- assertThat(otherAppPdf.createNewFile()).isTrue();
- assertThat(otherAppImage.createNewFile()).isTrue();
- assertThat(otherAppMusic.createNewFile()).isTrue();
} finally {
+ dropShellPermissionIdentity();
deleteFileAsNoThrow(TEST_APP_A, otherAppPdf.getAbsolutePath());
deleteFileAsNoThrow(TEST_APP_A, otherAppImage.getAbsolutePath());
deleteFileAsNoThrow(TEST_APP_A, otherAppMusic.getAbsolutePath());
- dropShellPermissionIdentity();
uninstallApp(TEST_APP_A);
}
}
@@ -1380,7 +1376,7 @@
topLevelPdf.delete();
musicFile.delete();
dropShellPermissionIdentity();
- otherAppPdf.delete();
+ deleteFileAsNoThrow(TEST_APP_A, otherAppPdf.getAbsolutePath());
uninstallApp(TEST_APP_A);
}
}