Merge "Fix setting timestamps via FUSE" into rvc-dev
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index d661b59..5660c9b 100644
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -1531,16 +1531,22 @@
 void FuseDaemon::InvalidateFuseDentryCache(const std::string& path) {
     LOG(VERBOSE) << "Invalidating FUSE dentry cache";
     if (active.load(std::memory_order_acquire)) {
-        std::lock_guard<std::recursive_mutex> guard(fuse->lock);
+        string name;
+        fuse_ino_t parent;
 
-        const node* node = node::LookupAbsolutePath(fuse->root, path);
-        if (node) {
-            string name = node->GetName();
-            fuse_ino_t parent = fuse->ToInode(node->GetParent());
-            if (fuse_lowlevel_notify_inval_entry(fuse->se, parent, name.c_str(), name.size())) {
-                LOG(WARNING) << "Failed to invalidate dentry for path";
+        {
+            std::lock_guard<std::recursive_mutex> guard(fuse->lock);
+            const node* node = node::LookupAbsolutePath(fuse->root, path);
+            if (node) {
+                name = node->GetName();
+                parent = fuse->ToInode(node->GetParent());
             }
         }
+
+        if (!name.empty() &&
+            fuse_lowlevel_notify_inval_entry(fuse->se, parent, name.c_str(), name.size())) {
+            LOG(WARNING) << "Failed to invalidate dentry for path";
+        }
     } else {
         LOG(WARNING) << "FUSE daemon is inactive. Cannot invalidate dentry";
     }
@@ -1553,7 +1559,7 @@
     return active.load(std::memory_order_acquire);
 }
 
-void FuseDaemon::Start(const int fd, const std::string& path) {
+void FuseDaemon::Start(android::base::unique_fd fd, const std::string& path) {
     struct fuse_args args;
     struct fuse_cmdline_opts opts;
 
diff --git a/jni/FuseDaemon.h b/jni/FuseDaemon.h
index d9925d4..3c4b947 100644
--- a/jni/FuseDaemon.h
+++ b/jni/FuseDaemon.h
@@ -20,6 +20,8 @@
 #include <memory>
 #include <string>
 
+#include <android-base/unique_fd.h>
+
 #include "MediaProviderWrapper.h"
 #include "jni.h"
 
@@ -35,7 +37,7 @@
     /**
      * Start the FUSE daemon loop that will handle filesystem calls.
      */
-    void Start(const int fd, const std::string& path);
+    void Start(android::base::unique_fd fd, const std::string& path);
 
     /**
      * Checks if the FUSE daemon is started.
diff --git a/jni/com_android_providers_media_FuseDaemon.cpp b/jni/com_android_providers_media_FuseDaemon.cpp
index ae1c175..3a65696 100644
--- a/jni/com_android_providers_media_FuseDaemon.cpp
+++ b/jni/com_android_providers_media_FuseDaemon.cpp
@@ -24,6 +24,7 @@
 #include "FuseDaemon.h"
 #include "MediaProviderWrapper.h"
 #include "android-base/logging.h"
+#include "android-base/unique_fd.h"
 
 namespace mediaprovider {
 namespace {
@@ -42,12 +43,14 @@
     LOG(DEBUG) << "Starting the FUSE daemon...";
     fuse::FuseDaemon* const daemon = reinterpret_cast<fuse::FuseDaemon*>(java_daemon);
 
+    android::base::unique_fd ufd(fd);
+
     ScopedUtfChars utf_chars_path(env, java_path);
     if (!utf_chars_path.c_str()) {
         return;
     }
 
-    daemon->Start(fd, utf_chars_path.c_str());
+    daemon->Start(std::move(ufd), utf_chars_path.c_str());
 }
 
 bool com_android_providers_media_FuseDaemon_is_started(JNIEnv* env, jobject self,
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 4d2f2d5..952f895 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -1732,16 +1732,12 @@
      * However, we update database entries for renamed files to keep the database consistent.
      */
     private int renameUncheckedForFuse(String oldPath, String newPath) {
-
-        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));
-        // }
+        if (new File(oldPath).isFile()) {
+            return renameFileUncheckedForFuse(oldPath, newPath);
+        } else {
+            return renameDirectoryUncheckedForFuse(oldPath, newPath,
+                    getAllFilesForRenameDirectory(oldPath));
+        }
     }
 
     /**
@@ -2352,6 +2348,22 @@
                 }
             }
 
+            // Allow apps with MANAGE_EXTERNAL_STORAGE to create files anywhere
+            if (!validPath) {
+                validPath = isCallingPackageExternalStorageManager();
+            }
+
+            // Allow system gallery to create image/video files.
+            if (!validPath) {
+                // System gallery can create image/video files in any existing directory, it can
+                // also create subdirectories in any existing top-level directory. However, system
+                // gallery is not allowed to create non-default top level directory.
+                final boolean createNonDefaultTopLevelDir = primary != null &&
+                        !FileUtils.buildPath(volumePath, primary).exists();
+                validPath = !createNonDefaultTopLevelDir &&
+                        canAccessMediaFile(res.getAbsolutePath(), /*allowLegacy*/ false);
+            }
+
             // Nothing left to check; caller can't use this path
             if (!validPath) {
                 throw new IllegalArgumentException(
@@ -5628,7 +5640,10 @@
         return MimeUtils.resolveMediaType(mimeType);
     }
 
-    private boolean canAccessMediaFile(String filePath) {
+    private boolean canAccessMediaFile(String filePath, boolean allowLegacy) {
+        if (!allowLegacy && isCallingPackageRequestingLegacy()) {
+            return false;
+        }
         switch (getFileMediaType(filePath)) {
             case FileColumns.MEDIA_TYPE_IMAGE:
                 return mCallingIdentity.get().hasPermission(PERMISSION_WRITE_IMAGES);
@@ -5657,7 +5672,7 @@
             return true;
         }
 
-        if (mCallingIdentity.get().hasPermission(PERMISSION_MANAGE_EXTERNAL_STORAGE)) {
+        if (isCallingPackageExternalStorageManager()) {
             return true;
         }
 
@@ -5669,7 +5684,7 @@
 
         // Apps with write access to images and/or videos can bypass our restrictions if all of the
         // the files they're accessing are of the compatible media type.
-        if (canAccessMediaFile(filePath)) {
+        if (canAccessMediaFile(filePath, /*allowLegacy*/ true)) {
             return true;
         }
 
@@ -6056,6 +6071,21 @@
         return false;
     }
 
+    private Uri insertFileForFuse(@NonNull String path, @NonNull Uri uri, @NonNull String mimeType,
+            boolean useData) {
+        ContentValues values = new ContentValues();
+        values.put(FileColumns.OWNER_PACKAGE_NAME, getCallingPackageOrSelf());
+        values.put(MediaColumns.MIME_TYPE, mimeType);
+
+        if (useData) {
+            values.put(FileColumns.DATA, getAbsoluteSanitizedPath(path));
+        } else {
+            values.put(FileColumns.RELATIVE_PATH, extractRelativePath(path));
+            values.put(FileColumns.DISPLAY_NAME, extractDisplayName(path));
+        }
+        return insert(uri, values, Bundle.EMPTY);
+    }
+
     /**
      * Enforces file creation restrictions (see return values) for the given file on behalf of the
      * app with the given {@code uid}. If the file is is added to the shared storage, creates a
@@ -6089,32 +6119,27 @@
                 return OsConstants.ENOENT;
             }
 
+            final String mimeType = MimeUtils.resolveMimeType(new File(path));
+
             if (shouldBypassFuseRestrictions(/*forWrite*/ true, path)) {
+                // Ignore insert errors for apps that bypass scoped storage restriction.
+                insertFileForFuse(path, Files.getContentUriForPath(path), mimeType,
+                        /*useData*/ isCallingPackageRequestingLegacy());
                 return 0;
             }
+
             // 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()) {
                 return OsConstants.EPERM;
             }
 
-            final String mimeType = MimeUtils.resolveMimeType(new File(path));
             final Uri contentUri = getContentUriForFile(path, mimeType);
             if (fileExists(path, contentUri)) {
                 return OsConstants.EEXIST;
             }
 
-            final String displayName = extractDisplayName(path);
-            final String callingPackageName = getCallingPackageOrSelf();
-            final String relativePath = extractRelativePath(path);
-
-            ContentValues values = new ContentValues();
-            values.put(FileColumns.RELATIVE_PATH, relativePath);
-            values.put(FileColumns.DISPLAY_NAME, displayName);
-            values.put(FileColumns.OWNER_PACKAGE_NAME, callingPackageName);
-            values.put(MediaColumns.MIME_TYPE, mimeType);
-
-            final Uri item = insert(contentUri, values);
+            final Uri item = insertFileForFuse(path, contentUri, mimeType, /*useData*/ false);
             if (item == null) {
                 return OsConstants.EPERM;
             }
@@ -6164,14 +6189,11 @@
                 return OsConstants.ENOENT;
             }
 
