Grant System Gallery rename concessions
This change allows System Gallery app to rename any directory under
DCIM, Movies and Pictures, even if the directory includes a file that
System Gallery can't read/query. This change doesn't allow System
Gallery to change the files' extensions or have any special powers when
renaming files.
Adds a test case for FilePathAccessTest.
Test: atest FuseDaemonHostTest
Bug: 146777893
Change-Id: Iaff6ce99ae1c70f9f07054a40458cd5d80129f99
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index d21a060..22c8c1e 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -244,6 +244,15 @@
private static final String DIRECTORY_MEDIA = "media";
/**
+ * Specify what default directories the caller gets full access to. By default, the caller
+ * shouldn't get full access to any default dirs.
+ * But for example, we do an exception for System Gallery apps and allow them full access to:
+ * DCIM, Pictures, Movies.
+ */
+ private static final String INCLUDED_DEFAULT_DIRECTORIES =
+ "android:included-default-directories";
+
+ /**
* Set of {@link Cursor} columns that refer to raw filesystem paths.
*/
private static final ArrayMap<String, Object> sDataColumns = new ArrayMap<>();
@@ -1169,15 +1178,21 @@
mimeType.startsWith(supportedPrimaryMimeType));
}
+ private boolean updateDatabaseForFuseRename(@NonNull DatabaseHelper helper,
+ @NonNull String oldPath, @NonNull String newPath, @NonNull ContentValues values) {
+ return updateDatabaseForFuseRename(helper, oldPath, newPath, values, Bundle.EMPTY);
+ }
+
/**
* Updates database entry for given {@code path} with {@code values}
*/
private boolean updateDatabaseForFuseRename(@NonNull DatabaseHelper helper,
- @NonNull String oldPath, @NonNull String newPath, @NonNull ContentValues values) {
+ @NonNull String oldPath, @NonNull String newPath, @NonNull ContentValues values,
+ @NonNull Bundle qbExtras) {
final Uri uriOldPath = Files.getContentUriForPath(oldPath);
boolean allowHidden = isCallingPackageAllowedHidden();
final SQLiteQueryBuilder qbForUpdate = getQueryBuilder(TYPE_UPDATE,
- matchUri(uriOldPath, allowHidden), uriOldPath, Bundle.EMPTY, null);
+ matchUri(uriOldPath, allowHidden), uriOldPath, qbExtras, null);
final String selection = MediaColumns.DATA + " =? ";
int count = 0;
boolean retryUpdateWithReplace = false;
@@ -1197,7 +1212,7 @@
// write permission for newPath, delete existing database entry and retry update.
final Uri uriNewPath = Files.getContentUriForPath(oldPath);
final SQLiteQueryBuilder qbForDelete = getQueryBuilder(TYPE_DELETE,
- matchUri(uriNewPath, allowHidden), uriNewPath, Bundle.EMPTY, null);
+ matchUri(uriNewPath, allowHidden), uriNewPath, qbExtras, null);
if (qbForDelete.delete(helper, selection, new String[] {newPath}) == 1) {
Log.i(TAG, "Retrying database update after deleting conflicting entry");
count = qbForUpdate.update(helper, values, selection, new String[]{oldPath});
@@ -1231,6 +1246,19 @@
return values;
}
+ private ArrayList<String> getIncludedDefaultDirectories() {
+ final ArrayList<String> includedDefaultDirs = new ArrayList<>();
+ if (checkCallingPermissionVideo(/*forWrite*/ true, null)) {
+ includedDefaultDirs.add(DIRECTORY_DCIM);
+ includedDefaultDirs.add(DIRECTORY_PICTURES);
+ includedDefaultDirs.add(DIRECTORY_MOVIES);
+ } else if (checkCallingPermissionImages(/*forWrite*/ true, null)) {
+ includedDefaultDirs.add(DIRECTORY_DCIM);
+ includedDefaultDirs.add(DIRECTORY_PICTURES);
+ }
+ return includedDefaultDirs;
+ }
+
/**
* Gets all files in the given {@code path} and subdirectories of the given {@code path}.
*/
@@ -1262,6 +1290,20 @@
*/
private ArrayList<String> getWritableFilesForRenameDirectory(String oldPath, String newPath)
throws IllegalArgumentException {
+ // Try a simple check to see if the caller has full access to the given collections first
+ // before falling back to performing a query to probe for access.
+ final String oldRelativePath = extractRelativePathForDirectory(oldPath);
+ final String newRelativePath = extractRelativePathForDirectory(newPath);
+ boolean hasFullAccessToOldPath = false;
+ boolean hasFullAccessToNewPath = false;
+ for (String defaultDir : getIncludedDefaultDirectories()) {
+ if (oldRelativePath.startsWith(defaultDir)) hasFullAccessToOldPath = true;
+ if (newRelativePath.startsWith(defaultDir)) hasFullAccessToNewPath = true;
+ }
+ if (hasFullAccessToNewPath && hasFullAccessToOldPath) {
+ return getAllFilesForRenameDirectory(oldPath);
+ }
+
final int countAllFilesInDirectory;
final String selection = MediaColumns.RELATIVE_PATH + " REGEXP '^" +
extractRelativePathForDirectory(oldPath) + "/?.*' and mime_type not like 'null'";
@@ -1368,11 +1410,14 @@
helper.beginTransaction();
try {
+ final Bundle qbExtras = new Bundle();
+ qbExtras.putStringArrayList(INCLUDED_DEFAULT_DIRECTORIES,
+ getIncludedDefaultDirectories());
for (String filePath : fileList) {
final String newFilePath = newPath + "/" + filePath;
final String mimeType = MimeUtils.resolveMimeType(new File(newFilePath));
if(!updateDatabaseForFuseRename(helper, oldPath + "/" + filePath, newFilePath,
- getContentValuesForFuseRename(newFilePath, mimeType, mimeType))) {
+ getContentValuesForFuseRename(newFilePath, mimeType, mimeType), qbExtras)) {
Log.e(TAG, "Calling package doesn't have write permission to rename file.");
return OsConstants.EPERM;
}
@@ -3045,6 +3090,9 @@
int matchTrashed = extras.getInt(QUERY_ARG_MATCH_TRASHED, MATCH_DEFAULT);
int matchFavorite = extras.getInt(QUERY_ARG_MATCH_FAVORITE, MATCH_DEFAULT);
+ final ArrayList<String> includedDefaultDirs = extras.getStringArrayList(
+ INCLUDED_DEFAULT_DIRECTORIES);
+
// Handle callers using legacy arguments
if (MediaStore.getIncludePending(uri)) matchPending = MATCH_INCLUDE;
@@ -3454,6 +3502,11 @@
FileColumns.MEDIA_TYPE_IMAGE));
options.add("media_type=0 AND mime_type LIKE 'image/%'");
}
+ if (includedDefaultDirs != null) {
+ for (String defaultDir : includedDefaultDirs) {
+ options.add(FileColumns.RELATIVE_PATH + " LIKE '" + defaultDir + "/%'");
+ }
+ }
}
if (options.size() > 0) {
appendWhereStandalone(qb, TextUtils.join(" OR ", options));
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 a4ab22b..2825040 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
@@ -249,4 +249,9 @@
public void testQueryOtherAppsFiles() throws Exception {
runDeviceTest("testQueryOtherAppsFiles");
}
+
+ @Test
+ public void testSystemGalleryCanRenameImageAndVideoDirs() throws Exception {
+ runDeviceTest("testSystemGalleryCanRenameImageAndVideoDirs");
+ }
}
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 b905bf5..a17e7b9 100644
--- a/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
+++ b/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
@@ -1460,6 +1460,57 @@
}
}
+ /**
+ * Test that System Gallery app can rename any directory under the default directories
+ * designated for images and videos, even if they contain other apps' contents that
+ * System Gallery doesn't have read access to.
+ */
+ @Test
+ public void testSystemGalleryCanRenameImageAndVideoDirs() throws Exception {
+ final File dirInDcim = new File(DCIM_DIR, TEST_DIRECTORY_NAME);
+ final File dirInPictures = new File(PICTURES_DIR, TEST_DIRECTORY_NAME);
+ final File dirInPodcasts = new File(PODCASTS_DIR, TEST_DIRECTORY_NAME);
+ final File otherAppImageFile1 = new File(dirInDcim, "other_" + IMAGE_FILE_NAME);
+ final File otherAppVideoFile1 = new File(dirInDcim, "other_" + VIDEO_FILE_NAME);
+ final File otherAppPdfFile1 = new File(dirInDcim, "other_" + NONMEDIA_FILE_NAME);
+ final File otherAppImageFile2 = new File(dirInPictures, "other_" + IMAGE_FILE_NAME);
+ final File otherAppVideoFile2 = new File(dirInPictures, "other_" + VIDEO_FILE_NAME);
+ final File otherAppPdfFile2 = new File(dirInPictures, "other_" + NONMEDIA_FILE_NAME);
+ try {
+ assertThat(dirInDcim.mkdir()).isTrue();
+
+ executeShellCommand("touch " + otherAppPdfFile1);
+
+ installApp(TEST_APP_A, true);
+ allowAppOpsToUid(Process.myUid(), SYSTEM_GALERY_APPOPS);
+
+ assertCreateFilesAs(TEST_APP_A, otherAppImageFile1, otherAppVideoFile1);
+
+ // System gallery privileges don't go beyond DCIM, Movies and Pictures boundaries.
+ assertCantRenameDirectory(dirInDcim, dirInPodcasts, /*oldFilesList*/null);
+
+ // Rename should succeed, but System Gallery still can't access that PDF file!
+ assertCanRenameDirectory(dirInDcim, dirInPictures,
+ new File[] {otherAppImageFile1, otherAppVideoFile1},
+ new File[] {otherAppImageFile2, otherAppVideoFile2});
+ assertThat(getFileRowIdFromDatabase(otherAppPdfFile1)).isEqualTo(-1);
+ assertThat(getFileRowIdFromDatabase(otherAppPdfFile2)).isEqualTo(-1);
+ } finally {
+ executeShellCommand("rm " + otherAppPdfFile1);
+ executeShellCommand("rm " + otherAppPdfFile2);
+ otherAppImageFile1.delete();
+ otherAppImageFile2.delete();
+ otherAppVideoFile1.delete();
+ otherAppVideoFile2.delete();
+ otherAppPdfFile1.delete();
+ otherAppPdfFile2.delete();
+ dirInDcim.delete();
+ dirInPictures.delete();
+ uninstallAppNoThrow(TEST_APP_A);
+ denyAppOpsToUid(Process.myUid(), SYSTEM_GALERY_APPOPS);
+ }
+ }
+
private static void assertCantQueryFile(File file) {
assertThat(getFileUri(file)).isNull();
}