Skip to content

Commit

Permalink
[fsp] Do not allow write operations on read only file systems.
Browse files Browse the repository at this point in the history 
A writable option for provided file systems has been added recently. This CL
adds checks at early stage to fail writing related operations on file systems
which were not mounted as R/W.

TEST=unit_tests: *FileSystemProvider*ReadOnly*
BUG=393506

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285878 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
mtomasz@chromium.org committed Jul 28, 2014
1 parent e4b9afd commit 5b48ea6
Show file tree
Hide file tree
Showing 15 changed files with 207 additions and 2 deletions.
3 changes: 3 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/copy_entry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ CopyEntry::~CopyEntry() {
}

bool CopyEntry::Execute(int request_id) {
if (!file_system_info_.writable())
return false;

scoped_ptr<base::DictionaryValue> values(new base::DictionaryValue);
values->SetString("sourcePath", source_path_.AsUTF8Unsafe());
values->SetString("targetPath", target_path_.AsUTF8Unsafe());
Expand Down
23 changes: 23 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/copy_entry_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,29 @@ TEST_F(FileSystemProviderOperationsCopyEntryTest, Execute_NoListener) {
EXPECT_FALSE(copy_entry.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsCopyEntryTest, Execute_ReadOnly) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;

const ProvidedFileSystemInfo read_only_file_system_info(
kExtensionId,
kFileSystemId,
"" /* file_system_name */,
false /* writable */,
base::FilePath() /* mount_path */);

CopyEntry copy_entry(NULL,
read_only_file_system_info,
base::FilePath::FromUTF8Unsafe(kSourcePath),
base::FilePath::FromUTF8Unsafe(kTargetPath),
base::Bind(&util::LogStatusCallback, &callback_log));
copy_entry.SetDispatchEventImplForTesting(
base::Bind(&util::LoggingDispatchEventImpl::OnDispatchEventImpl,
base::Unretained(&dispatcher)));

EXPECT_FALSE(copy_entry.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsCopyEntryTest, OnSuccess) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/create_directory.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ CreateDirectory::~CreateDirectory() {
}

bool CreateDirectory::Execute(int request_id) {
if (!file_system_info_.writable())
return false;

scoped_ptr<base::DictionaryValue> values(new base::DictionaryValue);
values->SetString("directoryPath", directory_path_.AsUTF8Unsafe());
values->SetBoolean("exclusive", exclusive_);
Expand Down
25 changes: 25 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/create_directory_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,31 @@ TEST_F(FileSystemProviderOperationsCreateDirectoryTest, Execute_NoListener) {
EXPECT_FALSE(create_directory.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsCreateDirectoryTest, Execute_ReadOnly) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;

const ProvidedFileSystemInfo read_only_file_system_info(
kExtensionId,
kFileSystemId,
"" /* file_system_name */,
false /* writable */,
base::FilePath() /* mount_path */);

CreateDirectory create_directory(
NULL,
read_only_file_system_info,
base::FilePath::FromUTF8Unsafe(kDirectoryPath),
false /* exclusive */,
true /* recursive */,
base::Bind(&util::LogStatusCallback, &callback_log));
create_directory.SetDispatchEventImplForTesting(
base::Bind(&util::LoggingDispatchEventImpl::OnDispatchEventImpl,
base::Unretained(&dispatcher)));

EXPECT_FALSE(create_directory.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsCreateDirectoryTest, OnSuccess) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/create_file.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ CreateFile::~CreateFile() {
}

bool CreateFile::Execute(int request_id) {
if (!file_system_info_.writable())
return false;

scoped_ptr<base::DictionaryValue> values(new base::DictionaryValue);
values->SetString("filePath", file_path_.AsUTF8Unsafe());

Expand Down
22 changes: 22 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/create_file_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,28 @@ TEST_F(FileSystemProviderOperationsCreateFileTest, Execute_NoListener) {
EXPECT_FALSE(create_file.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsCreateFileTest, Execute_ReadOnly) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;

const ProvidedFileSystemInfo read_only_file_system_info(
kExtensionId,
kFileSystemId,
"" /* file_system_name */,
false /* writable */,
base::FilePath() /* mount_path */);

CreateFile create_file(NULL,
read_only_file_system_info,
base::FilePath::FromUTF8Unsafe(kFilePath),
base::Bind(&util::LogStatusCallback, &callback_log));
create_file.SetDispatchEventImplForTesting(
base::Bind(&util::LoggingDispatchEventImpl::OnDispatchEventImpl,
base::Unretained(&dispatcher)));

EXPECT_FALSE(create_file.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsCreateFileTest, OnSuccess) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/delete_entry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ DeleteEntry::~DeleteEntry() {
}

bool DeleteEntry::Execute(int request_id) {
if (!file_system_info_.writable())
return false;

scoped_ptr<base::DictionaryValue> values(new base::DictionaryValue);
values->SetString("entryPath", entry_path_.AsUTF8Unsafe());
values->SetBoolean("recursive", recursive_);
Expand Down
25 changes: 24 additions & 1 deletion chrome/browser/chromeos/file_system_provider/operations/delete_entry_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class FileSystemProviderOperationsDeleteEntryTest : public testing::Test {
ProvidedFileSystemInfo(kExtensionId,
kFileSystemId,
"" /* file_system_name */,
false /* writable */,
true /* writable */,
base::FilePath() /* mount_path */);
}

Expand Down Expand Up @@ -107,6 +107,29 @@ TEST_F(FileSystemProviderOperationsDeleteEntryTest, Execute_NoListener) {
EXPECT_FALSE(delete_entry.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsDeleteEntryTest, Execute_ReadOnly) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;

const ProvidedFileSystemInfo read_only_file_system_info(
kExtensionId,
kFileSystemId,
"" /* file_system_name */,
false /* writable */,
base::FilePath() /* mount_path */);

DeleteEntry delete_entry(NULL,
read_only_file_system_info,
base::FilePath::FromUTF8Unsafe(kEntryPath),
true /* recursive */,
base::Bind(&util::LogStatusCallback, &callback_log));
delete_entry.SetDispatchEventImplForTesting(
base::Bind(&util::LoggingDispatchEventImpl::OnDispatchEventImpl,
base::Unretained(&dispatcher)));

EXPECT_FALSE(delete_entry.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsDeleteEntryTest, OnSuccess) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/move_entry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ MoveEntry::~MoveEntry() {
}

bool MoveEntry::Execute(int request_id) {
if (!file_system_info_.writable())
return false;

scoped_ptr<base::DictionaryValue> values(new base::DictionaryValue);
values->SetString("sourcePath", source_path_.AsUTF8Unsafe());
values->SetString("targetPath", target_path_.AsUTF8Unsafe());
Expand Down
23 changes: 23 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/move_entry_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,29 @@ TEST_F(FileSystemProviderOperationsMoveEntryTest, Execute_NoListener) {
EXPECT_FALSE(move_entry.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsMoveEntryTest, Execute_ReadOnly) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;

const ProvidedFileSystemInfo read_only_file_system_info(
kExtensionId,
kFileSystemId,
"" /* file_system_name */,
false /* writable */,
base::FilePath() /* mount_path */);

MoveEntry move_entry(NULL,
read_only_file_system_info,
base::FilePath::FromUTF8Unsafe(kSourcePath),
base::FilePath::FromUTF8Unsafe(kTargetPath),
base::Bind(&util::LogStatusCallback, &callback_log));
move_entry.SetDispatchEventImplForTesting(
base::Bind(&util::LoggingDispatchEventImpl::OnDispatchEventImpl,
base::Unretained(&dispatcher)));

EXPECT_FALSE(move_entry.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsMoveEntryTest, OnSuccess) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/open_file.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ OpenFile::~OpenFile() {
}

bool OpenFile::Execute(int request_id) {
if (!file_system_info_.writable() &&
mode_ == ProvidedFileSystemInterface::OPEN_FILE_MODE_WRITE) {
return false;
}

scoped_ptr<base::DictionaryValue> values(new base::DictionaryValue);
values->SetString("filePath", file_path_.AsUTF8Unsafe());

Expand Down
42 changes: 42 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/open_file_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,48 @@ TEST_F(FileSystemProviderOperationsOpenFileTest, Execute_NoListener) {
EXPECT_FALSE(open_file.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsOpenFileTest, Execute_ReadOnly) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
CallbackLogger callback_logger;

const ProvidedFileSystemInfo read_only_file_system_info(
kExtensionId,
kFileSystemId,
"" /* file_system_name */,
false /* writable */,
base::FilePath() /* mount_path */);

// Opening for read on a read-only file system is allowed.
{
OpenFile open_file(NULL,
read_only_file_system_info,
base::FilePath::FromUTF8Unsafe(kFilePath),
ProvidedFileSystemInterface::OPEN_FILE_MODE_READ,
base::Bind(&CallbackLogger::OnOpenFile,
base::Unretained(&callback_logger)));
open_file.SetDispatchEventImplForTesting(
base::Bind(&util::LoggingDispatchEventImpl::OnDispatchEventImpl,
base::Unretained(&dispatcher)));

EXPECT_TRUE(open_file.Execute(kRequestId));
}

// Opening for write on a read-only file system is forbidden and must fail.
{
OpenFile open_file(NULL,
read_only_file_system_info,
base::FilePath::FromUTF8Unsafe(kFilePath),
ProvidedFileSystemInterface::OPEN_FILE_MODE_WRITE,
base::Bind(&CallbackLogger::OnOpenFile,
base::Unretained(&callback_logger)));
open_file.SetDispatchEventImplForTesting(
base::Bind(&util::LoggingDispatchEventImpl::OnDispatchEventImpl,
base::Unretained(&dispatcher)));

EXPECT_FALSE(open_file.Execute(kRequestId));
}
}

TEST_F(FileSystemProviderOperationsOpenFileTest, OnSuccess) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
CallbackLogger callback_logger;
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/truncate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ Truncate::~Truncate() {
}

bool Truncate::Execute(int request_id) {
if (!file_system_info_.writable())
return false;

scoped_ptr<base::DictionaryValue> values(new base::DictionaryValue);
values->SetString("filePath", file_path_.AsUTF8Unsafe());
values->SetDouble("length", length_);
Expand Down
23 changes: 23 additions & 0 deletions chrome/browser/chromeos/file_system_provider/operations/truncate_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,29 @@ TEST_F(FileSystemProviderOperationsTruncateTest, Execute_NoListener) {
EXPECT_FALSE(truncate.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsTruncateTest, Execute_ReadOnly) {
util::LoggingDispatchEventImpl dispatcher(false /* dispatch_reply */);
util::StatusCallbackLog callback_log;

const ProvidedFileSystemInfo read_only_file_system_info(
kExtensionId,
kFileSystemId,
"" /* file_system_name */,
false /* writable */,
base::FilePath() /* mount_path */);

Truncate truncate(NULL,
file_system_info_,
base::FilePath::FromUTF8Unsafe(kFilePath),
kTruncateLength,
base::Bind(&util::LogStatusCallback, &callback_log));
truncate.SetDispatchEventImplForTesting(
base::Bind(&util::LoggingDispatchEventImpl::OnDispatchEventImpl,
base::Unretained(&dispatcher)));

EXPECT_FALSE(truncate.Execute(kRequestId));
}

TEST_F(FileSystemProviderOperationsTruncateTest, OnSuccess) {
util::LoggingDispatchEventImpl dispatcher(true /* dispatch_reply */);
util::StatusCallbackLog callback_log;
Expand Down
3 changes: 2 additions & 1 deletion chrome/test/data/extensions/api_test/file_system_provider/test_util/test_util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ test_util.mountFileSystem = function(callback) {
chrome.fileSystemProvider.mount(
{
fileSystemId: test_util.FILE_SYSTEM_ID,
displayName: test_util.FILE_SYSTEM_NAME
displayName: test_util.FILE_SYSTEM_NAME,
writable: true
},
function() {
var volumeId =
Expand Down

0 comments on commit 5b48ea6

Please sign in to comment.