Revert create/update/delete database for apps that bypass restrictions.
Apps are not aware of FuseDaemon modifying the database. Apps that
assume the old uri to be available even after rename/delete don't
work correctly because of the new behaviour.
As a temporary fix for this issue, reverting create/update/delete
database row from FuseDaemon for apps that bypass scoped storage
restrictions. Long term solution will be added as part of b/145737191.
Ignoring some FuseDaemonHostTest that fail becuase of the revert.
Test: atest FuseDaemonHostTest --iterations 2
Bug: 150147690
Change-Id: Ia0a8fe2461133ed0ef68384c4856c0fc93735541
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index 58d3f71..3bd9ea4 100644
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -1114,9 +1114,11 @@
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 e48eea1..a534194 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -1468,12 +1468,16 @@
* 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 @@
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 @@
}
}
- 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 @@
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 6af8a3a..a4ab22b 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 org.junit.After;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -118,6 +119,7 @@
runDeviceTest("testListFilesFromExternalMediaDirectory");
}
+ @Ignore("Re-enable as part of b/145737191")
@Test
public void testListUnsupportedFileType() throws Exception {
final ITestDevice device = getDevice();
@@ -194,21 +196,25 @@
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 0502aed..ba574b7 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 org.junit.After;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -153,6 +154,7 @@
}
+ @Ignore("Re-enable as part of b/145737191")
@Test
public void testCanRename_hasRW() throws Exception {
runDeviceTest("testCanRename_hasRW");
@@ -182,11 +184,13 @@
}
}
+ @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 8e0a0d9..8dfbb4d 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.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 @@
* 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 @@
* 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 @@
* 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 97daf94..579794d 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.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.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 @@
/**
* 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 @@
}
}
+ @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 @@
}
}
+ @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 @@
}
}
+ @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 @@
}
}
+ @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);