Merge "Revert create/update/delete database for apps that bypass restrictions." into rvc-dev

6 files changed, 42 insertions, 20 deletions

diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index 9387132d0..f960a9a7d 100644
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -1096,9 +1096,11 @@ static void pf_release(fuse_req_t req,
 
     fuse->fadviser.Close(h->fd);
     if (node) {
-        if (h->owner_uid != -1) {
-            fuse->mp->ScanFile(h->path, h->owner_uid);
-        }
+        // TODO(b/145737191) Legacy apps don't expect FuseDaemon to update database.
+        // Inserting/deleting the database entry might break app's functionality.
+        // if (h->owner_uid != -1) {
+        //    fuse->mp->ScanFile(h->path, h->owner_uid);
+        // }
         node->DestroyHandle(h);
     }
 
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index e48eea131..a53419430 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -1468,12 +1468,16 @@ public class MediaProvider extends ContentProvider {
      * However, we update database entries for renamed files to keep the database consistent.
      */
     private int renameUncheckedForFuse(String oldPath, String newPath) {
-        if (new File(oldPath).isFile()) {
-            return renameFileUncheckedForFuse(oldPath, newPath);
-        } else {
-            return renameDirectoryUncheckedForFuse(oldPath, newPath,
-                    getAllFilesForRenameDirectory(oldPath));
-        }
+
+        return renameInLowerFs(oldPath, newPath);
+        // TODO(b/145737191) Legacy apps don't expect FuseDaemon to update database.
+        // Inserting/deleting the database entry might break app functionality.
+        //if (new File(oldPath).isFile()) {
+        //     return renameFileUncheckedForFuse(oldPath, newPath);
+        // } else {
+        //    return renameDirectoryUncheckedForFuse(oldPath, newPath,
+        //            getAllFilesForRenameDirectory(oldPath));
+        // }
     }
 
     /**
@@ -5667,6 +5671,12 @@ public class MediaProvider extends ContentProvider {
         final LocalCallingIdentity token =
                 clearLocalCallingIdentity(LocalCallingIdentity.fromExternal(getContext(), uid));
         try {
+            if (shouldBypassFuseRestrictions(/*forWrite*/ true, path)) {
+                // TODO(b/145737191) Legacy apps don't expect FuseDaemon to update database.
+                // Inserting/deleting the database entry might break app functionality.
+                return deleteFileUnchecked(path);
+            }
+
             // Check if app is deleting a file under an app specific directory
             final String appSpecificDir = extractPathOwnerPackageName(path);
 
@@ -5680,12 +5690,9 @@ public class MediaProvider extends ContentProvider {
                 }
             }
 