-            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);
-            }
+            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;
             }
 
@@ -6185,6 +6207,9 @@
             final String[] whereArgs = {sanitizedPath};
 
             if (delete(contentUri, where, whereArgs) == 0) {
+                if (shouldBypass) {
+                    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
@@ -6318,7 +6343,7 @@
         }
 
         // Apps that have permission to manage external storage can work with all files
-        if (mCallingIdentity.get().hasPermission(PERMISSION_MANAGE_EXTERNAL_STORAGE)) {
+        if (isCallingPackageExternalStorageManager()) {
             return true;
         }
 
@@ -7056,6 +7081,11 @@
         return mCallingIdentity.get().hasPermission(PERMISSION_IS_LEGACY_READ);
     }
 
+    private boolean isCallingPackageExternalStorageManager() {
+        return mCallingIdentity.get().hasPermission(PERMISSION_MANAGE_EXTERNAL_STORAGE);
+    }
+
+
     @Override
     public void dump(FileDescriptor fd, PrintWriter writer, String[] args) {
         writer.println("mThumbSize=" + mThumbSize);
diff --git a/src/com/android/providers/media/fuse/FuseDaemon.java b/src/com/android/providers/media/fuse/FuseDaemon.java
index 95a160b..3b7a0f8 100644
--- a/src/com/android/providers/media/fuse/FuseDaemon.java
+++ b/src/com/android/providers/media/fuse/FuseDaemon.java
@@ -151,7 +151,10 @@
     }
 
     private native long native_new(MediaProvider mediaProvider);
+
+    // Takes ownership of the passed in file descriptor!
     private native void native_start(long daemon, int deviceFd, String path);
+
     private native void native_delete(long daemon);
     private native boolean native_should_open_with_fuse(long daemon, String path, boolean readLock,
             int fd);
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 259266b..7309cf3 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
@@ -129,7 +129,6 @@
         runDeviceTest("testListFilesFromExternalMediaDirectory");
     }
 
