Improve sharing of files from FUSE-based filesystems with Android

When requests are routed back to ChromeOS via ChromeContentProvider, do
not use virtual_file_provider for opening files from FUSE-backed
filesystems.

This expands existing support for DriveFS to Crostini, smbfs and
FUSE-backed archive implementations (fuse-zip, rar2fs).

Test: unit_tests --gtest_filter="*ArcFileSystemBridgeTest*"
Test: Open mp4 file from Crostini sshfs, fuse-zip and smbfs with VLC
Bug: 1132141
Change-Id: I29ec9f99664928b93061b02b94149d761e186a97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2430524Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: François Degros <fdegros@chromium.org>
Commit-Queue: Josh Simmons <simmonsjosh@google.com>
Cr-Commit-Position: refs/heads/master@{#823061}

chrome/browser/chromeos/arc/fileapi/arc_file_system_bridge.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/chromeos/drive/drive_integration_service.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
 #include "chrome/browser/chromeos/file_manager/fileapi_util.h"
+#include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/chromeos/fileapi/external_file_url_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -37,13 +38,13 @@
 #include "mojo/public/cpp/platform/platform_handle.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 #include "net/base/escape.h"
+#include "storage/browser/file_system/external_mount_points.h"
 #include "storage/browser/file_system/file_system_context.h"
 #include "url/gurl.h"
 
 namespace {
 constexpr char kChromeOSReleaseTrack[] = "CHROMEOS_RELEASE_TRACK";
 constexpr char kTestImageRelease[] = "testimage-channel";
-constexpr char kDriveFSPrefix[] = "drivefs-";
 }  // namespace
 
 namespace arc {
@@ -107,38 +108,9 @@ void GetFileSizeOnIOThread(scoped_refptr<storage::FileSystemContext> context,
           base::Passed(&callback)));
 }
 
-// Decodes a percent-encoded URL to the drivefs path in the filesystem.
-base::FilePath GetDriveFSPathFromURL(Profile* const profile, const GURL& url) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  // If DriveFS is not mounted, return an empty base::FilePath().
-  if (!drive::util::GetIntegrationServiceByProfile(profile) ||
-      !drive::util::GetIntegrationServiceByProfile(profile)->GetDriveFsHost()) {
-    return base::FilePath();
-  }
-  base::FilePath virtual_path = chromeos::ExternalFileURLToVirtualPath(url);
-  std::vector<base::FilePath::StringType> virtual_path_components;
-  virtual_path.GetComponents(&virtual_path_components);
-
-  // If the path is not DriveFS prefixed, then it might be FSP/MTP.
-  if (virtual_path_components.empty() ||
-      !base::StartsWith(virtual_path_components[0], kDriveFSPrefix,
-                        base::CompareCase::SENSITIVE)) {
-    return base::FilePath();
-  }
-
-  // Construct the path.
-  base::FilePath drivefs_path =
-      drive::util::GetIntegrationServiceByProfile(profile)->GetMountPointPath();
-  DCHECK(!drivefs_path.empty());
-  for (size_t i = 1; i < virtual_path_components.size(); i++) {
-    drivefs_path = drivefs_path.Append(virtual_path_components[i]);
-  }
-  return drivefs_path;
-}
-
 // TODO(risan): Write test.