-            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 (!shouldBypass && isCallingPackageRequestingLegacy()) {
+            if (isCallingPackageRequestingLegacy()) {
                 return OsConstants.EPERM;
             }
 
@@ -5699,12 +5706,6 @@ public class MediaProvider extends ContentProvider {
             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/host/src/com/android/tests/fused/host/FuseDaemonHostTest.java b/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/FuseDaemonHostTest.java
index 6af8a3a64..a4ab22bba 100644
--- a/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/FuseDaemonHostTest.java
+++ b/tests/jni/FuseDaemonTest/host/src/com/android/tests/fused/host/FuseDaemonHostTest.java
@@ -24,6 +24,7 @@ import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
 
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -118,6 +119,7 @@ public class FuseDaemonHostTest extends BaseHostJUnit4Test {
         runDeviceTest("testListFilesFromExternalMediaDirectory");
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testListUnsupportedFileType() throws Exception {
         final ITestDevice device = getDevice();
@@ -194,21 +196,25 @@ public class FuseDaemonHostTest extends BaseHostJUnit4Test {
         runDeviceTest("testSystemGalleryAppHasNoFullAccessToAudio");
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testSystemGalleryCanRenameImagesAndVideos() throws Exception {
         runDeviceTest("testSystemGalleryCanRenameImagesAndVideos");
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testManageExternalStorageCanCreateFilesAnywhere() throws Exception {
         runDeviceTest("testManageExternalStorageCanCreateFilesAnywhere");
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testManageExternalStorageCanDeleteOtherAppsContents() throws Exception {
         runDeviceTest("testManageExternalStorageCanDeleteOtherAppsContents");
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testManageExternalStorageCanRenameOtherAppsContents() throws Exception {
         runDeviceTest("testManageExternalStorageCanRenameOtherAppsContents");
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 0502aedac..ba574b727 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
@@ -25,6 +25,7 @@ import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
 
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -153,6 +154,7 @@ public class LegacyAccessHostTest extends BaseHostJUnit4Test {
 
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testCanRename_hasRW() throws Exception {
         runDeviceTest("testCanRename_hasRW");
@@ -182,11 +184,13 @@ public class LegacyAccessHostTest extends BaseHostJUnit4Test {
         }
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testCanDeleteAllFiles_hasRW() throws Exception {
         runDeviceTest("testCanDeleteAllFiles_hasRW");
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testLegacyAppCanOwnAFile_hasW() throws Exception {
         runDeviceTest("testLegacyAppCanOwnAFile_hasW");
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 8e0a0d95d..8dfbb4df1 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
@@ -53,6 +53,7 @@ import com.android.cts.install.lib.TestApp;
 import com.android.tests.fused.lib.ReaddirTestHelper;
 
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -277,6 +278,7 @@ public class LegacyFileAccessTest {
      * Test that rename for legacy app with WRITE_EXTERNAL_STORAGE permission bypasses rename
      * restrictions imposed by MediaProvider
      */
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testCanRename_hasRW() throws Exception {
         pollForPermission(Manifest.permission.READ_EXTERNAL_STORAGE, /*granted*/ true);
@@ -389,6 +391,7 @@ public class LegacyFileAccessTest {
      * Test that legacy app with WRITE_EXTERNAL_STORAGE can delete all files, and corresponding
      * database entry is deleted on deleting the file.
      */
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testCanDeleteAllFiles_hasRW() throws Exception {
         pollForPermission(Manifest.permission.READ_EXTERNAL_STORAGE, /*granted*/ true);
@@ -428,6 +431,7 @@ public class LegacyFileAccessTest {
      * Test that file created by legacy app is inserted to MediaProvider database. And,
      * MediaColumns.OWNER_PACKAGE_NAME is updated with calling package's name.
      */
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testLegacyAppCanOwnAFile_hasW() throws Exception {
         pollForPermission(Manifest.permission.READ_EXTERNAL_STORAGE, /*granted*/ true);
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 97daf940e..579794d8b 100644
--- a/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
+++ b/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
@@ -75,7 +75,6 @@ import android.system.Os;
 import android.system.OsConstants;
 import android.util.Log;
 
-import androidx.annotation.Nullable;
 import androidx.test.runner.AndroidJUnit4;
 
 import com.android.cts.install.lib.TestApp;
@@ -84,6 +83,7 @@ import com.android.tests.fused.lib.ReaddirTestHelper;
 import com.google.common.io.ByteStreams;
 
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -665,6 +665,7 @@ public class FilePathAccessTest {
     /**
      * Test that readdir lists unsupported file types in default directories.
      */
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testListUnsupportedFileType() throws Exception {
         final File pdfFile = new File(DCIM_DIR, NONMEDIA_FILE_NAME);
@@ -971,6 +972,7 @@ public class FilePathAccessTest {
         }
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testSystemGalleryCanRenameImagesAndVideos() throws Exception {
         final File otherAppVideoFile = new File(DCIM_DIR, "other_" + VIDEO_FILE_NAME);
@@ -1255,6 +1257,7 @@ public class FilePathAccessTest {
         }
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testManageExternalStorageCanCreateFilesAnywhere() throws Exception {
         final File topLevelPdf = new File(EXTERNAL_STORAGE_DIR, NONMEDIA_FILE_NAME);
@@ -1295,6 +1298,7 @@ public class FilePathAccessTest {
         }
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testManageExternalStorageCanDeleteOtherAppsContents() throws Exception {
         final File otherAppPdf = new File(DOWNLOAD_DIR, "other" + NONMEDIA_FILE_NAME);
@@ -1329,6 +1333,7 @@ public class FilePathAccessTest {
         }
     }
 
+    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testManageExternalStorageCanRenameOtherAppsContents() throws Exception {
         final File otherAppPdf = new File(DOWNLOAD_DIR, "other" + NONMEDIA_FILE_NAME);