From 41e889432dacf1044c38d23a49b7edbc44d273b6 Mon Sep 17 00:00:00 2001 From: "ericu@google.com" Date: Wed, 13 Apr 2011 19:53:30 +0000 Subject: More filesystem cleanup: convert URL-encoded-as-FilePath to actual URL, where possible without WebKit API changes. The WebKit changes will happen in another CL. This is a resubmit of http://codereview.chromium.org/6767010/, which bounced recently. There are a few changes here that weren't there [most notably in file_system_operation_write_unittest.cc and file_system_operation.cc], but they're pretty trivial build/test fixes. Review URL: http://codereview.chromium.org/6833007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@81463 0039d316-1c4b-4281-b951-d872f2087c98 --- webkit/fileapi/file_system_callback_dispatcher.h | 6 +- webkit/fileapi/file_system_file_util_proxy.cc | 2 +- webkit/fileapi/file_system_operation.cc | 87 ++++++++++----- webkit/fileapi/file_system_operation.h | 32 +++--- webkit/fileapi/file_system_operation_unittest.cc | 121 +++++++++++---------- .../file_system_operation_write_unittest.cc | 24 ++-- webkit/fileapi/file_system_path_manager.cc | 100 ++++------------- webkit/fileapi/file_system_path_manager.h | 13 +-- .../fileapi/file_system_path_manager_unittest.cc | 88 --------------- webkit/fileapi/file_system_util.cc | 12 +- webkit/fileapi/file_system_util_unittest.cc | 12 +- webkit/fileapi/webfilewriter_base.cc | 6 +- webkit/fileapi/webfilewriter_base.h | 14 +-- webkit/fileapi/webfilewriter_base_unittest.cc | 62 +++++------ webkit/plugins/ppapi/file_callbacks.cc | 7 +- webkit/plugins/ppapi/file_callbacks.h | 2 +- webkit/plugins/ppapi/mock_plugin_delegate.cc | 14 +-- webkit/plugins/ppapi/mock_plugin_delegate.h | 14 +-- webkit/plugins/ppapi/plugin_delegate.h | 14 +-- webkit/plugins/ppapi/ppb_directory_reader_impl.cc | 2 +- webkit/plugins/ppapi/ppb_file_ref_impl.cc | 43 ++++---- webkit/plugins/ppapi/ppb_file_ref_impl.h | 6 +- webkit/plugins/ppapi/ppb_file_system_impl.h | 10 +- webkit/tools/test_shell/simple_file_system.cc | 50 +++------ webkit/tools/test_shell/simple_file_writer.cc | 12 +- webkit/tools/test_shell/simple_file_writer.h | 6 +- 26 files changed, 325 insertions(+), 434 deletions(-) (limited to 'webkit') diff --git a/webkit/fileapi/file_system_callback_dispatcher.h b/webkit/fileapi/file_system_callback_dispatcher.h index 2bffe54..8504af9 100644 --- a/webkit/fileapi/file_system_callback_dispatcher.h +++ b/webkit/fileapi/file_system_callback_dispatcher.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -9,6 +9,8 @@ #include "base/file_util_proxy.h" +class GURL; + namespace fileapi { // This class mirrors the callbacks in @@ -41,7 +43,7 @@ class FileSystemCallbackDispatcher { // Callback for opening a file system. Called with a name and root path for // the FileSystem when the request is accepted. Used by WebFileSystem API. virtual void DidOpenFileSystem(const std::string& name, - const FilePath& root_path) = 0; + const GURL& root) = 0; // Called with an error code when a requested operation has failed. virtual void DidFail(base::PlatformFileError error_code) = 0; diff --git a/webkit/fileapi/file_system_file_util_proxy.cc b/webkit/fileapi/file_system_file_util_proxy.cc index 0f88ff0..06a61c4 100644 --- a/webkit/fileapi/file_system_file_util_proxy.cc +++ b/webkit/fileapi/file_system_file_util_proxy.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. diff --git a/webkit/fileapi/file_system_operation.cc b/webkit/fileapi/file_system_operation.cc index 4ddfd0c..0e258c1 100644 --- a/webkit/fileapi/file_system_operation.cc +++ b/webkit/fileapi/file_system_operation.cc @@ -5,6 +5,8 @@ #include "webkit/fileapi/file_system_operation.h" #include "base/time.h" +#include "base/utf_string_conversions.h" +#include "net/base/escape.h" #include "net/url_request/url_request_context.h" #include "webkit/fileapi/file_system_callback_dispatcher.h" #include "webkit/fileapi/file_system_context.h" @@ -63,7 +65,7 @@ void FileSystemOperation::OpenFileSystem( callback_factory_.NewCallback(&FileSystemOperation::DidGetRootPath)); } -void FileSystemOperation::CreateFile(const FilePath& path, +void FileSystemOperation::CreateFile(const GURL& path, bool exclusive) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); @@ -86,7 +88,7 @@ void FileSystemOperation::CreateFile(const FilePath& path, : &FileSystemOperation::DidEnsureFileExistsNonExclusive)); } -void FileSystemOperation::CreateDirectory(const FilePath& path, +void FileSystemOperation::CreateDirectory(const GURL& path, bool exclusive, bool recursive) { #ifndef NDEBUG @@ -110,8 +112,8 @@ void FileSystemOperation::CreateDirectory(const FilePath& path, &FileSystemOperation::DidFinishFileOperation)); } -void FileSystemOperation::Copy(const FilePath& src_path, - const FilePath& dest_path) { +void FileSystemOperation::Copy(const GURL& src_path, + const GURL& dest_path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationCopy; @@ -149,8 +151,8 @@ void FileSystemOperation::Copy(const FilePath& src_path, &FileSystemOperation::DidFinishFileOperation)); } -void FileSystemOperation::Move(const FilePath& src_path, - const FilePath& dest_path) { +void FileSystemOperation::Move(const GURL& src_path, + const GURL& dest_path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationMove; @@ -162,6 +164,8 @@ void FileSystemOperation::Move(const FilePath& src_path, FileSystemType src_type; FileSystemType dest_type; + //TODO(ericu): Move alters the source path as well, so we should be checking + //both for write! if (!VerifyFileSystemPathForRead(src_path, &src_origin_url, &src_type, &virtual_path_0) || !VerifyFileSystemPathForWrite(dest_path, true /* create */, @@ -186,7 +190,7 @@ void FileSystemOperation::Move(const FilePath& src_path, &FileSystemOperation::DidFinishFileOperation)); } -void FileSystemOperation::DirectoryExists(const FilePath& path) { +void FileSystemOperation::DirectoryExists(const GURL& path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationDirectoryExists; @@ -207,7 +211,7 @@ void FileSystemOperation::DirectoryExists(const FilePath& path) { &FileSystemOperation::DidDirectoryExists)); } -void FileSystemOperation::FileExists(const FilePath& path) { +void FileSystemOperation::FileExists(const GURL& path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationFileExists; @@ -228,7 +232,7 @@ void FileSystemOperation::FileExists(const FilePath& path) { &FileSystemOperation::DidFileExists)); } -void FileSystemOperation::GetMetadata(const FilePath& path) { +void FileSystemOperation::GetMetadata(const GURL& path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationGetMetadata; @@ -249,7 +253,7 @@ void FileSystemOperation::GetMetadata(const FilePath& path) { &FileSystemOperation::DidGetMetadata)); } -void FileSystemOperation::ReadDirectory(const FilePath& path) { +void FileSystemOperation::ReadDirectory(const GURL& path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationReadDirectory; @@ -270,7 +274,7 @@ void FileSystemOperation::ReadDirectory(const FilePath& path) { &FileSystemOperation::DidReadDirectory)); } -void FileSystemOperation::Remove(const FilePath& path, bool recursive) { +void FileSystemOperation::Remove(const GURL& path, bool recursive) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationRemove; @@ -294,7 +298,7 @@ void FileSystemOperation::Remove(const FilePath& path, bool recursive) { void FileSystemOperation::Write( scoped_refptr url_request_context, - const FilePath& path, + const GURL& path, const GURL& blob_url, int64 offset) { #ifndef NDEBUG @@ -326,7 +330,7 @@ void FileSystemOperation::Write( &FileSystemOperation::OnFileOpenedForWrite)); } -void FileSystemOperation::Truncate(const FilePath& path, int64 length) { +void FileSystemOperation::Truncate(const GURL& path, int64 length) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationTruncate; @@ -347,7 +351,7 @@ void FileSystemOperation::Truncate(const FilePath& path, int64 length) { &FileSystemOperation::DidFinishFileOperation)); } -void FileSystemOperation::TouchFile(const FilePath& path, +void FileSystemOperation::TouchFile(const GURL& path, const base::Time& last_access_time, const base::Time& last_modified_time) { #ifndef NDEBUG @@ -408,14 +412,13 @@ void FileSystemOperation::DidGetRootPath( bool success, const FilePath& path, const std::string& name) { DCHECK(success || path.empty()); - FilePath result; + GURL result; // We ignore the path, and return a URL instead. The point was just to verify // that we could create/find the path. if (success) { - GURL root_url = GetFileSystemRootURI( + result = GetFileSystemRootURI( file_system_operation_context_.src_origin_url(), file_system_operation_context_.src_type()); - result = FilePath().AppendASCII(root_url.spec()); } dispatcher_->DidOpenFileSystem(name, result); delete this; @@ -537,21 +540,37 @@ void FileSystemOperation::OnFileOpenedForWrite( } bool FileSystemOperation::VerifyFileSystemPathForRead( - const FilePath& path, GURL* origin_url, FileSystemType* type, + const GURL& path, GURL* origin_url, FileSystemType* type, FilePath* virtual_path) { // If we have no context, we just allow any operations, for testing. // TODO(ericu): Revisit this hack for security. if (!file_system_context()) { - *virtual_path = path; +#ifdef OS_WIN + // On Windows, the path will look like /C:/foo/bar; we need to remove the + // leading slash to make it valid. But if it's empty, we shouldn't do + // anything. + std::string temp = UnescapeURLComponent(path.path(), + UnescapeRule::SPACES | UnescapeRule::URL_SPECIAL_CHARS); + if (temp.size()) + temp = temp.substr(1); + *virtual_path = FilePath(UTF8ToWide(temp)).NormalizeWindowsPathSeparators(); +#else + *virtual_path = FilePath(path.path()); +#endif *type = file_system_operation_context_.src_type(); + *origin_url = file_system_operation_context_.src_origin_url(); return true; } // We may want do more checks, but for now it just checks if the given - // |path| is under the valid FileSystem root path for this host context. - if (!file_system_context()->path_manager()->CrackFileSystemPath( - path, origin_url, type, virtual_path)) { + // URL is valid. + if (!CrackFileSystemURL(path, origin_url, type, virtual_path)) { + dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY); + return false; + } + if (!file_system_context()->path_manager()->IsAllowedFileSystemType( + *origin_url, *type)) { dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY); return false; } @@ -560,19 +579,35 @@ bool FileSystemOperation::VerifyFileSystemPathForRead( } bool FileSystemOperation::VerifyFileSystemPathForWrite( - const FilePath& path, bool create, GURL* origin_url, FileSystemType* type, + const GURL& path, bool create, GURL* origin_url, FileSystemType* type, FilePath* virtual_path) { // If we have no context, we just allow any operations, for testing. // TODO(ericu): Revisit this hack for security. if (!file_system_context()) { - *virtual_path = path; +#ifdef OS_WIN + // On Windows, the path will look like /C:/foo/bar; we need to remove the + // leading slash to make it valid. But if it's empty, we shouldn't do + // anything. + std::string temp = UnescapeURLComponent(path.path(), + UnescapeRule::SPACES | UnescapeRule::URL_SPECIAL_CHARS); + if (temp.size()) + temp = temp.substr(1); + *virtual_path = FilePath(UTF8ToWide(temp)).NormalizeWindowsPathSeparators(); +#else + *virtual_path = FilePath(path.path()); +#endif *type = file_system_operation_context_.dest_type(); + *origin_url = file_system_operation_context_.dest_origin_url(); return true; } - if (!file_system_context()->path_manager()->CrackFileSystemPath( - path, origin_url, type, virtual_path)) { + if (!CrackFileSystemURL(path, origin_url, type, virtual_path)) { + dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY); + return false; + } + if (!file_system_context()->path_manager()->IsAllowedFileSystemType( + *origin_url, *type)) { dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY); return false; } diff --git a/webkit/fileapi/file_system_operation.h b/webkit/fileapi/file_system_operation.h index 5ba585e..8ca3be5 100644 --- a/webkit/fileapi/file_system_operation.h +++ b/webkit/fileapi/file_system_operation.h @@ -57,26 +57,26 @@ class FileSystemOperation { void OpenFileSystem(const GURL& origin_url, fileapi::FileSystemType type, bool create); - void CreateFile(const FilePath& path, + void CreateFile(const GURL& path, bool exclusive); - void CreateDirectory(const FilePath& path, + void CreateDirectory(const GURL& path, bool exclusive, bool recursive); - void Copy(const FilePath& src_path, - const FilePath& dest_path); - void Move(const FilePath& src_path, - const FilePath& dest_path); - void DirectoryExists(const FilePath& path); - void FileExists(const FilePath& path); - void GetMetadata(const FilePath& path); - void ReadDirectory(const FilePath& path); - void Remove(const FilePath& path, bool recursive); + void Copy(const GURL& src_path, + const GURL& dest_path); + void Move(const GURL& src_path, + const GURL& dest_path); + void DirectoryExists(const GURL& path); + void FileExists(const GURL& path); + void GetMetadata(const GURL& path); + void ReadDirectory(const GURL& path); + void Remove(const GURL& path, bool recursive); void Write(scoped_refptr url_request_context, - const FilePath& path, + const GURL& path, const GURL& blob_url, int64 offset); - void Truncate(const FilePath& path, int64 length); - void TouchFile(const FilePath& path, + void Truncate(const GURL& path, int64 length); + void TouchFile(const GURL& path, const base::Time& last_access_time, const base::Time& last_modified_time); @@ -143,7 +143,7 @@ class FileSystemOperation { // PLATFORM_FILE_ERROR_SECURITY and returns false. // (Note: this doesn't delete this when it calls DidFail and returns false; // it's the caller's responsibility.) - bool VerifyFileSystemPathForRead(const FilePath& path, + bool VerifyFileSystemPathForRead(const GURL& path, GURL* root_url, FileSystemType* type, FilePath* virtual_path); @@ -162,7 +162,7 @@ class FileSystemOperation { // DidFail with PLATFORM_FILE_ERROR_SECURITY and returns false. // (Note: this doesn't delete this when it calls DidFail and returns false; // it's the caller's responsibility.) - bool VerifyFileSystemPathForWrite(const FilePath& path, + bool VerifyFileSystemPathForWrite(const GURL& path, bool create, GURL* root_url, FileSystemType* type, diff --git a/webkit/fileapi/file_system_operation_unittest.cc b/webkit/fileapi/file_system_operation_unittest.cc index 75c9d31..292104b 100644 --- a/webkit/fileapi/file_system_operation_unittest.cc +++ b/webkit/fileapi/file_system_operation_unittest.cc @@ -56,6 +56,16 @@ class FileSystemOperationTest : public testing::Test { // Common temp base for nondestructive uses. ScopedTempDir base_; + GURL URLForRelativePath(const std::string& path) const { + // Only the path will actually get used. + return GURL("file://").Resolve(base_.path().value()).Resolve(path); + } + + GURL URLForPath(const FilePath& path) const { + // Only the path will actually get used. + return GURL("file://").Resolve(path.value()); + } + // For post-operation status. int status_; base::PlatformFileInfo info_; @@ -91,7 +101,7 @@ class MockDispatcher : public FileSystemCallbackDispatcher { test_->set_entries(entries); } - virtual void DidOpenFileSystem(const std::string&, const FilePath&) { + virtual void DidOpenFileSystem(const std::string&, const GURL&) { NOTREACHED(); } @@ -113,12 +123,15 @@ FileSystemOperation* FileSystemOperationTest::operation() { kFileSystemTypeTemporary); operation->file_system_operation_context()->set_dest_type( kFileSystemTypeTemporary); + GURL origin_url("fake://fake.foo/"); + operation->file_system_operation_context()->set_src_origin_url(origin_url); + operation->file_system_operation_context()->set_dest_origin_url(origin_url); return operation; } TEST_F(FileSystemOperationTest, TestMoveFailureSrcDoesntExist) { - FilePath src(base_.path().Append(FILE_PATH_LITERAL("a"))); - FilePath dest(base_.path().Append(FILE_PATH_LITERAL("b"))); + GURL src(URLForRelativePath("a")); + GURL dest(URLForRelativePath("b")); operation()->Move(src, dest); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); @@ -131,7 +144,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureContainsPath) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir(src_dir.path(), FILE_PATH_LITERAL("child_dir"), &dest_dir_path)); - operation()->Move(src_dir.path(), dest_dir_path); + operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_dir_path)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, status()); } @@ -146,7 +159,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcDirExistsDestFile) { FilePath dest_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file); - operation()->Move(src_dir.path(), dest_file); + operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, status()); } @@ -161,7 +174,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestNonEmptyDir) { FilePath child_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &child_file); - operation()->Move(src_dir.path(), dest_dir.path()); + operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, status()); } @@ -176,7 +189,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestDir) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Move(src_file, dest_dir.path()); + operation()->Move(URLForPath(src_file), URLForPath(dest_dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, status()); } @@ -189,7 +202,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureDestParentDoesntExist) { FILE_PATH_LITERAL("NonexistingDir")).Append( FILE_PATH_LITERAL("NonexistingFile")); - operation()->Move(src_dir.path(), nonexisting_file); + operation()->Move(URLForPath(src_dir.path()), URLForPath(nonexisting_file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } @@ -205,7 +218,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndOverwrite) { FilePath dest_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file); - operation()->Move(src_file, dest_file); + operation()->Move(URLForPath(src_file), URLForPath(dest_file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); @@ -221,7 +234,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndNew) { ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); FilePath dest_file(dest_dir.path().Append(FILE_PATH_LITERAL("NewFile"))); - operation()->Move(src_file, dest_file); + operation()->Move(URLForPath(src_file), URLForPath(dest_file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); @@ -234,7 +247,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndOverwrite) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Move(src_dir.path(), dest_dir.path()); + operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(src_dir.path())); @@ -253,7 +266,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndNew) { ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath dest_dir_path(dir.path().Append(FILE_PATH_LITERAL("NewDirectory"))); - operation()->Move(src_dir.path(), dest_dir_path); + operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_dir_path)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(src_dir.path())); @@ -272,7 +285,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirRecursive) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Move(src_dir.path(), dest_dir.path()); + operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(dest_dir.path().Append( @@ -283,9 +296,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirRecursive) { } TEST_F(FileSystemOperationTest, TestCopyFailureSrcDoesntExist) { - FilePath src(base_.path().Append(FILE_PATH_LITERAL("a"))); - FilePath dest(base_.path().Append(FILE_PATH_LITERAL("b"))); - operation()->Copy(src, dest); + operation()->Copy(URLForRelativePath("a"), URLForRelativePath("b")); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } @@ -297,7 +308,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureContainsPath) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir(src_dir.path(), FILE_PATH_LITERAL("child_dir"), &dest_dir_path)); - operation()->Copy(src_dir.path(), dest_dir_path); + operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_dir_path)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, status()); } @@ -312,7 +323,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcDirExistsDestFile) { FilePath dest_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file); - operation()->Copy(src_dir.path(), dest_file); + operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, status()); } @@ -327,7 +338,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestNonEmptyDir) { FilePath child_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &child_file); - operation()->Copy(src_dir.path(), dest_dir.path()); + operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, status()); } @@ -342,7 +353,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestDir) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Copy(src_file, dest_dir.path()); + operation()->Copy(URLForPath(src_file), URLForPath(dest_dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, status()); } @@ -358,7 +369,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureDestParentDoesntExist) { FilePath nonexisting_file = nonexisting.Append( FILE_PATH_LITERAL("DontExistFile")); - operation()->Copy(src_dir, nonexisting_file); + operation()->Copy(URLForPath(src_dir), URLForPath(nonexisting_file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } @@ -374,7 +385,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndOverwrite) { FilePath dest_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file); - operation()->Copy(src_file, dest_file); + operation()->Copy(URLForPath(src_file), URLForPath(dest_file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); @@ -390,7 +401,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndNew) { ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); FilePath dest_file(dest_dir.path().Append(FILE_PATH_LITERAL("NewFile"))); - operation()->Copy(src_file, dest_file); + operation()->Copy(URLForPath(src_file), URLForPath(dest_file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); @@ -403,7 +414,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndOverwrite) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Copy(src_dir.path(), dest_dir.path()); + operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); @@ -421,7 +432,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndNew) { ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath dest_dir(dir.path().Append(FILE_PATH_LITERAL("NewDirectory"))); - operation()->Copy(src_dir.path(), dest_dir); + operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_dir)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(dest_dir)); @@ -439,7 +450,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirRecursive) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Copy(src_dir.path(), dest_dir.path()); + operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(dest_dir.path().Append( @@ -456,7 +467,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileFailure) { FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); - operation()->CreateFile(file, true); + operation()->CreateFile(URLForPath(file), true); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status()); } @@ -468,7 +479,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileExists) { FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); - operation()->CreateFile(file, false); + operation()->CreateFile(URLForPath(file), false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(file)); @@ -479,7 +490,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessExclusive) { ScopedTempDir dir; ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath file = dir.path().Append(FILE_PATH_LITERAL("FileDoesntExist")); - operation()->CreateFile(file, true); + operation()->CreateFile(URLForPath(file), true); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(file)); @@ -490,7 +501,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileDoesntExist) { ScopedTempDir dir; ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath file = dir.path().Append(FILE_PATH_LITERAL("FileDoesntExist")); - operation()->CreateFile(file, false); + operation()->CreateFile(URLForPath(file), false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); } @@ -502,7 +513,7 @@ TEST_F(FileSystemOperationTest, FILE_PATH_LITERAL("DirDoesntExist"))); FilePath nonexisting_file = nonexisting.Append( FILE_PATH_LITERAL("FileDoesntExist")); - operation()->CreateDirectory(nonexisting_file, false, false); + operation()->CreateDirectory(URLForPath(nonexisting_file), false, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } @@ -511,7 +522,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirFailureDirExists) { // Exclusive and dir existing at path. ScopedTempDir src_dir; ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - operation()->CreateDirectory(src_dir.path(), true, false); + operation()->CreateDirectory(URLForPath(src_dir.path()), true, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status()); } @@ -522,7 +533,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirFailureFileExists) { ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); - operation()->CreateDirectory(file, true, false); + operation()->CreateDirectory(URLForPath(file), true, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status()); } @@ -531,14 +542,14 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccess) { // Dir exists and exclusive is false. ScopedTempDir dir; ASSERT_TRUE(dir.CreateUniqueTempDir()); - operation()->CreateDirectory(dir.path(), false, false); + operation()->CreateDirectory(URLForPath(dir.path()), false, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); // Dir doesn't exist. FilePath nonexisting_dir_path(base_.path().Append( FILE_PATH_LITERAL("nonexistingdir"))); - operation()->CreateDirectory(nonexisting_dir_path, false, false); + operation()->CreateDirectory(URLForPath(nonexisting_dir_path), false, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(nonexisting_dir_path)); @@ -549,7 +560,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccessExclusive) { FilePath nonexisting_dir_path(base_.path().Append( FILE_PATH_LITERAL("nonexistingdir"))); - operation()->CreateDirectory(nonexisting_dir_path, true, false); + operation()->CreateDirectory(URLForPath(nonexisting_dir_path), true, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(nonexisting_dir_path)); @@ -558,16 +569,16 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccessExclusive) { TEST_F(FileSystemOperationTest, TestExistsAndMetadataFailure) { FilePath nonexisting_dir_path(base_.path().Append( FILE_PATH_LITERAL("nonexistingdir"))); - operation()->GetMetadata(nonexisting_dir_path); + operation()->GetMetadata(URLForPath(nonexisting_dir_path)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); - operation()->FileExists(nonexisting_dir_path); + operation()->FileExists(URLForPath(nonexisting_dir_path)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); file_util::EnsureEndsWithSeparator(&nonexisting_dir_path); - operation()->DirectoryExists(nonexisting_dir_path); + operation()->DirectoryExists(URLForPath(nonexisting_dir_path)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } @@ -576,11 +587,11 @@ TEST_F(FileSystemOperationTest, TestExistsAndMetadataSuccess) { ScopedTempDir dir; ASSERT_TRUE(dir.CreateUniqueTempDir()); - operation()->DirectoryExists(dir.path()); + operation()->DirectoryExists(URLForPath(dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); - operation()->GetMetadata(dir.path()); + operation()->GetMetadata(URLForPath(dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(info().is_directory); @@ -588,11 +599,11 @@ TEST_F(FileSystemOperationTest, TestExistsAndMetadataSuccess) { FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); - operation()->FileExists(file); + operation()->FileExists(URLForPath(file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); - operation()->GetMetadata(file); + operation()->GetMetadata(URLForPath(file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(info().is_directory); @@ -602,13 +613,13 @@ TEST_F(FileSystemOperationTest, TestExistsAndMetadataSuccess) { TEST_F(FileSystemOperationTest, TestTypeMismatchErrors) { ScopedTempDir dir; ASSERT_TRUE(dir.CreateUniqueTempDir()); - operation()->FileExists(dir.path()); + operation()->FileExists(URLForPath(dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, status()); FilePath file; ASSERT_TRUE(file_util::CreateTemporaryFileInDir(dir.path(), &file)); - operation()->DirectoryExists(file); + operation()->DirectoryExists(URLForPath(file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, status()); } @@ -618,7 +629,7 @@ TEST_F(FileSystemOperationTest, TestReadDirFailure) { FilePath nonexisting_dir_path(base_.path().Append( FILE_PATH_LITERAL("NonExistingDir"))); file_util::EnsureEndsWithSeparator(&nonexisting_dir_path); - operation()->ReadDirectory(nonexisting_dir_path); + operation()->ReadDirectory(URLForPath(nonexisting_dir_path)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); @@ -627,7 +638,7 @@ TEST_F(FileSystemOperationTest, TestReadDirFailure) { ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); - operation()->ReadDirectory(file); + operation()->ReadDirectory(URLForPath(file)); MessageLoop::current()->RunAllPending(); // TODO(kkanetkar) crbug.com/54309 to change the error code. EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); @@ -646,7 +657,7 @@ TEST_F(FileSystemOperationTest, TestReadDirSuccess) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir( parent_dir.path(), FILE_PATH_LITERAL("child_dir"), &child_dir)); - operation()->ReadDirectory(parent_dir.path()); + operation()->ReadDirectory(URLForPath(parent_dir.path())); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationStatusNotSet, status()); EXPECT_EQ(2u, entries().size()); @@ -668,7 +679,7 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) { FILE_PATH_LITERAL("NonExistingDir"))); file_util::EnsureEndsWithSeparator(&nonexisting); - operation()->Remove(nonexisting, false /* recursive */); + operation()->Remove(URLForPath(nonexisting), false /* recursive */); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); @@ -686,7 +697,7 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir( parent_dir.path(), FILE_PATH_LITERAL("child_dir"), &child_dir)); - operation()->Remove(parent_dir.path(), false /* recursive */); + operation()->Remove(URLForPath(parent_dir.path()), false /* recursive */); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, status()); @@ -697,7 +708,7 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) { ASSERT_TRUE(empty_dir.CreateUniqueTempDir()); EXPECT_TRUE(file_util::DirectoryExists(empty_dir.path())); - operation()->Remove(empty_dir.path(), false /* recursive */); + operation()->Remove(URLForPath(empty_dir.path()), false /* recursive */); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(empty_dir.path())); @@ -715,7 +726,7 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir( parent_dir.path(), FILE_PATH_LITERAL("child_dir"), &child_dir)); - operation()->Remove(parent_dir.path(), true /* recursive */); + operation()->Remove(URLForPath(parent_dir.path()), true /* recursive */); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(parent_dir.path())); @@ -733,7 +744,7 @@ TEST_F(FileSystemOperationTest, TestTruncate) { file_util::WriteFile(file, test_data, data_size)); // Check that its length is the size of the data written. - operation()->GetMetadata(file); + operation()->GetMetadata(URLForPath(file)); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(info().is_directory); @@ -741,7 +752,7 @@ TEST_F(FileSystemOperationTest, TestTruncate) { // Extend the file by truncating it. int length = 17; - operation()->Truncate(file, length); + operation()->Truncate(URLForPath(file), length); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); @@ -762,7 +773,7 @@ TEST_F(FileSystemOperationTest, TestTruncate) { // Shorten the file by truncating it. length = 3; - operation()->Truncate(file, length); + operation()->Truncate(URLForPath(file), length); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); diff --git a/webkit/fileapi/file_system_operation_write_unittest.cc b/webkit/fileapi/file_system_operation_write_unittest.cc index 57f72fc..3c36168 100644 --- a/webkit/fileapi/file_system_operation_write_unittest.cc +++ b/webkit/fileapi/file_system_operation_write_unittest.cc @@ -10,7 +10,6 @@ #include "webkit/fileapi/file_system_operation.h" -#include "base/logging.h" #include "base/message_loop.h" #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_temp_dir.h" @@ -61,6 +60,16 @@ class FileSystemOperationWriteTest : public testing::Test { virtual void TearDown(); protected: + GURL URLForRelativePath(const std::string& path) const { + // Only the path will actually get used. + return GURL("file://").Resolve(file_.value()).Resolve(path); + } + + GURL URLForPath(const FilePath& path) const { + // Only the path will actually get used. + return GURL("file://").Resolve(path.value()); + } + MessageLoop loop_; ScopedTempDir dir_; @@ -122,7 +131,7 @@ class MockDispatcher : public FileSystemCallbackDispatcher { ADD_FAILURE(); } - virtual void DidOpenFileSystem(const std::string&, const FilePath&) { + virtual void DidOpenFileSystem(const std::string&, const GURL&) { ADD_FAILURE(); } @@ -170,7 +179,7 @@ TEST_F(FileSystemOperationWriteTest, TestWriteSuccess) { url_request_context->blob_storage_controller()-> RegisterBlobUrl(blob_url, blob_data); - operation()->Write(url_request_context, file_, blob_url, 0); + operation()->Write(url_request_context, URLForPath(file_), blob_url, 0); MessageLoop::current()->Run(); url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); @@ -190,7 +199,7 @@ TEST_F(FileSystemOperationWriteTest, TestWriteZero) { url_request_context->blob_storage_controller()-> RegisterBlobUrl(blob_url, blob_data); - operation()->Write(url_request_context, file_, blob_url, 0); + operation()->Write(url_request_context, URLForPath(file_), blob_url, 0); MessageLoop::current()->Run(); url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); @@ -204,7 +213,8 @@ TEST_F(FileSystemOperationWriteTest, TestWriteInvalidBlobUrl) { scoped_refptr url_request_context( new TestURLRequestContext()); - operation()->Write(url_request_context, file_, GURL("blob:invalid"), 0); + operation()->Write(url_request_context, URLForPath(file_), + GURL("blob:invalid"), 0); MessageLoop::current()->Run(); EXPECT_EQ(0, bytes_written()); @@ -223,7 +233,7 @@ TEST_F(FileSystemOperationWriteTest, TestWriteInvalidFile) { RegisterBlobUrl(blob_url, blob_data); operation()->Write(url_request_context, - FilePath(FILE_PATH_LITERAL("/nonexist")), blob_url, 0); + URLForRelativePath("nonexist"), blob_url, 0); MessageLoop::current()->Run(); url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); @@ -243,7 +253,7 @@ TEST_F(FileSystemOperationWriteTest, TestWriteDir) { url_request_context->blob_storage_controller()-> RegisterBlobUrl(blob_url, blob_data); - operation()->Write(url_request_context, dir_.path(), blob_url, 0); + operation()->Write(url_request_context, URLForPath(dir_.path()), blob_url, 0); MessageLoop::current()->Run(); url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); diff --git a/webkit/fileapi/file_system_path_manager.cc b/webkit/fileapi/file_system_path_manager.cc index 558dc31..16fc60b 100644 --- a/webkit/fileapi/file_system_path_manager.cc +++ b/webkit/fileapi/file_system_path_manager.cc @@ -100,83 +100,6 @@ FilePath FileSystemPathManager::GetFileSystemRootPathOnFileThread( } } -bool FileSystemPathManager::CrackFileSystemPath( - const FilePath& path, GURL* origin_url, FileSystemType* type, - FilePath* virtual_path) const { - // TODO(ericu): - // Paths come in here [for now] as a URL, followed by a virtual path in - // platform format. For example, on Windows, this will look like - // filesystem:http://www.example.com/temporary/\path\to\file.txt. - // A potentially dangerous malicious path on Windows might look like: - // filesystem:http://www.example.com/temporary/foo/../../\path\to\file.txt. - // This code is ugly, but will get cleaned up as we fix the calling side. - // Eventually there won't be a distinction between a filesystem path and a - // filesystem URL--they'll all be URLs. - // We should be passing these to WebKit as string, not FilePath, for ease of - // manipulation, or possibly as GURL/KURL. - - std::string path_as_string; -#ifdef OS_WIN - path_as_string = WideToUTF8(path.value()); -#else - path_as_string = path.value(); -#endif - GURL path_as_url(path_as_string); - - FilePath local_path; - GURL local_url; - FileSystemType local_type; - if (!CrackFileSystemURL(path_as_url, &local_url, &local_type, &local_path)) - return false; - -#if defined(FILE_PATH_USES_WIN_SEPARATORS) - // TODO(ericu): This puts the separators back to windows-standard; they come - // out of the above code as '/' no matter the platform. Long-term, we'll - // want to let the underlying FileSystemFileUtil implementation do this part, - // since they won't all need it. - local_path = local_path.NormalizeWindowsPathSeparators(); -#endif - - // Check if file access to this type of file system is allowed - // for this origin. - switch (local_type) { - case kFileSystemTypeTemporary: - case kFileSystemTypePersistent: - if (!sandbox_provider_->IsAccessAllowed(local_url)) - return false; - break; - case kFileSystemTypeLocal: - if (!local_provider_.get() || - !local_provider_->IsAccessAllowed(local_url)) { - return false; - } - break; - case kFileSystemTypeUnknown: - default: - NOTREACHED(); - return false; - } - // Any paths that include parent references are considered invalid. - // These should have been taken care of in CrackFileSystemURL. - DCHECK(!local_path.ReferencesParent()); - - // The given |local_path| seems valid. Populates the |origin_url|, |type| - // and |virtual_path| if they are given. - - if (origin_url) { - *origin_url = local_url; - } - - if (type) - *type = local_type; - - if (virtual_path) { - *virtual_path = local_path; - } - - return true; -} - bool FileSystemPathManager::IsAllowedScheme(const GURL& url) const { // Basically we only accept http or https. We allow file:// URLs // only if --allow-file-access-from-files flag is given. @@ -212,6 +135,29 @@ bool FileSystemPathManager::IsRestrictedFileName( } } +// Checks if an origin has access to a particular filesystem type. +bool FileSystemPathManager::IsAllowedFileSystemType( + GURL origin, FileSystemType type) { + switch (type) { + case kFileSystemTypeTemporary: + case kFileSystemTypePersistent: + if (!sandbox_provider_->IsAccessAllowed(origin)) + return false; + break; + case kFileSystemTypeLocal: + if (!local_provider_.get() || + !local_provider_->IsAccessAllowed(origin)) { + return false; + } + break; + case kFileSystemTypeUnknown: + default: + NOTREACHED(); + return false; + } + return true; +} + } // namespace fileapi COMPILE_ASSERT(int(WebFileSystem::TypeTemporary) == \ diff --git a/webkit/fileapi/file_system_path_manager.h b/webkit/fileapi/file_system_path_manager.h index 5a9613a..19718ae 100644 --- a/webkit/fileapi/file_system_path_manager.h +++ b/webkit/fileapi/file_system_path_manager.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -68,14 +68,6 @@ class FileSystemPathManager { FileSystemType type, const FilePath& virtual_path, bool create); - // Cracks the given |path|, retrieves the information embedded in the path - // and populates |origin_url|, |type| and |virtual_path|. The |virtual_path| - // is a sandboxed path in the file system, i.e. the relative path to the - // filesystem root for the given domain and type. - bool CrackFileSystemPath(const FilePath& path, - GURL* origin_url, - FileSystemType* type, - FilePath* virtual_path) const; // Returns true if the given |url|'s scheme is allowed to access // filesystem. @@ -89,6 +81,9 @@ class FileSystemPathManager { bool IsRestrictedFileName(FileSystemType type, const FilePath& filename); + // Checks if an origin has access to a particular filesystem type. + bool IsAllowedFileSystemType(GURL origin, FileSystemType type); + SandboxMountPointProvider* sandbox_provider() const { return sandbox_provider_.get(); } diff --git a/webkit/fileapi/file_system_path_manager_unittest.cc b/webkit/fileapi/file_system_path_manager_unittest.cc index fee38c1..8a99b84 100644 --- a/webkit/fileapi/file_system_path_manager_unittest.cc +++ b/webkit/fileapi/file_system_path_manager_unittest.cc @@ -228,11 +228,6 @@ class FileSystemPathManagerTest : public testing::Test { return root_path_callback_status_; } - bool CheckValidFileSystemPath(FileSystemPathManager* manager, - const FilePath& path) { - return manager->CrackFileSystemPath(path, NULL, NULL, NULL); - } - FilePath data_path() { return data_dir_.path(); } FilePath file_system_path() { return data_dir_.path().Append( @@ -367,89 +362,6 @@ TEST_F(FileSystemPathManagerTest, GetRootPathFileURIWithAllowFlag) { } } -TEST_F(FileSystemPathManagerTest, VirtualPathFromFileSystemPathTest) { - scoped_ptr manager(NewPathManager(false, false)); - GURL root_url = GetFileSystemRootURI( - GURL("http://foo.com/"), fileapi::kFileSystemTypeTemporary); - FilePath root_path = FilePath().AppendASCII(root_url.spec()); - - for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kPathToVirtualPathTestCases); ++i) { - SCOPED_TRACE(testing::Message() << "PathToVirtualPath #" - << i << " " << kPathToVirtualPathTestCases[i]); - FilePath absolute_path; - // TODO(ericu): Clean this up when we've got more sane path-handling. - // This hack is necessary because root_path is actually a URL [ending with a - // forward slash], and AppendASCII("") on Windows will delete the trailing - // slash, making the path invalid as far as CrackFileSystemPath is - // concerned. - if (strlen(kPathToVirtualPathTestCases[i])) - absolute_path = root_path.AppendASCII( - kPathToVirtualPathTestCases[i]); - else - absolute_path = root_path; - FilePath virtual_path; - EXPECT_TRUE(manager->CrackFileSystemPath(absolute_path, NULL, NULL, - &virtual_path)); - - FilePath test_case_path; - test_case_path = test_case_path.AppendASCII( - kPathToVirtualPathTestCases[i]); - EXPECT_EQ(test_case_path.value(), virtual_path.value()); - } -} - -TEST_F(FileSystemPathManagerTest, TypeFromFileSystemPathTest) { - scoped_ptr manager(NewPathManager(false, false)); - - fileapi::FileSystemType type; - - GURL root_url = GetFileSystemRootURI( - GURL("http://foo.com/"), fileapi::kFileSystemTypeTemporary); - FilePath root_path = FilePath().AppendASCII(root_url.spec()); - FilePath path = root_path.AppendASCII("test"); - EXPECT_TRUE(manager->CrackFileSystemPath(path, NULL, &type, NULL)); - EXPECT_EQ(fileapi::kFileSystemTypeTemporary, type); - - root_url = GetFileSystemRootURI( - GURL("http://foo.com/"), fileapi::kFileSystemTypePersistent); - root_path = FilePath().AppendASCII(root_url.spec()); - path = root_path.AppendASCII("test"); - EXPECT_TRUE(manager->CrackFileSystemPath(path, NULL, &type, NULL)); - EXPECT_EQ(fileapi::kFileSystemTypePersistent, type); -} - -TEST_F(FileSystemPathManagerTest, CheckValidPath) { - scoped_ptr manager(NewPathManager(false, false)); - GURL root_url = GetFileSystemRootURI( - GURL("http://foo.com/"), fileapi::kFileSystemTypePersistent); - FilePath root_path = FilePath().AppendASCII(root_url.spec()); - - // The root path must be valid, but upper directories or directories - // that are not in our temporary or persistent directory must be - // evaluated invalid. - EXPECT_TRUE(CheckValidFileSystemPath(manager.get(), root_path)); - EXPECT_FALSE(CheckValidFileSystemPath(manager.get(), root_path.DirName())); - EXPECT_FALSE(CheckValidFileSystemPath(manager.get(), - root_path.DirName().DirName())); - EXPECT_FALSE(CheckValidFileSystemPath(manager.get(), - root_path.DirName().DirName() - .AppendASCII("ArbitraryName") - .AppendASCII("chrome-dummy"))); - - for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kCheckValidPathTestCases); ++i) { - SCOPED_TRACE(testing::Message() << "CheckValidPath #" << i << " " - << kCheckValidPathTestCases[i].path); - FilePath path(kCheckValidPathTestCases[i].path); -#ifdef FILE_PATH_USES_WIN_SEPARATORS - path = path.NormalizeWindowsPathSeparators(); -#endif - if (!path.IsAbsolute()) - path = root_path.Append(path); - EXPECT_EQ(kCheckValidPathTestCases[i].expected_valid, - CheckValidFileSystemPath(manager.get(), path)); - } -} - TEST_F(FileSystemPathManagerTest, IsRestrictedName) { scoped_ptr manager(NewPathManager(false, false)); for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kIsRestrictedNameTestCases); ++i) { diff --git a/webkit/fileapi/file_system_util.cc b/webkit/fileapi/file_system_util.cc index a15c2a8..75013da 100644 --- a/webkit/fileapi/file_system_util.cc +++ b/webkit/fileapi/file_system_util.cc @@ -88,17 +88,17 @@ bool CrackFileSystemURL(const GURL& url, GURL* origin_url, FileSystemType* type, while (!path.empty() && path[0] == '/') path.erase(0, 1); -#if defined(OS_WIN) - const FilePath::StringType sys_path = base::SysUTF8ToWide(path); -#elif defined(OS_POSIX) - const FilePath::StringType sys_path = path; -#endif if (origin_url) *origin_url = origin; if (type) *type = file_system_type; if (file_path) - *file_path = FilePath(sys_path); +#if defined(OS_WIN) + *file_path = FilePath(base::SysUTF8ToWide(path)). + NormalizeWindowsPathSeparators(); +#elif defined(OS_POSIX) + *file_path = FilePath(path); +#endif return true; } diff --git a/webkit/fileapi/file_system_util_unittest.cc b/webkit/fileapi/file_system_util_unittest.cc index e08fec7..41863ae 100644 --- a/webkit/fileapi/file_system_util_unittest.cc +++ b/webkit/fileapi/file_system_util_unittest.cc @@ -29,7 +29,8 @@ TEST_F(FileSystemUtilTest, ParsePersistent) { "filesystem:http://chromium.org/persistent/directory/file")); EXPECT_EQ("http://chromium.org/", origin_url_.spec()); EXPECT_EQ(kFileSystemTypePersistent, type_); - EXPECT_EQ(FILE_PATH_LITERAL("directory/file"), file_path_.value()); + EXPECT_EQ(FILE_PATH_LITERAL("file"), file_path_.BaseName().value()); + EXPECT_EQ(FILE_PATH_LITERAL("directory"), file_path_.DirName().value()); } TEST_F(FileSystemUtilTest, ParseTemporary) { @@ -37,7 +38,8 @@ TEST_F(FileSystemUtilTest, ParseTemporary) { "filesystem:http://chromium.org/temporary/directory/file")); EXPECT_EQ("http://chromium.org/", origin_url_.spec()); EXPECT_EQ(kFileSystemTypeTemporary, type_); - EXPECT_EQ(FILE_PATH_LITERAL("directory/file"), file_path_.value()); + EXPECT_EQ(FILE_PATH_LITERAL("file"), file_path_.BaseName().value()); + EXPECT_EQ(FILE_PATH_LITERAL("directory"), file_path_.DirName().value()); } TEST_F(FileSystemUtilTest, EnsureFilePathIsRelative) { @@ -45,7 +47,8 @@ TEST_F(FileSystemUtilTest, EnsureFilePathIsRelative) { "filesystem:http://chromium.org/temporary/////directory/file")); EXPECT_EQ("http://chromium.org/", origin_url_.spec()); EXPECT_EQ(kFileSystemTypeTemporary, type_); - EXPECT_EQ(FILE_PATH_LITERAL("directory/file"), file_path_.value()); + EXPECT_EQ(FILE_PATH_LITERAL("file"), file_path_.BaseName().value()); + EXPECT_EQ(FILE_PATH_LITERAL("directory"), file_path_.DirName().value()); EXPECT_FALSE(file_path_.IsAbsolute()); } @@ -59,7 +62,8 @@ TEST_F(FileSystemUtilTest, RejectBadSchemes) { TEST_F(FileSystemUtilTest, UnescapePath) { ASSERT_TRUE(CrackFileSystemURL( "filesystem:http://chromium.org/persistent/%7Echromium/space%20bar")); - EXPECT_EQ(FILE_PATH_LITERAL("~chromium/space bar"), file_path_.value()); + EXPECT_EQ(FILE_PATH_LITERAL("space bar"), file_path_.BaseName().value()); + EXPECT_EQ(FILE_PATH_LITERAL("~chromium"), file_path_.DirName().value()); } TEST_F(FileSystemUtilTest, RejectBadType) { diff --git a/webkit/fileapi/webfilewriter_base.cc b/webkit/fileapi/webfilewriter_base.cc index cf2a7bc..1e0e8a3 100644 --- a/webkit/fileapi/webfilewriter_base.cc +++ b/webkit/fileapi/webfilewriter_base.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -13,8 +13,8 @@ namespace fileapi { WebFileWriterBase::WebFileWriterBase( - const WebKit::WebString& path, WebKit::WebFileWriterClient* client) - : path_(webkit_glue::WebStringToFilePath(path)), + const GURL& path, WebKit::WebFileWriterClient* client) + : path_(path), client_(client), operation_(kOperationNone), cancel_state_(kCancelNotInProgress) { diff --git a/webkit/fileapi/webfilewriter_base.h b/webkit/fileapi/webfilewriter_base.h index f47b383..3601256 100644 --- a/webkit/fileapi/webfilewriter_base.h +++ b/webkit/fileapi/webfilewriter_base.h @@ -1,16 +1,14 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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_WEBFILEWRITER_BASE_H_ #define WEBKIT_FILEAPI_WEBFILEWRITER_BASE_H_ -#include "base/file_path.h" #include "base/platform_file.h" +#include "googleurl/src/gurl.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebFileWriter.h" -class GURL; - namespace WebKit { class WebFileWriterClient; class WebString; @@ -22,7 +20,7 @@ namespace fileapi { class WebFileWriterBase : public WebKit::WebFileWriter { public: WebFileWriterBase( - const WebKit::WebString& path, WebKit::WebFileWriterClient* client); + const GURL& path, WebKit::WebFileWriterClient* client); virtual ~WebFileWriterBase(); // WebFileWriter implementation @@ -34,8 +32,8 @@ class WebFileWriterBase : public WebKit::WebFileWriter { // Derived classes must provide these methods to asynchronously perform // the requested operation, and they must call the appropiate DidSomething // method upon completion and as progress is made in the Write case. - virtual void DoTruncate(const FilePath& path, int64 offset) = 0; - virtual void DoWrite(const FilePath& path, const GURL& blob_url, + virtual void DoTruncate(const GURL& path, int64 offset) = 0; + virtual void DoWrite(const GURL& path, const GURL& blob_url, int64 offset) = 0; virtual void DoCancel() = 0; @@ -58,7 +56,7 @@ class WebFileWriterBase : public WebKit::WebFileWriter { void FinishCancel(); - FilePath path_; + GURL path_; WebKit::WebFileWriterClient* client_; OperationType operation_; CancelState cancel_state_; diff --git a/webkit/fileapi/webfilewriter_base_unittest.cc b/webkit/fileapi/webfilewriter_base_unittest.cc index 7a07cd2..43164e7 100644 --- a/webkit/fileapi/webfilewriter_base_unittest.cc +++ b/webkit/fileapi/webfilewriter_base_unittest.cc @@ -31,16 +31,8 @@ const int kMultiFileWrite_Offset = 3; const int kCancelFileWriteBeforeCompletion_Offset = 4; const int kCancelFileWriteAfterCompletion_Offset = 5; -std::string mock_path_as_ascii() { - return std::string("MockPath"); -} - -string16 mock_path_as_string16() { - return ASCIIToUTF16(mock_path_as_ascii()); -} - -FilePath mock_path_as_file_path() { - return FilePath().AppendASCII(mock_path_as_ascii()); +GURL mock_path_as_gurl() { + return GURL("MockPath"); } } // namespace @@ -48,32 +40,32 @@ FilePath mock_path_as_file_path() { class TestableFileWriter : public WebFileWriterBase { public: explicit TestableFileWriter(WebKit::WebFileWriterClient* client) - : WebFileWriterBase(mock_path_as_string16(), client) { + : WebFileWriterBase(mock_path_as_gurl(), client) { reset(); } void reset() { received_truncate_ = false; - received_truncate_path_ = FilePath(); + received_truncate_path_ = GURL(); received_truncate_offset_ = kNoOffset; received_write_ = false; - received_write_path_ = FilePath(); + received_write_path_ = GURL(); received_write_offset_ = kNoOffset; received_write_blob_url_ = GURL(); received_cancel_ = false; } bool received_truncate_; - FilePath received_truncate_path_; + GURL received_truncate_path_; int64 received_truncate_offset_; bool received_write_; - FilePath received_write_path_; + GURL received_write_path_; GURL received_write_blob_url_; int64 received_write_offset_; bool received_cancel_; protected: - virtual void DoTruncate(const FilePath& path, int64 offset) { + virtual void DoTruncate(const GURL& path, int64 offset) { received_truncate_ = true; received_truncate_path_ = path; received_truncate_offset_ = offset; @@ -95,7 +87,7 @@ class TestableFileWriter : public WebFileWriterBase { } } - virtual void DoWrite(const FilePath& path, const GURL& blob_url, + virtual void DoWrite(const GURL& path, const GURL& blob_url, int64 offset) { received_write_ = true; received_write_path_ = path; @@ -205,8 +197,8 @@ TEST_F(FileWriterTest, BasicFileWrite) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_write_); - EXPECT_EQ(testable_writer_->received_write_path_.value(), - mock_path_as_file_path().value()); + EXPECT_EQ(testable_writer_->received_write_path_, + mock_path_as_gurl()); EXPECT_EQ(kBasicFileWrite_Offset, testable_writer_->received_write_offset_); EXPECT_EQ(kBlobUrl, testable_writer_->received_write_blob_url_); @@ -227,8 +219,8 @@ TEST_F(FileWriterTest, BasicFileTruncate) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_truncate_); - EXPECT_EQ(mock_path_as_file_path().value(), - testable_writer_->received_truncate_path_.value()); + EXPECT_EQ(mock_path_as_gurl(), + testable_writer_->received_truncate_path_); EXPECT_EQ(kBasicFileTruncate_Offset, testable_writer_->received_truncate_offset_); EXPECT_FALSE(testable_writer_->received_write_); @@ -247,8 +239,8 @@ TEST_F(FileWriterTest, ErrorFileWrite) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_write_); - EXPECT_EQ(testable_writer_->received_write_path_.value(), - mock_path_as_file_path().value()); + EXPECT_EQ(testable_writer_->received_write_path_, + mock_path_as_gurl()); EXPECT_EQ(kErrorFileWrite_Offset, testable_writer_->received_write_offset_); EXPECT_EQ(kBlobUrl, testable_writer_->received_write_blob_url_); @@ -268,8 +260,8 @@ TEST_F(FileWriterTest, ErrorFileTruncate) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_truncate_); - EXPECT_EQ(mock_path_as_file_path().value(), - testable_writer_->received_truncate_path_.value()); + EXPECT_EQ(mock_path_as_gurl(), + testable_writer_->received_truncate_path_); EXPECT_EQ(kErrorFileTruncate_Offset, testable_writer_->received_truncate_offset_); EXPECT_FALSE(testable_writer_->received_write_); @@ -289,8 +281,8 @@ TEST_F(FileWriterTest, MultiFileWrite) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_write_); - EXPECT_EQ(testable_writer_->received_write_path_.value(), - mock_path_as_file_path().value()); + EXPECT_EQ(testable_writer_->received_write_path_, + mock_path_as_gurl()); EXPECT_EQ(kMultiFileWrite_Offset, testable_writer_->received_write_offset_); EXPECT_EQ(kBlobUrl, testable_writer_->received_write_blob_url_); @@ -312,8 +304,8 @@ TEST_F(FileWriterTest, CancelFileWriteBeforeCompletion) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_write_); - EXPECT_EQ(testable_writer_->received_write_path_.value(), - mock_path_as_file_path().value()); + EXPECT_EQ(testable_writer_->received_write_path_, + mock_path_as_gurl()); EXPECT_EQ(kCancelFileWriteBeforeCompletion_Offset, testable_writer_->received_write_offset_); EXPECT_EQ(kBlobUrl, testable_writer_->received_write_blob_url_); @@ -336,8 +328,8 @@ TEST_F(FileWriterTest, CancelFileWriteAfterCompletion) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_write_); - EXPECT_EQ(testable_writer_->received_write_path_.value(), - mock_path_as_file_path().value()); + EXPECT_EQ(testable_writer_->received_write_path_, + mock_path_as_gurl()); EXPECT_EQ(kCancelFileWriteAfterCompletion_Offset, testable_writer_->received_write_offset_); EXPECT_EQ(kBlobUrl, testable_writer_->received_write_blob_url_); @@ -359,8 +351,8 @@ TEST_F(FileWriterTest, CancelFileTruncate) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_truncate_); - EXPECT_EQ(mock_path_as_file_path().value(), - testable_writer_->received_truncate_path_.value()); + EXPECT_EQ(mock_path_as_gurl(), + testable_writer_->received_truncate_path_); EXPECT_EQ(kCancelFileTruncate_Offset, testable_writer_->received_truncate_offset_); EXPECT_TRUE(testable_writer_->received_cancel_); @@ -379,8 +371,8 @@ TEST_F(FileWriterTest, CancelFailedTruncate) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_truncate_); - EXPECT_EQ(mock_path_as_file_path().value(), - testable_writer_->received_truncate_path_.value()); + EXPECT_EQ(mock_path_as_gurl(), + testable_writer_->received_truncate_path_); EXPECT_EQ(kCancelFailedTruncate_Offset, testable_writer_->received_truncate_offset_); EXPECT_TRUE(testable_writer_->received_cancel_); diff --git a/webkit/plugins/ppapi/file_callbacks.cc b/webkit/plugins/ppapi/file_callbacks.cc index 92d443b..6706d47 100644 --- a/webkit/plugins/ppapi/file_callbacks.cc +++ b/webkit/plugins/ppapi/file_callbacks.cc @@ -1,10 +1,9 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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/plugins/ppapi/file_callbacks.h" -#include "base/file_path.h" #include "base/logging.h" #include "ppapi/c/dev/ppb_file_system_dev.h" #include "ppapi/c/dev/pp_file_info_dev.h" @@ -76,12 +75,12 @@ void FileCallbacks::DidReadDirectory( } void FileCallbacks::DidOpenFileSystem(const std::string&, - const FilePath& root_path) { + const GURL& root_url) { if (callback_->completed()) return; DCHECK(file_system_); - file_system_->set_root_path(root_path); + file_system_->set_root_url(root_url); file_system_->set_opened(true); callback_->Run(PP_OK); diff --git a/webkit/plugins/ppapi/file_callbacks.h b/webkit/plugins/ppapi/file_callbacks.h index e0a129f..4bdaa1d 100644 --- a/webkit/plugins/ppapi/file_callbacks.h +++ b/webkit/plugins/ppapi/file_callbacks.h @@ -45,7 +45,7 @@ class FileCallbacks : public fileapi::FileSystemCallbackDispatcher { virtual void DidReadDirectory( const std::vector& entries, bool has_more); virtual void DidOpenFileSystem(const std::string&, - const FilePath& root_path); + const GURL& root_url); virtual void DidFail(base::PlatformFileError error_code); virtual void DidWrite(int64 bytes, bool complete); diff --git a/webkit/plugins/ppapi/mock_plugin_delegate.cc b/webkit/plugins/ppapi/mock_plugin_delegate.cc index f81e9c2..a9da765 100644 --- a/webkit/plugins/ppapi/mock_plugin_delegate.cc +++ b/webkit/plugins/ppapi/mock_plugin_delegate.cc @@ -87,20 +87,20 @@ bool MockPluginDelegate::OpenFileSystem( } bool MockPluginDelegate::MakeDirectory( - const FilePath& path, + const GURL& path, bool recursive, fileapi::FileSystemCallbackDispatcher* dispatcher) { return false; } bool MockPluginDelegate::Query( - const FilePath& path, + const GURL& path, fileapi::FileSystemCallbackDispatcher* dispatcher) { return false; } bool MockPluginDelegate::Touch( - const FilePath& path, + const GURL& path, const base::Time& last_access_time, const base::Time& last_modified_time, fileapi::FileSystemCallbackDispatcher* dispatcher) { @@ -108,20 +108,20 @@ bool MockPluginDelegate::Touch( } bool MockPluginDelegate::Delete( - const FilePath& path, + const GURL& path, fileapi::FileSystemCallbackDispatcher* dispatcher) { return false; } bool MockPluginDelegate::Rename( - const FilePath& file_path, - const FilePath& new_file_path, + const GURL& file_path, + const GURL& new_file_path, fileapi::FileSystemCallbackDispatcher* dispatcher) { return false; } bool MockPluginDelegate::ReadDirectory( - const FilePath& directory_path, + const GURL& directory_path, fileapi::FileSystemCallbackDispatcher* dispatcher) { return false; } diff --git a/webkit/plugins/ppapi/mock_plugin_delegate.h b/webkit/plugins/ppapi/mock_plugin_delegate.h index dd5e8f1..78d59a2 100644 --- a/webkit/plugins/ppapi/mock_plugin_delegate.h +++ b/webkit/plugins/ppapi/mock_plugin_delegate.h @@ -44,22 +44,22 @@ class MockPluginDelegate : public PluginDelegate { long long size, fileapi::FileSystemCallbackDispatcher* dispatcher); virtual bool MakeDirectory( - const FilePath& path, + const GURL& path, bool recursive, fileapi::FileSystemCallbackDispatcher* dispatcher); - virtual bool Query(const FilePath& path, + virtual bool Query(const GURL& path, fileapi::FileSystemCallbackDispatcher* dispatcher); - virtual bool Touch(const FilePath& path, + virtual bool Touch(const GURL& path, const base::Time& last_access_time, const base::Time& last_modified_time, fileapi::FileSystemCallbackDispatcher* dispatcher); - virtual bool Delete(const FilePath& path, + virtual bool Delete(const GURL& path, fileapi::FileSystemCallbackDispatcher* dispatcher); - virtual bool Rename(const FilePath& file_path, - const FilePath& new_file_path, + virtual bool Rename(const GURL& file_path, + const GURL& new_file_path, fileapi::FileSystemCallbackDispatcher* dispatcher); virtual bool ReadDirectory( - const FilePath& directory_path, + const GURL& directory_path, fileapi::FileSystemCallbackDispatcher* dispatcher); virtual base::PlatformFileError OpenFile(const PepperFilePath& path, int flags, diff --git a/webkit/plugins/ppapi/plugin_delegate.h b/webkit/plugins/ppapi/plugin_delegate.h index 33fb820..786e924 100644 --- a/webkit/plugins/ppapi/plugin_delegate.h +++ b/webkit/plugins/ppapi/plugin_delegate.h @@ -288,22 +288,22 @@ class PluginDelegate { long long size, fileapi::FileSystemCallbackDispatcher* dispatcher) = 0; virtual bool MakeDirectory( - const FilePath& path, + const GURL& path, bool recursive, fileapi::FileSystemCallbackDispatcher* dispatcher) = 0; - virtual bool Query(const FilePath& path, + virtual bool Query(const GURL& path, fileapi::FileSystemCallbackDispatcher* dispatcher) = 0; - virtual bool Touch(const FilePath& path, + virtual bool Touch(const GURL& path, const base::Time& last_access_time, const base::Time& last_modified_time, fileapi::FileSystemCallbackDispatcher* dispatcher) = 0; - virtual bool Delete(const FilePath& path, + virtual bool Delete(const GURL& path, fileapi::FileSystemCallbackDispatcher* dispatcher) = 0; - virtual bool Rename(const FilePath& file_path, - const FilePath& new_file_path, + virtual bool Rename(const GURL& file_path, + const GURL& new_file_path, fileapi::FileSystemCallbackDispatcher* dispatcher) = 0; virtual bool ReadDirectory( - const FilePath& directory_path, + const GURL& directory_path, fileapi::FileSystemCallbackDispatcher* dispatcher) = 0; virtual base::PlatformFileError OpenFile(const PepperFilePath& path, diff --git a/webkit/plugins/ppapi/ppb_directory_reader_impl.cc b/webkit/plugins/ppapi/ppb_directory_reader_impl.cc index e704b3f..77935cf 100644 --- a/webkit/plugins/ppapi/ppb_directory_reader_impl.cc +++ b/webkit/plugins/ppapi/ppb_directory_reader_impl.cc @@ -113,7 +113,7 @@ int32_t PPB_DirectoryReader_Impl::GetNextEntry( PP_Resource resource_id = GetReferenceNoAddRef(); DCHECK(resource_id != 0); if (!instance->delegate()->ReadDirectory( - directory_ref_->GetSystemPath(), + directory_ref_->GetFileSystemURL(), new FileCallbacks(instance->module()->AsWeakPtr(), resource_id, callback, NULL, NULL, this))) diff --git a/webkit/plugins/ppapi/ppb_file_ref_impl.cc b/webkit/plugins/ppapi/ppb_file_ref_impl.cc index c2ec5b2..b048caf 100644 --- a/webkit/plugins/ppapi/ppb_file_ref_impl.cc +++ b/webkit/plugins/ppapi/ppb_file_ref_impl.cc @@ -6,6 +6,7 @@ #include "base/string_util.h" #include "base/utf_string_conversions.h" +#include "googleurl/src/gurl.h" #include "ppapi/c/pp_errors.h" #include "webkit/plugins/ppapi/common.h" #include "webkit/plugins/ppapi/file_callbacks.h" @@ -126,7 +127,7 @@ int32_t MakeDirectory(PP_Resource directory_ref_id, PluginInstance* instance = file_system->instance(); if (!instance->delegate()->MakeDirectory( - directory_ref->GetSystemPath(), PPBoolToBool(make_ancestors), + directory_ref->GetFileSystemURL(), PPBoolToBool(make_ancestors), new FileCallbacks(instance->module()->AsWeakPtr(), directory_ref_id, callback, NULL, NULL, NULL))) return PP_ERROR_FAILED; @@ -150,7 +151,8 @@ int32_t Touch(PP_Resource file_ref_id, PluginInstance* instance = file_system->instance(); if (!instance->delegate()->Touch( - file_ref->GetSystemPath(), base::Time::FromDoubleT(last_access_time), + file_ref->GetFileSystemURL(), + base::Time::FromDoubleT(last_access_time), base::Time::FromDoubleT(last_modified_time), new FileCallbacks(instance->module()->AsWeakPtr(), file_ref_id, callback, NULL, NULL, NULL))) @@ -173,7 +175,7 @@ int32_t Delete(PP_Resource file_ref_id, PluginInstance* instance = file_system->instance(); if (!instance->delegate()->Delete( - file_ref->GetSystemPath(), + file_ref->GetFileSystemURL(), new FileCallbacks(instance->module()->AsWeakPtr(), file_ref_id, callback, NULL, NULL, NULL))) return PP_ERROR_FAILED; @@ -204,7 +206,7 @@ int32_t Rename(PP_Resource file_ref_id, // http://crbug.com/67624 PluginInstance* instance = file_system->instance(); if (!instance->delegate()->Rename( - file_ref->GetSystemPath(), new_file_ref->GetSystemPath(), + file_ref->GetFileSystemURL(), new_file_ref->GetFileSystemURL(), new FileCallbacks(instance->module()->AsWeakPtr(), file_ref_id, callback, NULL, NULL, NULL))) return PP_ERROR_FAILED; @@ -320,22 +322,25 @@ std::string PPB_FileRef_Impl::GetPath() const { } FilePath PPB_FileRef_Impl::GetSystemPath() const { - if (GetFileSystemType() == PP_FILESYSTEMTYPE_EXTERNAL) - return system_path_; + if (GetFileSystemType() != PP_FILESYSTEMTYPE_EXTERNAL) { + NOTREACHED(); + return FilePath(); + } + return system_path_; +} - // Since |virtual_path_| starts with a '/', it is considered an absolute path - // on POSIX systems. We need to remove the '/' before calling Append() or we - // will run into a DCHECK. - FilePath virtual_file_path( -#if defined(OS_WIN) - UTF8ToWide(virtual_path_.substr(1)) -#elif defined(OS_POSIX) - virtual_path_.substr(1) -#else -#error "Unsupported platform." -#endif - ); - return file_system_->root_path().Append(virtual_file_path); +GURL PPB_FileRef_Impl::GetFileSystemURL() const { + if (GetFileSystemType() != PP_FILESYSTEMTYPE_LOCALPERSISTENT && + GetFileSystemType() != PP_FILESYSTEMTYPE_LOCALTEMPORARY) { + NOTREACHED(); + return GURL(); + } + if (!virtual_path_.size()) + return file_system_->root_url(); + // Since |virtual_path_| starts with a '/', it looks like an absolute path. + // We need to trim off the '/' before calling Resolve, as FileSystem URLs + // start with a storage type identifier that looks like a path segment. + return file_system_->root_url().Resolve(virtual_path_.substr(1)); } } // namespace ppapi diff --git a/webkit/plugins/ppapi/ppb_file_ref_impl.h b/webkit/plugins/ppapi/ppb_file_ref_impl.h index f006991..234e150 100644 --- a/webkit/plugins/ppapi/ppb_file_ref_impl.h +++ b/webkit/plugins/ppapi/ppb_file_ref_impl.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -8,6 +8,7 @@ #include #include "base/file_path.h" +#include "googleurl/src/gurl.h" #include "ppapi/c/dev/ppb_file_ref_dev.h" #include "webkit/plugins/ppapi/resource.h" @@ -52,6 +53,9 @@ class PPB_FileRef_Impl : public Resource { // Returns the system path corresponding to this file. FilePath GetSystemPath() const; + // Returns the FileSystem API URL corresponding to this file. + GURL GetFileSystemURL() const; + private: scoped_refptr file_system_; std::string virtual_path_; // UTF-8 encoded diff --git a/webkit/plugins/ppapi/ppb_file_system_impl.h b/webkit/plugins/ppapi/ppb_file_system_impl.h index 3ee3756..b381f56 100644 --- a/webkit/plugins/ppapi/ppb_file_system_impl.h +++ b/webkit/plugins/ppapi/ppb_file_system_impl.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -6,7 +6,7 @@ #define WEBKIT_PLUGINS_PPAPI_PPB_FILE_SYSTEM_IMPL_H_ #include "base/basictypes.h" -#include "base/file_path.h" +#include "googleurl/src/gurl.h" #include "ppapi/c/dev/pp_file_info_dev.h" #include "webkit/plugins/ppapi/resource.h" @@ -28,8 +28,8 @@ class PPB_FileSystem_Impl : public Resource { PluginInstance* instance() { return instance_; } PP_FileSystemType_Dev type() { return type_; } - const FilePath& root_path() const { return root_path_; } - void set_root_path(const FilePath& root_path) { root_path_ = root_path; } + const GURL& root_url() const { return root_url_; } + void set_root_url(const GURL& root_url) { root_url_ = root_url; } bool opened() const { return opened_; } void set_opened(bool opened) { opened_ = opened; } bool called_open() const { return called_open_; } @@ -38,7 +38,7 @@ class PPB_FileSystem_Impl : public Resource { private: PluginInstance* instance_; PP_FileSystemType_Dev type_; - FilePath root_path_; + GURL root_url_; bool opened_; bool called_open_; diff --git a/webkit/tools/test_shell/simple_file_system.cc b/webkit/tools/test_shell/simple_file_system.cc index ecbc6d5..ff634eeb 100644 --- a/webkit/tools/test_shell/simple_file_system.cc +++ b/webkit/tools/test_shell/simple_file_system.cc @@ -95,13 +95,13 @@ class SimpleFileSystemCallbackDispatcher } virtual void DidOpenFileSystem( - const std::string& name, const FilePath& path) { + const std::string& name, const GURL& root) { DCHECK(file_system_); - if (path.empty()) + if (!root.is_valid()) callbacks_->didFail(WebKit::WebFileErrorSecurity); else callbacks_->didOpenFileSystem( - UTF8ToUTF16(name), webkit_glue::FilePathToWebString(path)); + WebString::fromUTF8(name), WebString::fromUTF8(root.spec())); } virtual void DidFail(base::PlatformFileError error_code) { @@ -168,80 +168,58 @@ void SimpleFileSystem::OpenFileSystem( void SimpleFileSystem::move( const WebString& src_path, const WebString& dest_path, WebFileSystemCallbacks* callbacks) { - FilePath dest_filepath(webkit_glue::WebStringToFilePath(dest_path)); - FilePath src_filepath(webkit_glue::WebStringToFilePath(src_path)); - - GetNewOperation(callbacks)->Move(src_filepath, dest_filepath); + GetNewOperation(callbacks)->Move(GURL(src_path), GURL(dest_path)); } void SimpleFileSystem::copy( const WebString& src_path, const WebString& dest_path, WebFileSystemCallbacks* callbacks) { - FilePath dest_filepath(webkit_glue::WebStringToFilePath(dest_path)); - FilePath src_filepath(webkit_glue::WebStringToFilePath(src_path)); - - GetNewOperation(callbacks)->Copy(src_filepath, dest_filepath); + GetNewOperation(callbacks)->Copy(GURL(src_path), GURL(dest_path)); } void SimpleFileSystem::remove( const WebString& path, WebFileSystemCallbacks* callbacks) { - FilePath filepath(webkit_glue::WebStringToFilePath(path)); - - GetNewOperation(callbacks)->Remove(filepath, false /* recursive */); + GetNewOperation(callbacks)->Remove(GURL(path), false /* recursive */); } void SimpleFileSystem::removeRecursively( const WebString& path, WebFileSystemCallbacks* callbacks) { - FilePath filepath(webkit_glue::WebStringToFilePath(path)); - - GetNewOperation(callbacks)->Remove(filepath, true /* recursive */); + GetNewOperation(callbacks)->Remove(GURL(path), true /* recursive */); } void SimpleFileSystem::readMetadata( const WebString& path, WebFileSystemCallbacks* callbacks) { - FilePath filepath(webkit_glue::WebStringToFilePath(path)); - - GetNewOperation(callbacks)->GetMetadata(filepath); + GetNewOperation(callbacks)->GetMetadata(GURL(path)); } void SimpleFileSystem::createFile( const WebString& path, bool exclusive, WebFileSystemCallbacks* callbacks) { - FilePath filepath(webkit_glue::WebStringToFilePath(path)); - - GetNewOperation(callbacks)->CreateFile(filepath, exclusive); + GetNewOperation(callbacks)->CreateFile(GURL(path), exclusive); } void SimpleFileSystem::createDirectory( const WebString& path, bool exclusive, WebFileSystemCallbacks* callbacks) { - FilePath filepath(webkit_glue::WebStringToFilePath(path)); - - GetNewOperation(callbacks)->CreateDirectory(filepath, exclusive, false); + GetNewOperation(callbacks)->CreateDirectory(GURL(path), exclusive, false); } void SimpleFileSystem::fileExists( const WebString& path, WebFileSystemCallbacks* callbacks) { - FilePath filepath(webkit_glue::WebStringToFilePath(path)); - - GetNewOperation(callbacks)->FileExists(filepath); + GetNewOperation(callbacks)->FileExists(GURL(path)); } void SimpleFileSystem::directoryExists( const WebString& path, WebFileSystemCallbacks* callbacks) { - FilePath filepath(webkit_glue::WebStringToFilePath(path)); - - GetNewOperation(callbacks)->DirectoryExists(filepath); + GetNewOperation(callbacks)->DirectoryExists(GURL(path)); } void SimpleFileSystem::readDirectory( const WebString& path, WebFileSystemCallbacks* callbacks) { - FilePath filepath(webkit_glue::WebStringToFilePath(path)); - - GetNewOperation(callbacks)->ReadDirectory(filepath); + GetNewOperation(callbacks)->ReadDirectory(GURL(path)); } WebFileWriter* SimpleFileSystem::createFileWriter( const WebString& path, WebFileWriterClient* client) { - return new SimpleFileWriter(path, client, file_system_context_.get()); + return new SimpleFileWriter(GURL(path), client, file_system_context_.get()); } FileSystemOperation* SimpleFileSystem::GetNewOperation( diff --git a/webkit/tools/test_shell/simple_file_writer.cc b/webkit/tools/test_shell/simple_file_writer.cc index 9304978..657d247 100644 --- a/webkit/tools/test_shell/simple_file_writer.cc +++ b/webkit/tools/test_shell/simple_file_writer.cc @@ -45,7 +45,7 @@ class SimpleFileWriter::IOThreadProxy virtual ~IOThreadProxy() { } - void Truncate(const FilePath& path, int64 offset) { + void Truncate(const GURL& path, int64 offset) { if (!io_thread_->BelongsToCurrentThread()) { io_thread_->PostTask(FROM_HERE, NewRunnableMethod( this, &IOThreadProxy::Truncate, path, offset)); @@ -56,7 +56,7 @@ class SimpleFileWriter::IOThreadProxy operation_->Truncate(path, offset); } - void Write(const FilePath& path, const GURL& blob_url, int64 offset) { + void Write(const GURL& path, const GURL& blob_url, int64 offset) { if (!io_thread_->BelongsToCurrentThread()) { io_thread_->PostTask(FROM_HERE, NewRunnableMethod( this, &IOThreadProxy::Write, path, blob_url, offset)); @@ -118,7 +118,7 @@ class SimpleFileWriter::IOThreadProxy virtual void DidOpenFileSystem( const std::string& name, - const FilePath& root_path) { + const GURL& root) { NOTREACHED(); } @@ -181,7 +181,7 @@ class SimpleFileWriter::IOThreadProxy SimpleFileWriter::SimpleFileWriter( - const WebString& path, + const GURL& path, WebFileWriterClient* client, FileSystemContext* file_system_context) : WebFileWriterBase(path, client), @@ -191,12 +191,12 @@ SimpleFileWriter::SimpleFileWriter( SimpleFileWriter::~SimpleFileWriter() { } -void SimpleFileWriter::DoTruncate(const FilePath& path, int64 offset) { +void SimpleFileWriter::DoTruncate(const GURL& path, int64 offset) { io_thread_proxy_->Truncate(path, offset); } void SimpleFileWriter::DoWrite( - const FilePath& path, const GURL& blob_url, int64 offset) { + const GURL& path, const GURL& blob_url, int64 offset) { io_thread_proxy_->Write(path, blob_url, offset); } diff --git a/webkit/tools/test_shell/simple_file_writer.h b/webkit/tools/test_shell/simple_file_writer.h index 6ef27b9..9ea2da5 100644 --- a/webkit/tools/test_shell/simple_file_writer.h +++ b/webkit/tools/test_shell/simple_file_writer.h @@ -22,7 +22,7 @@ class SimpleFileWriter : public fileapi::WebFileWriterBase, public base::SupportsWeakPtr { public: SimpleFileWriter( - const WebKit::WebString& path, + const GURL& path, WebKit::WebFileWriterClient* client, fileapi::FileSystemContext* file_system_context); virtual ~SimpleFileWriter(); @@ -38,8 +38,8 @@ class SimpleFileWriter : public fileapi::WebFileWriterBase, protected: // WebFileWriterBase overrides - virtual void DoTruncate(const FilePath& path, int64 offset); - virtual void DoWrite(const FilePath& path, const GURL& blob_url, + virtual void DoTruncate(const GURL& path, int64 offset); + virtual void DoWrite(const GURL& path, const GURL& blob_url, int64 offset); virtual void DoCancel(); -- cgit v1.1