Cleanup file permission check code in FileAPIMessageFilter

- Move detailed permission check code into each MountPointProvider
- Add common enum for fileapi permission policy (webkit/fileapi/file_permission_policy.{h,cc})

BUG=none, cleanup only
TEST=content_browsertests:FileSystemLayoutTest.*,browser_tests:FileBrowser*)

Review URL: https://codereview.chromium.org/11804005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@176014 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 115 insertions, 27 deletions

diff --git a/webkit/chromeos/fileapi/cros_mount_point_provider.cc b/webkit/chromeos/fileapi/cros_mount_point_provider.cc
index f98b138..54beb96 100644
--- a/webkit/chromeos/fileapi/cros_mount_point_provider.cc
+++ b/webkit/chromeos/fileapi/cros_mount_point_provider.cc
@@ -235,9 +235,14 @@ fileapi::FileSystemFileUtil* CrosMountPointProvider::GetFileUtil(
   return local_file_util_.get();
 }
 
-FilePath CrosMountPointProvider::GetPathForPermissionsCheck(
-    const FilePath& virtual_path) const {
-  return virtual_path;
+fileapi::FilePermissionPolicy CrosMountPointProvider::GetPermissionPolicy(
+    const fileapi::FileSystemURL& url, int permissions) const {
+  if (url.mount_type() == fileapi::kFileSystemTypeIsolated) {
+    // Permissions in isolated filesystems should be examined with
+    // FileSystem permission.
+    return fileapi::FILE_PERMISSION_USE_FILESYSTEM_PERMISSION;
+  }
+  return fileapi::FILE_PERMISSION_USE_FILE_PERMISSION;
 }
 
 fileapi::FileSystemOperation* CrosMountPointProvider::CreateFileSystemOperation(
diff --git a/webkit/chromeos/fileapi/cros_mount_point_provider.h b/webkit/chromeos/fileapi/cros_mount_point_provider.h
index 8e32f4d..6f7b38b 100644
--- a/webkit/chromeos/fileapi/cros_mount_point_provider.h
+++ b/webkit/chromeos/fileapi/cros_mount_point_provider.h
@@ -57,8 +57,9 @@ class WEBKIT_STORAGE_EXPORT CrosMountPointProvider
   virtual bool IsRestrictedFileName(const FilePath& filename) const OVERRIDE;
   virtual fileapi::FileSystemFileUtil* GetFileUtil(
       fileapi::FileSystemType type) OVERRIDE;
-  virtual FilePath GetPathForPermissionsCheck(const FilePath& virtual_path)
-      const OVERRIDE;
+  virtual fileapi::FilePermissionPolicy GetPermissionPolicy(
+      const fileapi::FileSystemURL& url,
+      int permissions) const OVERRIDE;
   virtual fileapi::FileSystemOperation* CreateFileSystemOperation(
       const fileapi::FileSystemURL& url,
       fileapi::FileSystemContext* context,
diff --git a/webkit/fileapi/file_permission_policy.cc b/webkit/fileapi/file_permission_policy.cc
new file mode 100644
index 0000000..8c0fcbe
--- /dev/null
+++ b/webkit/fileapi/file_permission_policy.cc
@@ -0,0 +1,34 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "webkit/fileapi/file_permission_policy.h"
+
+#include "base/platform_file.h"
+
+namespace fileapi {
+
+const int kReadFilePermissions = base::PLATFORM_FILE_OPEN |
+                                 base::PLATFORM_FILE_READ |
+                                 base::PLATFORM_FILE_EXCLUSIVE_READ |
+                                 base::PLATFORM_FILE_ASYNC;
+
+const int kWriteFilePermissions = base::PLATFORM_FILE_OPEN |
+                                  base::PLATFORM_FILE_WRITE |
+                                  base::PLATFORM_FILE_EXCLUSIVE_WRITE |
+                                  base::PLATFORM_FILE_ASYNC |
+                                  base::PLATFORM_FILE_WRITE_ATTRIBUTES;
+
+const int kCreateFilePermissions = base::PLATFORM_FILE_CREATE;
+
+const int kOpenFilePermissions = base::PLATFORM_FILE_CREATE |
+                                 base::PLATFORM_FILE_OPEN_ALWAYS |
+                                 base::PLATFORM_FILE_CREATE_ALWAYS |
+                                 base::PLATFORM_FILE_OPEN_TRUNCATED |
+                                 base::PLATFORM_FILE_WRITE |
+                                 base::PLATFORM_FILE_EXCLUSIVE_WRITE |
+                                 base::PLATFORM_FILE_DELETE_ON_CLOSE |
+                                 base::PLATFORM_FILE_WRITE_ATTRIBUTES;
+
+
+}  // namespace fileapi
diff --git a/webkit/fileapi/file_permission_policy.h b/webkit/fileapi/file_permission_policy.h
new file mode 100644
index 0000000..df8e63e
--- /dev/null
+++ b/webkit/fileapi/file_permission_policy.h
@@ -0,0 +1,34 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef WEBKIT_FILEAPI_FILE_PERMISSION_POLICY_H_
+#define WEBKIT_FILEAPI_FILE_PERMISSION_POLICY_H_
+
+#include "webkit/storage/webkit_storage_export.h"
+
+namespace fileapi {
+
+WEBKIT_STORAGE_EXPORT extern const int kReadFilePermissions;
+WEBKIT_STORAGE_EXPORT extern const int kWriteFilePermissions;
+WEBKIT_STORAGE_EXPORT extern const int kCreateFilePermissions;
+WEBKIT_STORAGE_EXPORT extern const int kOpenFilePermissions;
+
+enum FilePermissionPolicy {
+  // Any access should be always denied.
+  FILE_PERMISSION_ALWAYS_DENY,
+
+  // Any access should be always allowed. (This should be used only for
+  // access to sandbox directories.)
+  FILE_PERMISSION_ALWAYS_ALLOW,
+
+  // Access should be examined by per-file permission policy.
+  FILE_PERMISSION_USE_FILE_PERMISSION,
+
+  // Access should be examined by per-filesystem permission policy.
+  FILE_PERMISSION_USE_FILESYSTEM_PERMISSION,
+};
+
+}  // namespace fileapi
+
+#endif  // WEBKIT_FILEAPI_FILE_PERMISSION_POLICY_H_
diff --git a/webkit/fileapi/file_system_mount_point_provider.h b/webkit/fileapi/file_system_mount_point_provider.h
index 7e2ef11..4f0950e 100644
--- a/webkit/fileapi/file_system_mount_point_provider.h
+++ b/webkit/fileapi/file_system_mount_point_provider.h
@@ -11,6 +11,7 @@
 #include "base/callback_forward.h"
 #include "base/file_path.h"
 #include "base/platform_file.h"
+#include "webkit/fileapi/file_permission_policy.h"
 #include "webkit/fileapi/file_system_types.h"
 #include "webkit/storage/webkit_storage_export.h"
 
@@ -68,10 +69,10 @@ class WEBKIT_STORAGE_EXPORT FileSystemMountPointProvider {
   // Returns the specialized FileSystemFileUtil for this mount point.
   virtual FileSystemFileUtil* GetFileUtil(FileSystemType type) = 0;
 
-  // Returns file path we should use to check access permissions for
-  // |virtual_path|.
-  virtual FilePath GetPathForPermissionsCheck(const FilePath& virtual_path)
-      const = 0;
+  // Returns file permission policy we should apply for the given |url|.
+  virtual FilePermissionPolicy GetPermissionPolicy(
+      const FileSystemURL& url,
+      int permissions) const = 0;
 
   // Returns a new instance of the specialized FileSystemOperation for this
   // mount point based on the given triplet of |origin_url|, |file_system_type|
diff --git a/webkit/fileapi/isolated_mount_point_provider.cc b/webkit/fileapi/isolated_mount_point_provider.cc
index 7e72f4a..1bbb940 100644
--- a/webkit/fileapi/isolated_mount_point_provider.cc
+++ b/webkit/fileapi/isolated_mount_point_provider.cc
@@ -100,10 +100,16 @@ FileSystemFileUtil* IsolatedMountPointProvider::GetFileUtil(
   return NULL;
 }
 
-FilePath IsolatedMountPointProvider::GetPathForPermissionsCheck(
-    const FilePath& virtual_path) const {
-  // For isolated filesystems we only check per-filesystem permissions.
-  return FilePath();
+FilePermissionPolicy IsolatedMountPointProvider::GetPermissionPolicy(
+    const FileSystemURL& url, int permissions) const {
+  if (url.type() == kFileSystemTypeDragged && url.path().empty()) {
+    // The root directory of the dragged filesystem must be always read-only.
+    if (permissions != kReadFilePermissions)
+      return FILE_PERMISSION_ALWAYS_DENY;
+  }
+  // Access to isolated file systems should be checked using per-filesystem
+  // access permission.
+  return FILE_PERMISSION_USE_FILESYSTEM_PERMISSION;
 }
 
 FileSystemOperation* IsolatedMountPointProvider::CreateFileSystemOperation(
diff --git a/webkit/fileapi/isolated_mount_point_provider.h b/webkit/fileapi/isolated_mount_point_provider.h
index a4788d7..63964d1 100644
--- a/webkit/fileapi/isolated_mount_point_provider.h
+++ b/webkit/fileapi/isolated_mount_point_provider.h
@@ -38,8 +38,9 @@ class IsolatedMountPointProvider : public FileSystemMountPointProvider {
   virtual bool IsAccessAllowed(const FileSystemURL& url) OVERRIDE;
   virtual bool IsRestrictedFileName(const FilePath& filename) const OVERRIDE;
   virtual FileSystemFileUtil* GetFileUtil(FileSystemType type) OVERRIDE;
-  virtual FilePath GetPathForPermissionsCheck(const FilePath& virtual_path)
-      const OVERRIDE;
+  virtual FilePermissionPolicy GetPermissionPolicy(
+      const FileSystemURL& url,
+      int permissions) const OVERRIDE;
   virtual FileSystemOperation* CreateFileSystemOperation(
       const FileSystemURL& url,
       FileSystemContext* context,
diff --git a/webkit/fileapi/sandbox_mount_point_provider.cc b/webkit/fileapi/sandbox_mount_point_provider.cc
index 8ffd144..0ed81b4 100644
--- a/webkit/fileapi/sandbox_mount_point_provider.cc
+++ b/webkit/fileapi/sandbox_mount_point_provider.cc
@@ -255,11 +255,13 @@ FileSystemFileUtil* SandboxMountPointProvider::GetFileUtil(
   return sandbox_file_util_.get();
 }
 
-FilePath SandboxMountPointProvider::GetPathForPermissionsCheck(
-    const FilePath& virtual_path) const {
-  // Sandbox provider shouldn't directly grant permissions for its
-  // data directory.
-  return FilePath();
+FilePermissionPolicy SandboxMountPointProvider::GetPermissionPolicy(
+    const FileSystemURL& url, int permissions) const {
+  // Access to the sandbox directory (and only to the directory) should be
+  // always allowed.
+  CHECK(CanHandleType(url.type()));
+  CHECK(!url.path().ReferencesParent());
+  return FILE_PERMISSION_ALWAYS_ALLOW;
 }
 
 FileSystemOperation* SandboxMountPointProvider::CreateFileSystemOperation(
diff --git a/webkit/fileapi/sandbox_mount_point_provider.h b/webkit/fileapi/sandbox_mount_point_provider.h
index 3d5dd11..1302cc4 100644
--- a/webkit/fileapi/sandbox_mount_point_provider.h
+++ b/webkit/fileapi/sandbox_mount_point_provider.h
@@ -83,8 +83,9 @@ class WEBKIT_STORAGE_EXPORT SandboxMountPointProvider
   virtual bool IsAccessAllowed(const FileSystemURL& url) OVERRIDE;
   virtual bool IsRestrictedFileName(const FilePath& filename) const OVERRIDE;
   virtual FileSystemFileUtil* GetFileUtil(FileSystemType type) OVERRIDE;
-  virtual FilePath GetPathForPermissionsCheck(const FilePath& virtual_path)
-      const OVERRIDE;
+  virtual FilePermissionPolicy GetPermissionPolicy(
+      const FileSystemURL& url,
+      int permissions) const OVERRIDE;
   virtual FileSystemOperation* CreateFileSystemOperation(
       const FileSystemURL& url,
       FileSystemContext* context,
diff --git a/webkit/fileapi/test_mount_point_provider.cc b/webkit/fileapi/test_mount_point_provider.cc
index 3cc3804..4ec5ede 100644
--- a/webkit/fileapi/test_mount_point_provider.cc
+++ b/webkit/fileapi/test_mount_point_provider.cc
@@ -114,9 +114,9 @@ FileSystemFileUtil* TestMountPointProvider::GetFileUtil(FileSystemType type) {
   return local_file_util_.get();
 }
 
-FilePath TestMountPointProvider::GetPathForPermissionsCheck(
-    const FilePath& virtual_path) const {
-  return base_path_.Append(virtual_path);
+FilePermissionPolicy TestMountPointProvider::GetPermissionPolicy(
+    const FileSystemURL& url, int permissions) const {
+  return FILE_PERMISSION_ALWAYS_DENY;
 }
 
 FileSystemOperation* TestMountPointProvider::CreateFileSystemOperation(
diff --git a/webkit/fileapi/test_mount_point_provider.h b/webkit/fileapi/test_mount_point_provider.h
index dce922f..d61c1e9 100644
--- a/webkit/fileapi/test_mount_point_provider.h
+++ b/webkit/fileapi/test_mount_point_provider.h
@@ -44,8 +44,9 @@ class WEBKIT_STORAGE_EXPORT_PRIVATE TestMountPointProvider
   virtual bool IsAccessAllowed(const FileSystemURL& url) OVERRIDE;
   virtual bool IsRestrictedFileName(const FilePath& filename) const OVERRIDE;
   virtual FileSystemFileUtil* GetFileUtil(FileSystemType type) OVERRIDE;
-  virtual FilePath GetPathForPermissionsCheck(const FilePath& virtual_path)
-      const OVERRIDE;
+  virtual FilePermissionPolicy GetPermissionPolicy(
+      const FileSystemURL& url,
+      int permissions) const OVERRIDE;
   virtual FileSystemOperation* CreateFileSystemOperation(
       const FileSystemURL& url,
       FileSystemContext* context,
diff --git a/webkit/fileapi/webkit_fileapi.gypi b/webkit/fileapi/webkit_fileapi.gypi
index 7f8737f..69162bf 100644
--- a/webkit/fileapi/webkit_fileapi.gypi
+++ b/webkit/fileapi/webkit_fileapi.gypi
@@ -6,6 +6,8 @@
   'variables': {
     'webkit_fileapi_sources': [
       '../fileapi/file_observers.h',
+      '../fileapi/file_permission_policy.cc',
+      '../fileapi/file_permission_policy.h',
       '../fileapi/file_stream_writer.h',
       '../fileapi/file_system_callback_dispatcher.cc',
       '../fileapi/file_system_callback_dispatcher.h',