-    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testListUnsupportedFileType() throws Exception {
         final ITestDevice device = getDevice();
@@ -224,25 +223,21 @@
         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 856b040..fea4362 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
@@ -164,7 +164,6 @@
 
     }
 
-    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testCanRename_hasRW() throws Exception {
         runDeviceTest("testCanRename_hasRW");
@@ -181,7 +180,6 @@
         }
     }
 
-
     @Test
     public void testCantRename_noStoragePermission() throws Exception {
         revokePermissions("android.permission.WRITE_EXTERNAL_STORAGE",
@@ -194,13 +192,11 @@
         }
     }
 
-    @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");
@@ -216,7 +212,6 @@
         runDeviceTest("testRenameDoesntInvalidateUri_hasRW");
     }
 
-
     @Test
     public void testCanRenameAFileWithNoDBRow_hasRW() throws Exception {
         runDeviceTest("testCanRenameAFileWithNoDBRow_hasRW");
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 650eba7..2a5c842 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
@@ -297,7 +297,6 @@
      * 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);
@@ -404,7 +403,6 @@
      * 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);
@@ -442,7 +440,6 @@
      * 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);
@@ -501,7 +498,6 @@
         } finally {
             videoFile.delete();
             renamedVideoFile.delete();
-            MediaStore.scanFile(cr, renamedVideoFile);
         }
     }
 
