diff options
author | ericu@google.com <ericu@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-04-12 23:03:29 +0000 |
---|---|---|
committer | ericu@google.com <ericu@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-04-12 23:03:29 +0000 |
commit | 4520c1ef61642fc8130f1cd9de26db892c3da684 (patch) | |
tree | 11140eca636e129f8b21f56fd9504ff33f2ec458 /webkit/fileapi | |
parent | 3e0c664682b788f3093e0528a772b51c72e95a1f (diff) | |
download | chromium_src-4520c1ef61642fc8130f1cd9de26db892c3da684.zip chromium_src-4520c1ef61642fc8130f1cd9de26db892c3da684.tar.gz chromium_src-4520c1ef61642fc8130f1cd9de26db892c3da684.tar.bz2 |
Revert 81319 - 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/ [third try lucky,fourth try persistent?], which bounced most recently due to bugs manifestingonly on XP. There are a few changes here that weren't there [infile_system_operation_write_unittest.cc and file_system_operation.cc], butthey're pretty trivial build/test fixes.BUG=none TEST=noneReview URL: http://codereview.chromium.org/6821065
TBR=ericu@google.com
Review URL: http://codereview.chromium.org/6836002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@81320 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'webkit/fileapi')
-rw-r--r-- | webkit/fileapi/file_system_callback_dispatcher.h | 6 | ||||
-rw-r--r-- | webkit/fileapi/file_system_file_util_proxy.cc | 2 | ||||
-rw-r--r-- | webkit/fileapi/file_system_operation.cc | 87 | ||||
-rw-r--r-- | webkit/fileapi/file_system_operation.h | 32 | ||||
-rw-r--r-- | webkit/fileapi/file_system_operation_unittest.cc | 121 | ||||
-rw-r--r-- | webkit/fileapi/file_system_operation_write_unittest.cc | 24 | ||||
-rw-r--r-- | webkit/fileapi/file_system_path_manager.cc | 100 | ||||
-rw-r--r-- | webkit/fileapi/file_system_path_manager.h | 13 | ||||
-rw-r--r-- | webkit/fileapi/file_system_path_manager_unittest.cc | 88 | ||||
-rw-r--r-- | webkit/fileapi/file_system_util.cc | 12 | ||||
-rw-r--r-- | webkit/fileapi/file_system_util_unittest.cc | 12 | ||||
-rw-r--r-- | webkit/fileapi/webfilewriter_base.cc | 6 | ||||
-rw-r--r-- | webkit/fileapi/webfilewriter_base.h | 14 | ||||
-rw-r--r-- | webkit/fileapi/webfilewriter_base_unittest.cc | 62 |
14 files changed, 337 insertions, 242 deletions
diff --git a/webkit/fileapi/file_system_callback_dispatcher.h b/webkit/fileapi/file_system_callback_dispatcher.h index 8504af9..2bffe54 100644 --- a/webkit/fileapi/file_system_callback_dispatcher.h +++ b/webkit/fileapi/file_system_callback_dispatcher.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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,8 +9,6 @@ #include "base/file_util_proxy.h" -class GURL; - namespace fileapi { // This class mirrors the callbacks in @@ -43,7 +41,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 GURL& root) = 0; + const FilePath& root_path) = 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 06a61c4..0f88ff0 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) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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 0e258c1..4ddfd0c 100644 --- a/webkit/fileapi/file_system_operation.cc +++ b/webkit/fileapi/file_system_operation.cc @@ -5,8 +5,6 @@ #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" @@ -65,7 +63,7 @@ void FileSystemOperation::OpenFileSystem( callback_factory_.NewCallback(&FileSystemOperation::DidGetRootPath)); } -void FileSystemOperation::CreateFile(const GURL& path, +void FileSystemOperation::CreateFile(const FilePath& path, bool exclusive) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); @@ -88,7 +86,7 @@ void FileSystemOperation::CreateFile(const GURL& path, : &FileSystemOperation::DidEnsureFileExistsNonExclusive)); } -void FileSystemOperation::CreateDirectory(const GURL& path, +void FileSystemOperation::CreateDirectory(const FilePath& path, bool exclusive, bool recursive) { #ifndef NDEBUG @@ -112,8 +110,8 @@ void FileSystemOperation::CreateDirectory(const GURL& path, &FileSystemOperation::DidFinishFileOperation)); } -void FileSystemOperation::Copy(const GURL& src_path, - const GURL& dest_path) { +void FileSystemOperation::Copy(const FilePath& src_path, + const FilePath& dest_path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationCopy; @@ -151,8 +149,8 @@ void FileSystemOperation::Copy(const GURL& src_path, &FileSystemOperation::DidFinishFileOperation)); } -void FileSystemOperation::Move(const GURL& src_path, - const GURL& dest_path) { +void FileSystemOperation::Move(const FilePath& src_path, + const FilePath& dest_path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationMove; @@ -164,8 +162,6 @@ void FileSystemOperation::Move(const GURL& 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 */, @@ -190,7 +186,7 @@ void FileSystemOperation::Move(const GURL& src_path, &FileSystemOperation::DidFinishFileOperation)); } -void FileSystemOperation::DirectoryExists(const GURL& path) { +void FileSystemOperation::DirectoryExists(const FilePath& path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationDirectoryExists; @@ -211,7 +207,7 @@ void FileSystemOperation::DirectoryExists(const GURL& path) { &FileSystemOperation::DidDirectoryExists)); } -void FileSystemOperation::FileExists(const GURL& path) { +void FileSystemOperation::FileExists(const FilePath& path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationFileExists; @@ -232,7 +228,7 @@ void FileSystemOperation::FileExists(const GURL& path) { &FileSystemOperation::DidFileExists)); } -void FileSystemOperation::GetMetadata(const GURL& path) { +void FileSystemOperation::GetMetadata(const FilePath& path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationGetMetadata; @@ -253,7 +249,7 @@ void FileSystemOperation::GetMetadata(const GURL& path) { &FileSystemOperation::DidGetMetadata)); } -void FileSystemOperation::ReadDirectory(const GURL& path) { +void FileSystemOperation::ReadDirectory(const FilePath& path) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationReadDirectory; @@ -274,7 +270,7 @@ void FileSystemOperation::ReadDirectory(const GURL& path) { &FileSystemOperation::DidReadDirectory)); } -void FileSystemOperation::Remove(const GURL& path, bool recursive) { +void FileSystemOperation::Remove(const FilePath& path, bool recursive) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationRemove; @@ -298,7 +294,7 @@ void FileSystemOperation::Remove(const GURL& path, bool recursive) { void FileSystemOperation::Write( scoped_refptr<net::URLRequestContext> url_request_context, - const GURL& path, + const FilePath& path, const GURL& blob_url, int64 offset) { #ifndef NDEBUG @@ -330,7 +326,7 @@ void FileSystemOperation::Write( &FileSystemOperation::OnFileOpenedForWrite)); } -void FileSystemOperation::Truncate(const GURL& path, int64 length) { +void FileSystemOperation::Truncate(const FilePath& path, int64 length) { #ifndef NDEBUG DCHECK(kOperationNone == pending_operation_); pending_operation_ = kOperationTruncate; @@ -351,7 +347,7 @@ void FileSystemOperation::Truncate(const GURL& path, int64 length) { &FileSystemOperation::DidFinishFileOperation)); } -void FileSystemOperation::TouchFile(const GURL& path, +void FileSystemOperation::TouchFile(const FilePath& path, const base::Time& last_access_time, const base::Time& last_modified_time) { #ifndef NDEBUG @@ -412,13 +408,14 @@ void FileSystemOperation::DidGetRootPath( bool success, const FilePath& path, const std::string& name) { DCHECK(success || path.empty()); - GURL result; + FilePath 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) { - result = GetFileSystemRootURI( + GURL root_url = 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; @@ -540,37 +537,21 @@ void FileSystemOperation::OnFileOpenedForWrite( } bool FileSystemOperation::VerifyFileSystemPathForRead( - const GURL& path, GURL* origin_url, FileSystemType* type, + const FilePath& 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()) { -#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 + *virtual_path = path; *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 - // 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)) { + // |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)) { dispatcher_->DidFail(base::PLATFORM_FILE_ERROR_SECURITY); return false; } @@ -579,35 +560,19 @@ bool FileSystemOperation::VerifyFileSystemPathForRead( } bool FileSystemOperation::VerifyFileSystemPathForWrite( - const GURL& path, bool create, GURL* origin_url, FileSystemType* type, + const FilePath& 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()) { -#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 + *virtual_path = path; *type = file_system_operation_context_.dest_type(); - *origin_url = file_system_operation_context_.dest_origin_url(); return true; } - 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)) { + if (!file_system_context()->path_manager()->CrackFileSystemPath( + path, origin_url, type, virtual_path)) { 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 4017235..f6d2016 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 GURL& path, + void CreateFile(const FilePath& path, bool exclusive); - void CreateDirectory(const GURL& path, + void CreateDirectory(const FilePath& path, bool exclusive, 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 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 Write(scoped_refptr<net::URLRequestContext> url_request_context, - const GURL& path, + const FilePath& path, const GURL& blob_url, int64 offset); - void Truncate(const GURL& path, int64 length); - void TouchFile(const GURL& path, + void Truncate(const FilePath& path, int64 length); + void TouchFile(const FilePath& path, const base::Time& last_access_time, const base::Time& last_modified_time); @@ -142,7 +142,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 GURL& path, + bool VerifyFileSystemPathForRead(const FilePath& path, GURL* root_url, FileSystemType* type, FilePath* virtual_path); @@ -161,7 +161,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 GURL& path, + bool VerifyFileSystemPathForWrite(const FilePath& 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 292104b..75c9d31 100644 --- a/webkit/fileapi/file_system_operation_unittest.cc +++ b/webkit/fileapi/file_system_operation_unittest.cc @@ -56,16 +56,6 @@ 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_; @@ -101,7 +91,7 @@ class MockDispatcher : public FileSystemCallbackDispatcher { test_->set_entries(entries); } - virtual void DidOpenFileSystem(const std::string&, const GURL&) { + virtual void DidOpenFileSystem(const std::string&, const FilePath&) { NOTREACHED(); } @@ -123,15 +113,12 @@ 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) { - GURL src(URLForRelativePath("a")); - GURL dest(URLForRelativePath("b")); + FilePath src(base_.path().Append(FILE_PATH_LITERAL("a"))); + FilePath dest(base_.path().Append(FILE_PATH_LITERAL("b"))); operation()->Move(src, dest); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); @@ -144,7 +131,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureContainsPath) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir(src_dir.path(), FILE_PATH_LITERAL("child_dir"), &dest_dir_path)); - operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_dir_path)); + operation()->Move(src_dir.path(), dest_dir_path); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, status()); } @@ -159,7 +146,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcDirExistsDestFile) { FilePath dest_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file); - operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_file)); + operation()->Move(src_dir.path(), dest_file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, status()); } @@ -174,7 +161,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestNonEmptyDir) { FilePath child_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &child_file); - operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); + operation()->Move(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, status()); } @@ -189,7 +176,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureSrcFileExistsDestDir) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Move(URLForPath(src_file), URLForPath(dest_dir.path())); + operation()->Move(src_file, dest_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, status()); } @@ -202,7 +189,7 @@ TEST_F(FileSystemOperationTest, TestMoveFailureDestParentDoesntExist) { FILE_PATH_LITERAL("NonexistingDir")).Append( FILE_PATH_LITERAL("NonexistingFile")); - operation()->Move(URLForPath(src_dir.path()), URLForPath(nonexisting_file)); + operation()->Move(src_dir.path(), nonexisting_file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } @@ -218,7 +205,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndOverwrite) { FilePath dest_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file); - operation()->Move(URLForPath(src_file), URLForPath(dest_file)); + operation()->Move(src_file, dest_file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); @@ -234,7 +221,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcFileAndNew) { ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); FilePath dest_file(dest_dir.path().Append(FILE_PATH_LITERAL("NewFile"))); - operation()->Move(URLForPath(src_file), URLForPath(dest_file)); + operation()->Move(src_file, dest_file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); @@ -247,7 +234,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndOverwrite) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); + operation()->Move(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(src_dir.path())); @@ -266,7 +253,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirAndNew) { ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath dest_dir_path(dir.path().Append(FILE_PATH_LITERAL("NewDirectory"))); - operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_dir_path)); + operation()->Move(src_dir.path(), dest_dir_path); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(src_dir.path())); @@ -285,7 +272,7 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirRecursive) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Move(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); + operation()->Move(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(dest_dir.path().Append( @@ -296,7 +283,9 @@ TEST_F(FileSystemOperationTest, TestMoveSuccessSrcDirRecursive) { } TEST_F(FileSystemOperationTest, TestCopyFailureSrcDoesntExist) { - operation()->Copy(URLForRelativePath("a"), URLForRelativePath("b")); + FilePath src(base_.path().Append(FILE_PATH_LITERAL("a"))); + FilePath dest(base_.path().Append(FILE_PATH_LITERAL("b"))); + operation()->Copy(src, dest); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } @@ -308,7 +297,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureContainsPath) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir(src_dir.path(), FILE_PATH_LITERAL("child_dir"), &dest_dir_path)); - operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_dir_path)); + operation()->Copy(src_dir.path(), dest_dir_path); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_INVALID_OPERATION, status()); } @@ -323,7 +312,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcDirExistsDestFile) { FilePath dest_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file); - operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_file)); + operation()->Copy(src_dir.path(), dest_file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, status()); } @@ -338,7 +327,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestNonEmptyDir) { FilePath child_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &child_file); - operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); + operation()->Copy(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, status()); } @@ -353,7 +342,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureSrcFileExistsDestDir) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Copy(URLForPath(src_file), URLForPath(dest_dir.path())); + operation()->Copy(src_file, dest_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_FILE, status()); } @@ -369,7 +358,7 @@ TEST_F(FileSystemOperationTest, TestCopyFailureDestParentDoesntExist) { FilePath nonexisting_file = nonexisting.Append( FILE_PATH_LITERAL("DontExistFile")); - operation()->Copy(URLForPath(src_dir), URLForPath(nonexisting_file)); + operation()->Copy(src_dir, nonexisting_file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } @@ -385,7 +374,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndOverwrite) { FilePath dest_file; file_util::CreateTemporaryFileInDir(dest_dir.path(), &dest_file); - operation()->Copy(URLForPath(src_file), URLForPath(dest_file)); + operation()->Copy(src_file, dest_file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); @@ -401,7 +390,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcFileAndNew) { ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); FilePath dest_file(dest_dir.path().Append(FILE_PATH_LITERAL("NewFile"))); - operation()->Copy(URLForPath(src_file), URLForPath(dest_file)); + operation()->Copy(src_file, dest_file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(dest_file)); @@ -414,7 +403,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndOverwrite) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); + operation()->Copy(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); @@ -432,7 +421,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirAndNew) { ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath dest_dir(dir.path().Append(FILE_PATH_LITERAL("NewDirectory"))); - operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_dir)); + operation()->Copy(src_dir.path(), dest_dir); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(dest_dir)); @@ -450,7 +439,7 @@ TEST_F(FileSystemOperationTest, TestCopySuccessSrcDirRecursive) { ScopedTempDir dest_dir; ASSERT_TRUE(dest_dir.CreateUniqueTempDir()); - operation()->Copy(URLForPath(src_dir.path()), URLForPath(dest_dir.path())); + operation()->Copy(src_dir.path(), dest_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(dest_dir.path().Append( @@ -467,7 +456,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileFailure) { FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); - operation()->CreateFile(URLForPath(file), true); + operation()->CreateFile(file, true); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status()); } @@ -479,7 +468,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileExists) { FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); - operation()->CreateFile(URLForPath(file), false); + operation()->CreateFile(file, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(file)); @@ -490,7 +479,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessExclusive) { ScopedTempDir dir; ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath file = dir.path().Append(FILE_PATH_LITERAL("FileDoesntExist")); - operation()->CreateFile(URLForPath(file), true); + operation()->CreateFile(file, true); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(FileExists(file)); @@ -501,7 +490,7 @@ TEST_F(FileSystemOperationTest, TestCreateFileSuccessFileDoesntExist) { ScopedTempDir dir; ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath file = dir.path().Append(FILE_PATH_LITERAL("FileDoesntExist")); - operation()->CreateFile(URLForPath(file), false); + operation()->CreateFile(file, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); } @@ -513,7 +502,7 @@ TEST_F(FileSystemOperationTest, FILE_PATH_LITERAL("DirDoesntExist"))); FilePath nonexisting_file = nonexisting.Append( FILE_PATH_LITERAL("FileDoesntExist")); - operation()->CreateDirectory(URLForPath(nonexisting_file), false, false); + operation()->CreateDirectory(nonexisting_file, false, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } @@ -522,7 +511,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirFailureDirExists) { // Exclusive and dir existing at path. ScopedTempDir src_dir; ASSERT_TRUE(src_dir.CreateUniqueTempDir()); - operation()->CreateDirectory(URLForPath(src_dir.path()), true, false); + operation()->CreateDirectory(src_dir.path(), true, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status()); } @@ -533,7 +522,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirFailureFileExists) { ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); - operation()->CreateDirectory(URLForPath(file), true, false); + operation()->CreateDirectory(file, true, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_EXISTS, status()); } @@ -542,14 +531,14 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccess) { // Dir exists and exclusive is false. ScopedTempDir dir; ASSERT_TRUE(dir.CreateUniqueTempDir()); - operation()->CreateDirectory(URLForPath(dir.path()), false, false); + operation()->CreateDirectory(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(URLForPath(nonexisting_dir_path), false, false); + operation()->CreateDirectory(nonexisting_dir_path, false, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(nonexisting_dir_path)); @@ -560,7 +549,7 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccessExclusive) { FilePath nonexisting_dir_path(base_.path().Append( FILE_PATH_LITERAL("nonexistingdir"))); - operation()->CreateDirectory(URLForPath(nonexisting_dir_path), true, false); + operation()->CreateDirectory(nonexisting_dir_path, true, false); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(file_util::DirectoryExists(nonexisting_dir_path)); @@ -569,16 +558,16 @@ TEST_F(FileSystemOperationTest, TestCreateDirSuccessExclusive) { TEST_F(FileSystemOperationTest, TestExistsAndMetadataFailure) { FilePath nonexisting_dir_path(base_.path().Append( FILE_PATH_LITERAL("nonexistingdir"))); - operation()->GetMetadata(URLForPath(nonexisting_dir_path)); + operation()->GetMetadata(nonexisting_dir_path); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); - operation()->FileExists(URLForPath(nonexisting_dir_path)); + operation()->FileExists(nonexisting_dir_path); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); file_util::EnsureEndsWithSeparator(&nonexisting_dir_path); - operation()->DirectoryExists(URLForPath(nonexisting_dir_path)); + operation()->DirectoryExists(nonexisting_dir_path); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); } @@ -587,11 +576,11 @@ TEST_F(FileSystemOperationTest, TestExistsAndMetadataSuccess) { ScopedTempDir dir; ASSERT_TRUE(dir.CreateUniqueTempDir()); - operation()->DirectoryExists(URLForPath(dir.path())); + operation()->DirectoryExists(dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); - operation()->GetMetadata(URLForPath(dir.path())); + operation()->GetMetadata(dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_TRUE(info().is_directory); @@ -599,11 +588,11 @@ TEST_F(FileSystemOperationTest, TestExistsAndMetadataSuccess) { FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); - operation()->FileExists(URLForPath(file)); + operation()->FileExists(file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); - operation()->GetMetadata(URLForPath(file)); + operation()->GetMetadata(file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(info().is_directory); @@ -613,13 +602,13 @@ TEST_F(FileSystemOperationTest, TestExistsAndMetadataSuccess) { TEST_F(FileSystemOperationTest, TestTypeMismatchErrors) { ScopedTempDir dir; ASSERT_TRUE(dir.CreateUniqueTempDir()); - operation()->FileExists(URLForPath(dir.path())); + operation()->FileExists(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(URLForPath(file)); + operation()->DirectoryExists(file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_A_DIRECTORY, status()); } @@ -629,7 +618,7 @@ TEST_F(FileSystemOperationTest, TestReadDirFailure) { FilePath nonexisting_dir_path(base_.path().Append( FILE_PATH_LITERAL("NonExistingDir"))); file_util::EnsureEndsWithSeparator(&nonexisting_dir_path); - operation()->ReadDirectory(URLForPath(nonexisting_dir_path)); + operation()->ReadDirectory(nonexisting_dir_path); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); @@ -638,7 +627,7 @@ TEST_F(FileSystemOperationTest, TestReadDirFailure) { ASSERT_TRUE(dir.CreateUniqueTempDir()); FilePath file; file_util::CreateTemporaryFileInDir(dir.path(), &file); - operation()->ReadDirectory(URLForPath(file)); + operation()->ReadDirectory(file); MessageLoop::current()->RunAllPending(); // TODO(kkanetkar) crbug.com/54309 to change the error code. EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); @@ -657,7 +646,7 @@ TEST_F(FileSystemOperationTest, TestReadDirSuccess) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir( parent_dir.path(), FILE_PATH_LITERAL("child_dir"), &child_dir)); - operation()->ReadDirectory(URLForPath(parent_dir.path())); + operation()->ReadDirectory(parent_dir.path()); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationStatusNotSet, status()); EXPECT_EQ(2u, entries().size()); @@ -679,7 +668,7 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) { FILE_PATH_LITERAL("NonExistingDir"))); file_util::EnsureEndsWithSeparator(&nonexisting); - operation()->Remove(URLForPath(nonexisting), false /* recursive */); + operation()->Remove(nonexisting, false /* recursive */); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_FOUND, status()); @@ -697,7 +686,7 @@ TEST_F(FileSystemOperationTest, TestRemoveFailure) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir( parent_dir.path(), FILE_PATH_LITERAL("child_dir"), &child_dir)); - operation()->Remove(URLForPath(parent_dir.path()), false /* recursive */); + operation()->Remove(parent_dir.path(), false /* recursive */); MessageLoop::current()->RunAllPending(); EXPECT_EQ(base::PLATFORM_FILE_ERROR_NOT_EMPTY, status()); @@ -708,7 +697,7 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) { ASSERT_TRUE(empty_dir.CreateUniqueTempDir()); EXPECT_TRUE(file_util::DirectoryExists(empty_dir.path())); - operation()->Remove(URLForPath(empty_dir.path()), false /* recursive */); + operation()->Remove(empty_dir.path(), false /* recursive */); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(empty_dir.path())); @@ -726,7 +715,7 @@ TEST_F(FileSystemOperationTest, TestRemoveSuccess) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir( parent_dir.path(), FILE_PATH_LITERAL("child_dir"), &child_dir)); - operation()->Remove(URLForPath(parent_dir.path()), true /* recursive */); + operation()->Remove(parent_dir.path(), true /* recursive */); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(file_util::DirectoryExists(parent_dir.path())); @@ -744,7 +733,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(URLForPath(file)); + operation()->GetMetadata(file); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); EXPECT_FALSE(info().is_directory); @@ -752,7 +741,7 @@ TEST_F(FileSystemOperationTest, TestTruncate) { // Extend the file by truncating it. int length = 17; - operation()->Truncate(URLForPath(file), length); + operation()->Truncate(file, length); MessageLoop::current()->RunAllPending(); EXPECT_EQ(kFileOperationSucceeded, status()); @@ -773,7 +762,7 @@ TEST_F(FileSystemOperationTest, TestTruncate) { // Shorten the file by truncating it. length = 3; - operation()->Truncate(URLForPath(file), length); + operation()->Truncate(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 a678b0a..e331338 100644 --- a/webkit/fileapi/file_system_operation_write_unittest.cc +++ b/webkit/fileapi/file_system_operation_write_unittest.cc @@ -10,6 +10,7 @@ #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" @@ -80,16 +81,6 @@ class FileSystemOperationTest : 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_; @@ -128,7 +119,7 @@ class MockDispatcher : public FileSystemCallbackDispatcher { ADD_FAILURE(); } - virtual void DidOpenFileSystem(const std::string&, const GURL&) { + virtual void DidOpenFileSystem(const std::string&, const FilePath&) { ADD_FAILURE(); } @@ -174,7 +165,7 @@ TEST_F(FileSystemOperationTest, TestWriteSuccess) { url_request_context->blob_storage_controller()-> RegisterBlobUrl(blob_url, blob_data); - operation()->Write(url_request_context, URLForPath(file_), blob_url, 0); + operation()->Write(url_request_context, file_, blob_url, 0); MessageLoop::current()->Run(); url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); @@ -194,7 +185,7 @@ TEST_F(FileSystemOperationTest, TestWriteZero) { url_request_context->blob_storage_controller()-> RegisterBlobUrl(blob_url, blob_data); - operation()->Write(url_request_context, URLForPath(file_), blob_url, 0); + operation()->Write(url_request_context, file_, blob_url, 0); MessageLoop::current()->Run(); url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); @@ -208,8 +199,7 @@ TEST_F(FileSystemOperationTest, TestWriteInvalidBlobUrl) { scoped_refptr<TestURLRequestContext> url_request_context( new TestURLRequestContext()); - operation()->Write(url_request_context, URLForPath(file_), - GURL("blob:invalid"), 0); + operation()->Write(url_request_context, file_, GURL("blob:invalid"), 0); MessageLoop::current()->Run(); EXPECT_EQ(0, bytes_written()); @@ -228,7 +218,7 @@ TEST_F(FileSystemOperationTest, TestWriteInvalidFile) { RegisterBlobUrl(blob_url, blob_data); operation()->Write(url_request_context, - URLForRelativePath("nonexist"), blob_url, 0); + FilePath(FILE_PATH_LITERAL("/nonexist")), blob_url, 0); MessageLoop::current()->Run(); url_request_context->blob_storage_controller()->UnregisterBlobUrl(blob_url); @@ -248,7 +238,7 @@ TEST_F(FileSystemOperationTest, TestWriteDir) { url_request_context->blob_storage_controller()-> RegisterBlobUrl(blob_url, blob_data); - operation()->Write(url_request_context, URLForPath(dir_.path()), blob_url, 0); + operation()->Write(url_request_context, 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 16fc60b..558dc31 100644 --- a/webkit/fileapi/file_system_path_manager.cc +++ b/webkit/fileapi/file_system_path_manager.cc @@ -100,6 +100,83 @@ 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. @@ -135,29 +212,6 @@ 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 19718ae..5a9613a 100644 --- a/webkit/fileapi/file_system_path_manager.h +++ b/webkit/fileapi/file_system_path_manager.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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,6 +68,14 @@ 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. @@ -81,9 +89,6 @@ 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 8a99b84..fee38c1 100644 --- a/webkit/fileapi/file_system_path_manager_unittest.cc +++ b/webkit/fileapi/file_system_path_manager_unittest.cc @@ -228,6 +228,11 @@ 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( @@ -362,6 +367,89 @@ TEST_F(FileSystemPathManagerTest, GetRootPathFileURIWithAllowFlag) { } } +TEST_F(FileSystemPathManagerTest, VirtualPathFromFileSystemPathTest) { + scoped_ptr<FileSystemPathManager> 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<FileSystemPathManager> 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<FileSystemPathManager> 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<FileSystemPathManager> 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 4993e7f..a15c2a8 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) -#if defined(OS_WIN) - *file_path = FilePath(base::SysUTF8ToWide(path)). - NormalizeWindowsPathSeparators(); -#elif defined(OS_POSIX) - *file_path = path; -#endif + *file_path = FilePath(sys_path); return true; } diff --git a/webkit/fileapi/file_system_util_unittest.cc b/webkit/fileapi/file_system_util_unittest.cc index 41863ae..e08fec7 100644 --- a/webkit/fileapi/file_system_util_unittest.cc +++ b/webkit/fileapi/file_system_util_unittest.cc @@ -29,8 +29,7 @@ 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("file"), file_path_.BaseName().value()); - EXPECT_EQ(FILE_PATH_LITERAL("directory"), file_path_.DirName().value()); + EXPECT_EQ(FILE_PATH_LITERAL("directory/file"), file_path_.value()); } TEST_F(FileSystemUtilTest, ParseTemporary) { @@ -38,8 +37,7 @@ 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("file"), file_path_.BaseName().value()); - EXPECT_EQ(FILE_PATH_LITERAL("directory"), file_path_.DirName().value()); + EXPECT_EQ(FILE_PATH_LITERAL("directory/file"), file_path_.value()); } TEST_F(FileSystemUtilTest, EnsureFilePathIsRelative) { @@ -47,8 +45,7 @@ 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("file"), file_path_.BaseName().value()); - EXPECT_EQ(FILE_PATH_LITERAL("directory"), file_path_.DirName().value()); + EXPECT_EQ(FILE_PATH_LITERAL("directory/file"), file_path_.value()); EXPECT_FALSE(file_path_.IsAbsolute()); } @@ -62,8 +59,7 @@ TEST_F(FileSystemUtilTest, RejectBadSchemes) { TEST_F(FileSystemUtilTest, UnescapePath) { ASSERT_TRUE(CrackFileSystemURL( "filesystem:http://chromium.org/persistent/%7Echromium/space%20bar")); - EXPECT_EQ(FILE_PATH_LITERAL("space bar"), file_path_.BaseName().value()); - EXPECT_EQ(FILE_PATH_LITERAL("~chromium"), file_path_.DirName().value()); + EXPECT_EQ(FILE_PATH_LITERAL("~chromium/space bar"), file_path_.value()); } TEST_F(FileSystemUtilTest, RejectBadType) { diff --git a/webkit/fileapi/webfilewriter_base.cc b/webkit/fileapi/webfilewriter_base.cc index 1e0e8a3..cf2a7bc 100644 --- a/webkit/fileapi/webfilewriter_base.cc +++ b/webkit/fileapi/webfilewriter_base.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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 GURL& path, WebKit::WebFileWriterClient* client) - : path_(path), + const WebKit::WebString& path, WebKit::WebFileWriterClient* client) + : path_(webkit_glue::WebStringToFilePath(path)), client_(client), operation_(kOperationNone), cancel_state_(kCancelNotInProgress) { diff --git a/webkit/fileapi/webfilewriter_base.h b/webkit/fileapi/webfilewriter_base.h index 3601256..f47b383 100644 --- a/webkit/fileapi/webfilewriter_base.h +++ b/webkit/fileapi/webfilewriter_base.h @@ -1,14 +1,16 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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; @@ -20,7 +22,7 @@ namespace fileapi { class WebFileWriterBase : public WebKit::WebFileWriter { public: WebFileWriterBase( - const GURL& path, WebKit::WebFileWriterClient* client); + const WebKit::WebString& path, WebKit::WebFileWriterClient* client); virtual ~WebFileWriterBase(); // WebFileWriter implementation @@ -32,8 +34,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 GURL& path, int64 offset) = 0; - virtual void DoWrite(const GURL& path, const GURL& blob_url, + virtual void DoTruncate(const FilePath& path, int64 offset) = 0; + virtual void DoWrite(const FilePath& path, const GURL& blob_url, int64 offset) = 0; virtual void DoCancel() = 0; @@ -56,7 +58,7 @@ class WebFileWriterBase : public WebKit::WebFileWriter { void FinishCancel(); - GURL path_; + FilePath 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 43164e7..7a07cd2 100644 --- a/webkit/fileapi/webfilewriter_base_unittest.cc +++ b/webkit/fileapi/webfilewriter_base_unittest.cc @@ -31,8 +31,16 @@ const int kMultiFileWrite_Offset = 3; const int kCancelFileWriteBeforeCompletion_Offset = 4; const int kCancelFileWriteAfterCompletion_Offset = 5; -GURL mock_path_as_gurl() { - return GURL("MockPath"); +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()); } } // namespace @@ -40,32 +48,32 @@ GURL mock_path_as_gurl() { class TestableFileWriter : public WebFileWriterBase { public: explicit TestableFileWriter(WebKit::WebFileWriterClient* client) - : WebFileWriterBase(mock_path_as_gurl(), client) { + : WebFileWriterBase(mock_path_as_string16(), client) { reset(); } void reset() { received_truncate_ = false; - received_truncate_path_ = GURL(); + received_truncate_path_ = FilePath(); received_truncate_offset_ = kNoOffset; received_write_ = false; - received_write_path_ = GURL(); + received_write_path_ = FilePath(); received_write_offset_ = kNoOffset; received_write_blob_url_ = GURL(); received_cancel_ = false; } bool received_truncate_; - GURL received_truncate_path_; + FilePath received_truncate_path_; int64 received_truncate_offset_; bool received_write_; - GURL received_write_path_; + FilePath received_write_path_; GURL received_write_blob_url_; int64 received_write_offset_; bool received_cancel_; protected: - virtual void DoTruncate(const GURL& path, int64 offset) { + virtual void DoTruncate(const FilePath& path, int64 offset) { received_truncate_ = true; received_truncate_path_ = path; received_truncate_offset_ = offset; @@ -87,7 +95,7 @@ class TestableFileWriter : public WebFileWriterBase { } } - virtual void DoWrite(const GURL& path, const GURL& blob_url, + virtual void DoWrite(const FilePath& path, const GURL& blob_url, int64 offset) { received_write_ = true; received_write_path_ = path; @@ -197,8 +205,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_, - mock_path_as_gurl()); + EXPECT_EQ(testable_writer_->received_write_path_.value(), + mock_path_as_file_path().value()); EXPECT_EQ(kBasicFileWrite_Offset, testable_writer_->received_write_offset_); EXPECT_EQ(kBlobUrl, testable_writer_->received_write_blob_url_); @@ -219,8 +227,8 @@ TEST_F(FileWriterTest, BasicFileTruncate) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_truncate_); - EXPECT_EQ(mock_path_as_gurl(), - testable_writer_->received_truncate_path_); + EXPECT_EQ(mock_path_as_file_path().value(), + testable_writer_->received_truncate_path_.value()); EXPECT_EQ(kBasicFileTruncate_Offset, testable_writer_->received_truncate_offset_); EXPECT_FALSE(testable_writer_->received_write_); @@ -239,8 +247,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_, - mock_path_as_gurl()); + EXPECT_EQ(testable_writer_->received_write_path_.value(), + mock_path_as_file_path().value()); EXPECT_EQ(kErrorFileWrite_Offset, testable_writer_->received_write_offset_); EXPECT_EQ(kBlobUrl, testable_writer_->received_write_blob_url_); @@ -260,8 +268,8 @@ TEST_F(FileWriterTest, ErrorFileTruncate) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_truncate_); - EXPECT_EQ(mock_path_as_gurl(), - testable_writer_->received_truncate_path_); + EXPECT_EQ(mock_path_as_file_path().value(), + testable_writer_->received_truncate_path_.value()); EXPECT_EQ(kErrorFileTruncate_Offset, testable_writer_->received_truncate_offset_); EXPECT_FALSE(testable_writer_->received_write_); @@ -281,8 +289,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_, - mock_path_as_gurl()); + EXPECT_EQ(testable_writer_->received_write_path_.value(), + mock_path_as_file_path().value()); EXPECT_EQ(kMultiFileWrite_Offset, testable_writer_->received_write_offset_); EXPECT_EQ(kBlobUrl, testable_writer_->received_write_blob_url_); @@ -304,8 +312,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_, - mock_path_as_gurl()); + EXPECT_EQ(testable_writer_->received_write_path_.value(), + mock_path_as_file_path().value()); EXPECT_EQ(kCancelFileWriteBeforeCompletion_Offset, testable_writer_->received_write_offset_); EXPECT_EQ(kBlobUrl, testable_writer_->received_write_blob_url_); @@ -328,8 +336,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_, - mock_path_as_gurl()); + EXPECT_EQ(testable_writer_->received_write_path_.value(), + mock_path_as_file_path().value()); EXPECT_EQ(kCancelFileWriteAfterCompletion_Offset, testable_writer_->received_write_offset_); EXPECT_EQ(kBlobUrl, testable_writer_->received_write_blob_url_); @@ -351,8 +359,8 @@ TEST_F(FileWriterTest, CancelFileTruncate) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_truncate_); - EXPECT_EQ(mock_path_as_gurl(), - testable_writer_->received_truncate_path_); + EXPECT_EQ(mock_path_as_file_path().value(), + testable_writer_->received_truncate_path_.value()); EXPECT_EQ(kCancelFileTruncate_Offset, testable_writer_->received_truncate_offset_); EXPECT_TRUE(testable_writer_->received_cancel_); @@ -371,8 +379,8 @@ TEST_F(FileWriterTest, CancelFailedTruncate) { // Check that the derived class gets called correctly. EXPECT_TRUE(testable_writer_->received_truncate_); - EXPECT_EQ(mock_path_as_gurl(), - testable_writer_->received_truncate_path_); + EXPECT_EQ(mock_path_as_file_path().value(), + testable_writer_->received_truncate_path_.value()); EXPECT_EQ(kCancelFailedTruncate_Offset, testable_writer_->received_truncate_offset_); EXPECT_TRUE(testable_writer_->received_cancel_); |