-// Open DriveFS file from the fuse filesystem.
-mojo::ScopedHandle OpenDriveFSFileToRead(const base::FilePath& fs_path) {
+// Open a file from a VFS (vs Chrome-only) filesystem.
+mojo::ScopedHandle OpenVFSFileToRead(const base::FilePath& fs_path) {
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::WILL_BLOCK);
   // Open the file and wrap the fd to be returned through mojo.
@@ -331,11 +303,10 @@ void ArcFileSystemBridge::OpenFileToRead(const std::string& url,
     return;
   }
 
-  // TODO(risan): Remove the fallback path in M75+ after DriveFS is always
-  // enabled.
-  base::FilePath fs_path = GetDriveFSPathFromURL(profile_, url_decoded);
-  // If either DriveFS is not enabled/not mounted, or the URL doesn't represent
-  // drivefs file (e.g., FSP, MTP), use VirtualFileProvider instead.
+  base::FilePath fs_path =
+      GetLinuxVFSPathFromExternalFileURL(profile_, url_decoded);
+  // If the URL represents a file on a virtual (Chrome-only, e.g., FSP,
+  // MTP) filesystem, use VirtualFileProvider instead.
   if (fs_path.empty()) {
     GetVirtualFileIdInternal(
         url_decoded, base::BindOnce(&ArcFileSystemBridge::OpenFileById,
@@ -346,7 +317,7 @@ void ArcFileSystemBridge::OpenFileToRead(const std::string& url,
 
   base::ThreadPool::PostTaskAndReplyWithResult(
       FROM_HERE, {base::MayBlock()},
-      base::BindOnce(&OpenDriveFSFileToRead, fs_path), std::move(callback));
+      base::BindOnce(&OpenVFSFileToRead, fs_path), std::move(callback));
 }
 
 void ArcFileSystemBridge::GetVirtualFileIdInternal(
@@ -512,4 +483,57 @@ void ArcFileSystemBridge::OnConnectionClosed() {
   select_files_handlers_manager_->DeleteAllHandlers();
 }
 
+base::FilePath ArcFileSystemBridge::GetLinuxVFSPathFromExternalFileURL(
+    Profile* const profile,
+    const GURL& url) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  base::FilePath virtual_path = chromeos::ExternalFileURLToVirtualPath(url);
+
+  std::string mount_name, cracked_id;
+  storage::FileSystemType file_system_type;
+  base::FilePath absolute_path;
+  storage::FileSystemMountOption mount_option;
+
+  if (!storage::ExternalMountPoints::GetSystemInstance()->CrackVirtualPath(
+          virtual_path, &mount_name, &file_system_type, &cracked_id,
+          &absolute_path, &mount_option)) {
+    LOG(WARNING) << "Couldn't find mount point for: " << url;
+    return base::FilePath();
+  }
+
+  return GetLinuxVFSPathForPathOnFileSystemType(profile, absolute_path,
+                                                file_system_type);
+}
+
+base::FilePath ArcFileSystemBridge::GetLinuxVFSPathForPathOnFileSystemType(
+    Profile* const profile,
+    const base::FilePath& path,
+    storage::FileSystemType file_system_type) {
+  switch (file_system_type) {
+    case storage::FileSystemType::kFileSystemTypeDriveFs:
+    case storage::FileSystemType::kFileSystemTypeSmbFs:
+      return path;
+    case storage::FileSystemType::kFileSystemTypeNativeLocal: {
+      base::FilePath crostini_mount_path =
+          file_manager::util::GetCrostiniMountDirectory(profile);
+      if (crostini_mount_path == path || crostini_mount_path.IsParent(path))
+        return path;
+
+      // fuse-zip, rar2fs.
+      base::FilePath archive_mount_path =
+          base::FilePath(file_manager::util::kArchiveMountPath);
+      if (archive_mount_path.IsParent(path))
+        return path;
+
+      break;
+    }
+    default:
+      break;
+  }
+
+  // The path is not representable on the Linux VFS.
+  return base::FilePath();
+}
+
 }  // namespace arc

chrome/browser/chromeos/arc/fileapi/arc_file_system_bridge.h

@@ -12,6 +12,7 @@
 #include <memory>
 #include <string>
 
+#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
@@ -107,6 +108,11 @@ class ArcFileSystemBridge
   void OnConnectionClosed() override;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemBridgeTest,
+                           GetLinuxVFSPathFromExternalFileURL);
+  FRIEND_TEST_ALL_PREFIXES(ArcFileSystemBridgeTest,
+                           GetLinuxVFSPathForPathOnFileSystemType);
+
   using GenerateVirtualFileIdCallback =
       base::OnceCallback<void(const base::Optional<std::string>& id)>;
 
@@ -139,6 +145,27 @@ class ArcFileSystemBridge
                       const std::string& id,
                       base::ScopedFD fd);
 
