FUSE rename: Check file type and write permission for renaming file.
On FUSE rename file, check if the calling app has write permissions for old
and new path of the file to be renamed. Also, check if the new file type
is supported by new path's top level directory.
After renaming the file in lower file system, MediaProvider database entry is
updated with new path. Renaming a file doesn't change the owner package
name in the database entry.
Bug: 144279181
Bug: 142475473
Test: atest -c FuseDaemonHostTest#testRenameFile
atest -c FuseDaemonHostTest#testRenameFileType
atest -c FuseDaemonHostTest#testRenameFileNotOwned
Change-Id: Ib8ba8a9e80030537403fa7909c017c469d5ff51f
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index ca3c24d..b6c0094 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -96,6 +96,7 @@
import android.database.AbstractCursor;
import android.database.Cursor;
import android.database.MatrixCursor;
+import android.database.sqlite.SQLiteConstraintException;
import android.database.sqlite.SQLiteDatabase;
import android.database.sqlite.SQLiteQueryBuilder;
import android.graphics.Bitmap;
@@ -1045,6 +1046,190 @@
}
/**
+ * Checks if given {@code mimeType} is supported in {@code path}.
+ */
+ private boolean isMimeTypeSupportedInPath(String path, String mimeType) {
+ final String supportedPrimaryMimeType;
+ switch (matchUri(getContentUriForFile(path, mimeType), true)) {
+ case AUDIO_MEDIA:
+ supportedPrimaryMimeType = "audio";
+ break;
+ case VIDEO_MEDIA:
+ supportedPrimaryMimeType = "video";
+ break;
+ case IMAGES_MEDIA:
+ supportedPrimaryMimeType = "image";
+ break;
+ default:
+ supportedPrimaryMimeType = ClipDescription.MIMETYPE_UNKNOWN;
+ }
+ return (supportedPrimaryMimeType.equals(ClipDescription.MIMETYPE_UNKNOWN) ||
+ mimeType.startsWith(supportedPrimaryMimeType));
+ }
+
+ /**
+ * Updates database entry for given {@code path} with {@code values}
+ */
+ private boolean updateDatabaseForFuseRename(SQLiteDatabase db, String oldPath, String newPath,
+ ContentValues values) {
+ final Uri uriOldPath = Files.getContentUriForPath(oldPath);
+ boolean allowHidden = isCallingPackageAllowedHidden();
+ final SQLiteQueryBuilder qbForUpdate = getQueryBuilder(TYPE_UPDATE,
+ matchUri(uriOldPath, allowHidden), uriOldPath, Bundle.EMPTY, null);
+ final String selection = MediaColumns.DATA + " =? ";
+ int count = 0;
+ boolean retryUpdateWithReplace = false;
+
+ try {
+ count = qbForUpdate.update(db, values, selection, new String[]{oldPath});
+ } catch (SQLiteConstraintException e) {
+ Log.w(TAG, "Database update failed while renaming " + oldPath, e);
+ retryUpdateWithReplace = true;
+ }
+
+ if (retryUpdateWithReplace) {
+ // We are replacing file in newPath with file in oldPath. If calling package has
+ // 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);
+ if (qbForDelete.delete(db, selection, new String[] {newPath}) == 1) {
+ Log.i(TAG, "Retrying database update after deleting conflicting entry");
+ count = qbForUpdate.update(db, values, selection, new String[]{oldPath});
+ } else {
+ return false;
+ }
+ }
+ return count == 1;
+ }
+
+ /**
+ * Gets {@link ContentValues} for updating database entry to {@code path}.
+ */
+ private ContentValues getContentValuesForFuseRename(String path, String oldMimeType,
+ String newMimeType) {
+ ContentValues values = new ContentValues();
+ values.put(MediaColumns.MIME_TYPE, newMimeType);
+ values.put(MediaColumns.DATA, path);
+
+ if (!oldMimeType.equals(newMimeType)) {
+ int mediaType = MimeUtils.resolveMediaType(newMimeType);
+ values.put(FileColumns.MEDIA_TYPE, mediaType);
+ }
+ final boolean allowHidden = isCallingPackageAllowedHidden();
+ if (!newMimeType.equals("null") &&
+ matchUri(getContentUriForFile(path, newMimeType), allowHidden) == AUDIO_MEDIA) {
+ computeAudioLocalizedValues(values);
+ computeAudioKeyValues(values);
+ }
+ computeDataValues(values);
+ return values;
+ }
+
+ /**
+ * Process metadata after renaming file/directory. This method does post processing in
+ * background thread so that rename is not blocked on post processing and also any error
+ * occurred while post processing is not reported as rename error.
+ */
+ private void postProcessMetadataForFuseRename(String oldPath, String newPath) {
+ final LocalCallingIdentity token = clearLocalCallingIdentity();
+ try {
+ BackgroundThread.getExecutor().execute(() -> {
+ Uri uri = Files.getContentUriForPath(newPath);
+ final DatabaseHelper helper;
+ try {
+ helper = getDatabaseForUri(uri);
+ } catch (VolumeNotFoundException e) {
+ Log.w("Volume not found while trying to process metadata for rename of "
+ + oldPath + " to " + newPath, e);
+ return;
+ }
+
+ try (Cursor c = queryForSingleItem(uri, new String [] {MediaColumns._ID},
+ MediaColumns.DATA + " =? ", new String[]{newPath}, null)) {
+ c.moveToFirst();
+ uri = ContentUris.withAppendedId(uri, c.getInt(0));
+ } catch(FileNotFoundException e) {
+ Log.w("Failed to process metadata after renaming " + oldPath, e);
+ return;
+ }
+ final String oldMimeType = MimeUtils.resolveMimeType(new File(oldPath));
+ final String newMimeType = MimeUtils.resolveMimeType(new File(newPath));
+ if (!oldMimeType.equals(newMimeType)) {
+ invalidateThumbnails(uri);
+ int mediaType = MimeUtils.resolveMediaType(newMimeType);
+ // If we're changing media types, invalidate any cached "empty answers for
+ // the new collection type.
+ MediaDocumentsProvider.onMediaStoreInsert(getContext(), getVolumeName(uri),
+ mediaType, -1);
+ }
+ acceptWithExpansion(helper::notifyChange, uri);
+ });
+ } finally {
+ restoreLocalCallingIdentity(token);
+ }
+ }
+
+ /**
+ * Rename a file from {@code oldPath} to {@code newPath}.
+ *
+ * Renaming a file is split into three parts:
+ * 1. Check if {@code newPath} supports new file type.
+ * 2. Try updating database entry from {@code oldPath} to {@code newPath}. This update may fail
+ * if calling package doesn't have write permission for {@code oldPath} and {@code newPath}.
+ * 3. Rename the file in lower file system. If Rename in lower file system succeeds, commit
+ * database update.
+ * @param oldPath path of the file to be renamed.
+ * @param newPath new path of the file to be renamed.
+ * @return 0 on successful rename, appropriate negated errno value if the rename is not allowed.
+ * <ul>
+ * <li>{@link OsConstants#EPERM} Calling package doesn't have write permission for
+ * {@code oldPath} or {@code newPath}, or file type is not supported by {@code newPath}.
+ * This method can also return errno returned from {@code Os.rename} function.
+ */
+ private int renameFileForFuse(String oldPath, String newPath) {
+ // Check if new mime type is supported in new path.
+ final String newMimeType = MimeUtils.resolveMimeType(new File(newPath));
+ if (!isMimeTypeSupportedInPath(newPath, newMimeType)) {
+ return -OsConstants.EPERM;
+ }
+
+ final SQLiteDatabase db;
+ try {
+ final DatabaseHelper helper = getDatabaseForUri(Files.getContentUriForPath(oldPath));
+ db = helper.getWritableDatabase();
+ } catch (VolumeNotFoundException e) {
+ throw new IllegalStateException("Volume not found while trying to update database for"
+ + oldPath + ". Rename failed due to database update error", e);
+ }
+
+ db.beginTransaction();
+ try {
+ final String oldMimeType = MimeUtils.resolveMimeType(new File(oldPath));
+ if (!updateDatabaseForFuseRename(db, oldPath, newPath,
+ getContentValuesForFuseRename(newPath, oldMimeType, newMimeType))) {
+ Log.e(TAG, "Calling package doesn't have write permission to rename file.");
+ return -OsConstants.EPERM;
+ }
+
+ // Try renaming oldPath to newPath in lower file system.
+ try {
+ Os.rename(oldPath, newPath);
+ } catch (ErrnoException e) {
+ Log.e(TAG, "Failed to rename " + oldPath, e);
+ return -e.errno;
+ }
+
+ db.setTransactionSuccessful();
+ } finally {
+ db.endTransaction();
+ }
+ // Process metadata in background thread.
+ postProcessMetadataForFuseRename(oldPath, newPath);
+ return 0;
+ }
+
+ /**
* Rename file or directory from {@code oldPath} to {@code newPath}.
*
* @param oldPath path of the file or directory to be renamed.
@@ -1054,12 +1239,13 @@
* <ul>
* <li>{@link OsConstants#ENOENT} Renaming a non-existing file or renaming a file from path that
* is not indexed by MediaProvider database.
- * <li>{@link OsConstants#EPERM} Renaming a default directory.
- * </ul>
+ * <li>{@link OsConstants#EPERM} Renaming a default directory or renaming a file to a file type
+ * not supported by new path.
+ *
* This method can also return errno returned from {@code Os.rename} function.
- * MediaProvider database entries corresponding to files/directories being renamed is not
- * updated on rename and hence, FUSE rename can make MediaProvider database inconsistent with
- * lower file system.
+ * MediaProvider database entries corresponding to directory and files/directories in the
+ * directory being renamed is not updated on rename directory and hence, FUSE rename directory
+ * can make MediaProvider database inconsistent with lower file system.
*
* Called from JNI in jni/MediaProviderWrapper.cpp
*/
@@ -1120,19 +1306,22 @@
}
// Continue renaming files/directories if rename of oldPath to newPath is allowed.
- try {
- Os.rename(oldPath, newPath);
- return 0;
- } catch (ErrnoException e) {
- Log.e(TAG, errorMessage, e);
- return -e.errno;
+ if (new File(oldPath).isFile()) {
+ return renameFileForFuse(oldPath, newPath);
+ } else {
+ try {
+ Os.rename(oldPath, newPath);
+ return 0;
+ } catch (ErrnoException e) {
+ Log.e(TAG, errorMessage, e);
+ return -e.errno;
+ }
}
} finally {
restoreLocalCallingIdentity(token);
}
}
-
@Override
public int checkUriPermission(@NonNull Uri uri, int uid,
/* @Intent.AccessUriMode */ int modeFlags) {
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 1bab94f..1df6211 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
@@ -155,6 +155,21 @@
}
@Test
+ public void testRenameFileType() throws Exception {
+ runDeviceTest("testRenameFileType");
+ }
+
+ @Test
+ public void testRenameAndReplaceFile() throws Exception {
+ runDeviceTest("testRenameAndReplaceFile");
+ }
+
+ @Test
+ public void testRenameFileNotOwned() throws Exception {
+ runDeviceTest("testRenameFileNotOwned");
+ }
+
+ @Test
public void testRenameDirectory() throws Exception {
runDeviceTest("testRenameDirectory");
}
diff --git a/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java b/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java
index 115e4c9..b4202e5 100644
--- a/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java
+++ b/tests/jni/FuseDaemonTest/libs/FuseDaemonTestLib/src/com/android/tests/fused/lib/TestUtils.java
@@ -196,6 +196,47 @@
}
}
+ /**
+ * Queries {@link ContentResolver} for a file and returns the corresponding row ID for its
+ * entry in the database.
+ */
+ @NonNull
+ public static int getFileRowIdFromDatabase(@NonNull ContentResolver cr, @NonNull File file) {
+ int id = -1;
+ final Uri contentUri = MediaStore.Files.getContentUri(MediaStore.VOLUME_EXTERNAL);
+ try (Cursor c = cr.query(contentUri,
+ /*projection*/ new String[] { MediaStore.MediaColumns._ID },
+ /*selection*/ MediaStore.MediaColumns.DATA + " = ?",
+ /*selectionArgs*/ new String[] { file.getAbsolutePath() },
+ /*sortOrder*/ null)) {
+ if (c.moveToFirst()) {
+ id = c.getInt(0);
+ }
+ }
+ return id;
+ }
+
+ /**
+ * Queries {@link ContentResolver} for a file and returns the corresponding mime type for its
+ * entry in the database.
+ */
+ @NonNull
+ public static String getFileMimeTypeFromDatabase(@NonNull ContentResolver cr,
+ @NonNull File file) {
+ final Uri contentUri = MediaStore.Files.getContentUri(MediaStore.VOLUME_EXTERNAL);
+ String mimeType = "";
+ try (Cursor c = cr.query(contentUri,
+ /*projection*/ new String[] { MediaStore.MediaColumns.MIME_TYPE},
+ /*selection*/ MediaStore.MediaColumns.DATA + " = ?",
+ /*selectionArgs*/ new String[] { file.getAbsolutePath() },
+ /*sortOrder*/ null)) {
+ if(c.moveToFirst()) {
+ mimeType = c.getString(0);
+ }
+ }
+ return mimeType;
+ }
+
public static <T extends Exception> void assertThrows(Class<T> clazz, Operation<T> r)
throws Exception {
assertThrows(clazz, "", r);
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 028afa9..2144e82 100644
--- a/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
+++ b/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
@@ -29,6 +29,8 @@
import static com.android.tests.fused.lib.TestUtils.createFileAs;
import static com.android.tests.fused.lib.TestUtils.deleteFileAs;
import static com.android.tests.fused.lib.TestUtils.executeShellCommand;
+import static com.android.tests.fused.lib.TestUtils.getFileMimeTypeFromDatabase;
+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.listAs;
import static com.android.tests.fused.lib.TestUtils.readExifMetadataFromTestApp;
@@ -41,7 +43,6 @@
import android.content.ContentResolver;
import android.content.ContentUris;
import android.content.ContentValues;
-import android.content.Context;
import android.database.Cursor;
import android.net.Uri;
import android.os.Environment;
@@ -53,13 +54,9 @@
import android.system.OsConstants;
import android.util.Log;
-import androidx.test.InstrumentationRegistry;
import androidx.test.runner.AndroidJUnit4;
-import com.android.cts.install.lib.Install;
import com.android.cts.install.lib.TestApp;
-import com.android.cts.install.lib.TestApp;
-import com.android.tests.fused.lib.ReaddirTestHelper;
import com.android.tests.fused.lib.ReaddirTestHelper;
import com.google.common.io.ByteStreams;
@@ -433,7 +430,7 @@
}
/**
- * Test that app can not see non-media files created by other packages
+ * Test that app can't see non-media files created by other packages
*/
@Test
public void testListDirectoriesWithNonMediaFiles() throws Exception {
@@ -500,6 +497,7 @@
assertThat(listAs(TEST_APP_A, EXTERNAL_FILES_DIR.getPath())).isEmpty();
} finally {
videoFile.delete();
+ uninstallApp(TEST_APP_A);
}
}
@@ -541,6 +539,8 @@
// .containsExactly(videoFileName);
} finally {
videoFile.delete();
+ // TODO(b/145757667): Uncomment this when we start indexing Android/media files.
+// uninstallApp(TEST_APP_A);
}
}
@@ -567,6 +567,7 @@
} finally {
executeShellCommand("rm " + pdfFile.getAbsolutePath());
executeShellCommand("rm " + videoFile.getAbsolutePath());
+ uninstallApp(TEST_APP_A);
}
}
@@ -777,31 +778,24 @@
final File videoFile3 = new File(DOWNLOAD_DIR, VIDEO_FILE_NAME);
try {
- // Rename Non-media file
+ // Renaming non media file to media directory is not allowed.
assertThat(pdfFile1.createNewFile()).isTrue();
+ assertCantRenameFile(pdfFile1, new File(DCIM_DIR, NONMEDIA_FILE_NAME));
+ assertCantRenameFile(pdfFile1, new File(MUSIC_DIR, NONMEDIA_FILE_NAME));
+ assertCantRenameFile(pdfFile1, new File(MOVIES_DIR, NONMEDIA_FILE_NAME));
+
+ // Renaming non media files to non media directories is allowed.
if (!nonMediaDir.exists()) {
assertThat(nonMediaDir.mkdirs()).isTrue();
}
- assertThat(pdfFile1.renameTo(pdfFile2)).isTrue();
- assertThat(pdfFile1.exists()).isFalse();
- assertThat(pdfFile2.exists()).isTrue();
+ // App can rename pdfFile to non media directory.
+ assertCanRenameFile(pdfFile1, pdfFile2);
- assertThat(pdfFile2.renameTo(pdfFile1)).isTrue();
- assertThat(pdfFile2.exists()).isFalse();
- assertThat(pdfFile1.exists()).isTrue();
-
- // Rename media file
assertThat(videoFile1.createNewFile()).isTrue();
- assertThat(videoFile1.renameTo(videoFile2)).isTrue();
- assertThat(videoFile1.exists()).isFalse();
- assertThat(videoFile2.exists()).isTrue();
-
- assertThat(videoFile2.renameTo(videoFile3)).isTrue();
- assertThat(videoFile2.exists()).isFalse();
- assertThat(videoFile3.exists()).isTrue();
-
- // Move video file back to DCIM to ensure database entry is deleted on delete().
- assertThat(videoFile3.renameTo(videoFile1)).isTrue();
+ // App can rename video file to Movies directory
+ assertCanRenameFile(videoFile1, videoFile2);
+ // App can rename video file to Download directory
+ assertCanRenameFile(videoFile2, videoFile3);
} finally {
pdfFile1.delete();
pdfFile2.delete();
@@ -813,6 +807,86 @@
}
/**
+ * Test that renaming file to different mime type is allowed.
+ */
+ @Test
+ public void testRenameFileType() throws Exception {
+ final File pdfFile = new File(DOWNLOAD_DIR, NONMEDIA_FILE_NAME);
+ final File videoFile = new File(DCIM_DIR, VIDEO_FILE_NAME);
+ try {
+ assertThat(pdfFile.createNewFile()).isTrue();
+ assertThat(videoFile.exists()).isFalse();
+ // Moving pdfFile to DCIM directory is not allowed.
+ assertCantRenameFile(pdfFile, new File(DCIM_DIR, NONMEDIA_FILE_NAME));
+ // However, moving pdfFile to DCIM directory with changing the mime type to video is
+ // allowed.
+ assertCanRenameFile(pdfFile, videoFile);
+
+ // On rename, MediaProvider database entry for pdfFile should be updated with new
+ // videoFile path and mime type should be updated to video/mp4.
+ ContentResolver cr = getContentResolver();
+ assertThat(getFileMimeTypeFromDatabase(cr, videoFile))
+ .isEqualTo("video/mp4");
+ } finally {
+ pdfFile.delete();
+ videoFile.delete();
+ }
+ }
+
+ /**
+ * Test that renaming files overwrites files in newPath.
+ */
+ @Test
+ public void testRenameAndReplaceFile() throws Exception {
+ final File videoFile1 = new File(DCIM_DIR, VIDEO_FILE_NAME);
+ final File videoFile2 = new File(MOVIES_DIR, VIDEO_FILE_NAME);
+ try {
+ assertThat(videoFile1.createNewFile()).isTrue();
+ assertThat(videoFile2.createNewFile()).isTrue();
+ ContentResolver cr = getContentResolver();
+ final String[] projection = new String[] {MediaColumns._ID};
+ // Get id of video file in movies which will be deleted on rename.
+ final int id = getFileRowIdFromDatabase(cr, videoFile2);
+
+ // Renaming a file which replaces file in newPath videoFile2 is allowed.
+ assertCanRenameFile(videoFile1, videoFile2);
+
+ // MediaProvider database entry for videoFile2 should be deleted on rename.
+ assertThat(getFileRowIdFromDatabase(cr, videoFile2)).isNotEqualTo((id));
+ } finally {
+ videoFile1.delete();
+ videoFile2.delete();
+ }
+ }
+
+ /**
+ * Test that app without write permission for file can't update the file.
+ */
+ @Test
+ public void testRenameFileNotOwned() throws Exception {
+ final File videoFile1 = new File(DCIM_DIR, VIDEO_FILE_NAME);
+ final File videoFile2 = new File(MOVIES_DIR, VIDEO_FILE_NAME);
+ try {
+ installApp(TEST_APP_A, false);
+ assertThat(createFileAs(TEST_APP_A, videoFile1.getAbsolutePath())).isTrue();
+ // App can't rename a file owned by TEST_APP_A.
+ assertCantRenameFile(videoFile1, videoFile2);
+
+ assertThat(videoFile2.createNewFile()).isTrue();
+ // App can't rename a file to videoFile1 which is owned by TEST_APP_A
+ assertCantRenameFile(videoFile2, videoFile1);
+ // TODO(b/146346138): Test that app with right URI permission should be able to rename
+ // the corresponding file
+ } finally {
+ if(videoFile1.exists()) {
+ deleteFileAs(TEST_APP_A, videoFile1.getAbsolutePath());
+ }
+ videoFile2.delete();
+ uninstallApp(TEST_APP_A);
+ }
+ }
+
+ /**
* Test that renaming directories is allowed and aligns to default directory restrictions.
*/
@Test
@@ -984,6 +1058,23 @@
}
}
+ private static void assertCanRenameFile(File oldFile, File newFile) {
+ assertThat(oldFile.renameTo(newFile)).isTrue();
+ assertThat(oldFile.exists()).isFalse();
+ assertThat(newFile.exists()).isTrue();
+ ContentResolver cr = getContentResolver();
+ assertThat(getFileRowIdFromDatabase(cr, oldFile)).isEqualTo(-1);
+ assertThat(getFileRowIdFromDatabase(cr, newFile)).isNotEqualTo(-1);
+ }
+
+ private static void assertCantRenameFile(File oldFile, File newFile) {
+ ContentResolver cr = getContentResolver();
+ final int rowId = getFileRowIdFromDatabase(cr, oldFile);
+ assertThat(oldFile.renameTo(newFile)).isFalse();
+ assertThat(oldFile.exists()).isTrue();
+ assertThat(getFileRowIdFromDatabase(cr, oldFile)).isEqualTo(rowId);
+ }
+
/**
* Asserts the entire content of the file equals exactly {@code expectedContent}.
*/