@@ -545,7 +541,6 @@
         } finally {
             imageFile.delete();
             temporaryImageFile.delete();
-            MediaStore.scanFile(cr, imageFile);
         }
     }
 
@@ -586,7 +581,6 @@
         } finally {
             imageInNoMediaDir.delete();
             renamedImageInDCIM.delete();
-            MediaStore.scanFile(cr, renamedImageInDCIM);
             noMediaFile.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 bcb50a4..87f7641 100644
--- a/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
+++ b/tests/jni/FuseDaemonTest/src/com/android/tests/fused/FilePathAccessTest.java
@@ -669,7 +669,6 @@
     /**
      * Test that app can see files and directories in Android/media.
      */
-    @Ignore("Re-enable as part of b/145737191")
     @Test
     public void testListFilesFromExternalMediaDirectory() throws Exception {
         final File videoFile = new File(EXTERNAL_MEDIA_DIR, VIDEO_FILE_NAME);
@@ -702,7 +701,6 @@
     /**
      * 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);
@@ -718,9 +716,8 @@
 
 
             executeShellCommand("touch " + videoFile.getAbsolutePath());
-            // ScanFile doesn't insert an empty media file to database. Write some data to ensure
-            // file is inserted into database.
-            executeShellCommand("echo " + STR_DATA1 + " > " + videoFile.getAbsolutePath());
+            // We don't insert files to db for files created by shell.
+            assertThat(MediaStore.scanFile(getContentResolver(), videoFile)).isNotNull();
             // TEST_APP_A with storage permission should see video file in Music directory.
             assertThat(listAs(TEST_APP_A, MUSIC_DIR.getPath())).contains(VIDEO_FILE_NAME);
         } finally {
@@ -1203,7 +1200,6 @@
         }
     }
 
-    @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);
@@ -1245,7 +1241,7 @@
             imageFile.delete();
             videoFile.delete();
             topLevelVideoFile.delete();
-            musicFile.delete();
+            executeShellCommand("rm  " + musicFile.getAbsolutePath());
             denyAppOpsToUid(Process.myUid(), SYSTEM_GALERY_APPOPS);
         }
     }
@@ -1492,7 +1488,6 @@
         }
     }
 
-    @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);
@@ -1533,7 +1528,6 @@
         }
     }
 
-    @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);
@@ -1566,7 +1560,6 @@
         }
     }
 
-    @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);
@@ -1734,12 +1727,8 @@
             executeShellCommand("rm " + otherAppPdfFile2);
             otherAppImageFile1.delete();
             otherAppImageFile2.delete();
-            MediaStore.scanFile(getContentResolver(), otherAppImageFile1);
-            MediaStore.scanFile(getContentResolver(), otherAppImageFile2);
             otherAppVideoFile1.delete();
             otherAppVideoFile2.delete();
-            MediaStore.scanFile(getContentResolver(), otherAppVideoFile1);
-            MediaStore.scanFile(getContentResolver(), otherAppVideoFile2);
             otherAppPdfFile1.delete();
             otherAppPdfFile2.delete();
             dirInDcim.delete();