+  // Used to implement OpenFileToRead(), needs to be testable.
+  //
+  // Decode a percent-encoded externalfile: URL to an absolute path on
+  // the Linux VFS (virtual file system). This returns a non-empty path
+  // for FUSE filesystems (ie. DriveFS, SmbFs, archives) that utilise FD
+  // passing and externalfile: in file_manager::util::ConvertPathToArcUrl().
+  // Returns an empty path for Chrome's virtual filesystems that are not exposed
+  // on the Linux VFS (ie. MTP, FSP).
+  base::FilePath GetLinuxVFSPathFromExternalFileURL(Profile* const profile,
+                                                    const GURL& url);
+
+  // Used to implement OpenFileToRead(), needs to be testable.
+  //
+  // Takes a path within the mount namespace of a specific FileSystemType and
+  // returns the path on the Linux VFS, if it exists, or an empty path
+  // otherwise.
+  base::FilePath GetLinuxVFSPathForPathOnFileSystemType(
+      Profile* const profile,
+      const base::FilePath& path,
+      storage::FileSystemType file_system_type);
+
   // Called when FileStreamForwarder completes read request.
   void OnReadRequestCompleted(const std::string& id,
                               std::list<FileStreamForwarderPtr>::iterator it,

chrome/browser/chromeos/arc/fileapi/arc_file_system_bridge_unittest.cc

@@ -14,9 +14,12 @@
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chromeos/arc/fileapi/chrome_content_provider_url_util.h"
+#include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/chromeos/file_system_provider/fake_extension_provider.h"
 #include "chrome/browser/chromeos/file_system_provider/service.h"
 #include "chrome/browser/chromeos/file_system_provider/service_factory.h"
+#include "chrome/browser/chromeos/fileapi/external_file_url_util.h"
+#include "chrome/browser/chromeos/fileapi/file_system_backend.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -55,20 +58,19 @@ class ArcFileSystemBridgeTest : public testing::Test {
     profile_manager_ = std::make_unique<TestingProfileManager>(
         TestingBrowserProcess::GetGlobal());
     ASSERT_TRUE(profile_manager_->SetUp());
-    Profile* profile =
-        profile_manager_->CreateTestingProfile(kTestingProfileName);
+    profile_ = profile_manager_->CreateTestingProfile(kTestingProfileName);
     auto fake_provider =
         chromeos::file_system_provider::FakeExtensionProvider::Create(
             kExtensionId);
     const auto kProviderId = fake_provider->GetId();
-    auto* service = chromeos::file_system_provider::Service::Get(profile);
+    auto* service = chromeos::file_system_provider::Service::Get(profile_);
     service->RegisterProvider(std::move(fake_provider));
     service->MountFileSystem(kProviderId,
                              chromeos::file_system_provider::MountOptions(
                                  kFileSystemId, "Test FileSystem"));
 
     arc_file_system_bridge_ =
-        std::make_unique<ArcFileSystemBridge>(profile, &arc_bridge_service_);
+        std::make_unique<ArcFileSystemBridge>(profile_, &arc_bridge_service_);
     arc_bridge_service_.file_system()->SetInstance(&fake_file_system_);
     WaitForInstanceReady(arc_bridge_service_.file_system());
   }
@@ -84,6 +86,7 @@ class ArcFileSystemBridgeTest : public testing::Test {
   base::ScopedTempDir temp_dir_;
   content::BrowserTaskEnvironment task_environment_;
   std::unique_ptr<TestingProfileManager> profile_manager_;
+  Profile* profile_ = nullptr;
 
   FakeFileSystemInstance fake_file_system_;
   ArcBridgeService arc_bridge_service_;
@@ -261,4 +264,90 @@ TEST_F(ArcFileSystemBridgeTest, OpenFileToRead) {
   EXPECT_TRUE(arc_file_system_bridge_->HandleIdReleased(kId));
 }
 
+TEST_F(ArcFileSystemBridgeTest, GetLinuxVFSPathFromExternalFileURL) {
+  storage::ExternalMountPoints* system_mount_points =
+      storage::ExternalMountPoints::GetSystemInstance();
+
+  // Check: FSPs aren't visible on the VFS so should yield no path.
+  base::FilePath fsp_path =
+      arc_file_system_bridge_->GetLinuxVFSPathFromExternalFileURL(
+          profile_, GURL(kTestUrl));
+  EXPECT_EQ(fsp_path, base::FilePath());
+
+  // SmbFs is visible on the VFS, so should yield a path.
+  constexpr char kSmbFsTestMountName[] = "test-smb";
+  constexpr char kSmbFsTestMountPoint[] = "/dummy/mount";
+  constexpr char kTestPathInsideMount[] = "path/to/file";
+  EXPECT_TRUE(system_mount_points->RegisterFileSystem(
+      kSmbFsTestMountName, storage::FileSystemType::kFileSystemTypeSmbFs, {},
+      base::FilePath(kSmbFsTestMountPoint)));
+
+  base::FilePath smbfs_path_expected(
+      base::FilePath(kSmbFsTestMountPoint).Append(kTestPathInsideMount));
+
+  // Create externalfile: URL as would be encoded inside the
+  // ChromeContentProvider URL.
+  GURL smbfs_url = chromeos::CreateExternalFileURLFromPath(
+      profile_, smbfs_path_expected, true);
+
+  // Check: The path returned matches the path encoded into the URL.
+  base::FilePath smbfs_path =
+      arc_file_system_bridge_->GetLinuxVFSPathFromExternalFileURL(profile_,
+                                                                  smbfs_url);
+  EXPECT_EQ(smbfs_path, smbfs_path_expected);
+  system_mount_points->RevokeFileSystem(kSmbFsTestMountName);
+}
+
+TEST_F(ArcFileSystemBridgeTest, GetLinuxVFSPathForPathOnFileSystemType) {
+  // Check: DriveFS paths are returned as passed in.
+  const base::FilePath filesystem_path("/path/on/filesystem/file");
+  base::FilePath drivefs_vfs_path =
+      arc_file_system_bridge_->GetLinuxVFSPathForPathOnFileSystemType(
+          profile_, filesystem_path, storage::kFileSystemTypeDriveFs);
+  EXPECT_EQ(drivefs_vfs_path, filesystem_path);
+
+  // Check: SmbFs paths are returned as passed in.
+  base::FilePath smbfs_vfs_path =
+      arc_file_system_bridge_->GetLinuxVFSPathForPathOnFileSystemType(
+          profile_, filesystem_path, storage::kFileSystemTypeSmbFs);
+  EXPECT_EQ(smbfs_vfs_path, filesystem_path);
+
+  // Check: Crostini paths are returned as passed in.
+  const base::FilePath crostini_path =
+      file_manager::util::GetCrostiniMountDirectory(profile_).Append(
+          "path/to/file");
+  base::FilePath crostini_vfs_path =
+      arc_file_system_bridge_->GetLinuxVFSPathForPathOnFileSystemType(
+          profile_, crostini_path, storage::kFileSystemTypeNativeLocal);
+  EXPECT_EQ(crostini_vfs_path, crostini_path);
+
+  // Check: fuse-zip and rar2fs paths are returned as passed in.
+  const base::FilePath archive_path =
+      base::FilePath(file_manager::util::kArchiveMountPath)
+          .Append("path/to/file");
+  base::FilePath archive_vfs_path =
+      arc_file_system_bridge_->GetLinuxVFSPathForPathOnFileSystemType(
+          profile_, archive_path, storage::kFileSystemTypeNativeLocal);
+  EXPECT_EQ(archive_vfs_path, archive_path);
+
+  // Check: Other kFileSystemTypeNativeLocal paths that are not descendants of
+  // the Crostini, fuse-zip or rar2fs mount points return an empty path.
+  const base::FilePath empty_path,
+      unsupported_local_path = base::FilePath("/path/to/file");
+  base::FilePath unsupported_local_vfs_path =
+      arc_file_system_bridge_->GetLinuxVFSPathForPathOnFileSystemType(
+          profile_, unsupported_local_path,
+          storage::kFileSystemTypeNativeLocal);
+  EXPECT_EQ(empty_path, unsupported_local_vfs_path);
+
+  // Check: Paths from unsupported FileSystemTypes return an empty path.
+  const base::FilePath unsupported_filesystem_path =
+      base::FilePath("/special/path");
+  base::FilePath unsupported_filesystem_vfs_path =
+      arc_file_system_bridge_->GetLinuxVFSPathForPathOnFileSystemType(
+          profile_, unsupported_filesystem_path,
+          storage::kFileSystemTypeProvided);
+  EXPECT_EQ(empty_path, unsupported_filesystem_vfs_path);
+}
+
 }  // namespace arc