diff options
author | ahendrickson@chromium.org <ahendrickson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-14 03:19:03 +0000 |
---|---|---|
committer | ahendrickson@chromium.org <ahendrickson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-14 03:19:03 +0000 |
commit | f798d7a4a79a04ea303558e1b24d4161fb0b84d7 (patch) | |
tree | a7d638b9f4ade253029b9aaa418afa0f5a8510f5 | |
parent | 1c69f8b7c992962fc6387d31eeeb5a3b16c93660 (diff) | |
download | chromium_src-f798d7a4a79a04ea303558e1b24d4161fb0b84d7.zip chromium_src-f798d7a4a79a04ea303558e1b24d4161fb0b84d7.tar.gz chromium_src-f798d7a4a79a04ea303558e1b24d4161fb0b84d7.tar.bz2 |
Added read-only file error test.
Depends on CLs 9568003 and 9570005.
BUG=None
TEST=None
Review URL: http://codereview.chromium.org/9355050
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@126541 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | base/test/test_file_util.h | 17 | ||||
-rw-r--r-- | base/test/test_file_util_posix.cc | 51 | ||||
-rw-r--r-- | base/test/test_file_util_win.cc | 63 | ||||
-rw-r--r-- | chrome/browser/download/download_browsertest.cc | 81 |
4 files changed, 206 insertions, 6 deletions
diff --git a/base/test/test_file_util.h b/base/test/test_file_util.h index ce0b302..99ec478f 100644 --- a/base/test/test_file_util.h +++ b/base/test/test_file_util.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -11,6 +11,7 @@ #include <string> #include "base/compiler_specific.h" +#include "base/file_path.h" class FilePath; @@ -56,6 +57,20 @@ FilePath WStringAsFilePath(const std::wstring& path); bool MakeFileUnreadable(const FilePath& path) WARN_UNUSED_RESULT; bool MakeFileUnwritable(const FilePath& path) WARN_UNUSED_RESULT; +// Saves the current permissions for a path, and restores it on destruction. +class PermissionRestorer { + public: + explicit PermissionRestorer(const FilePath& path); + ~PermissionRestorer(); + + private: + const FilePath path_; + void* info_; // The opaque stored permission information. + size_t length_; // The length of the stored permission information. + + DISALLOW_COPY_AND_ASSIGN(PermissionRestorer); +}; + } // namespace file_util #endif // BASE_TEST_TEST_FILE_UTIL_H_ diff --git a/base/test/test_file_util_posix.cc b/base/test/test_file_util_posix.cc index 0096c9e..67c450f 100644 --- a/base/test/test_file_util_posix.cc +++ b/base/test/test_file_util_posix.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -32,6 +32,43 @@ bool DenyFilePermission(const FilePath& path, mode_t permission) { return rv == 0; } +// Gets a blob indicating the permission information for |path|. +// |length| is the length of the blob. Zero on failure. +// Returns the blob pointer, or NULL on failure. +void* GetPermissionInfo(const FilePath& path, size_t* length) { + DCHECK(length); + *length = 0; + + struct stat stat_buf; + if (stat(path.value().c_str(), &stat_buf) != 0) + return NULL; + + *length = sizeof(mode_t); + mode_t* mode = new mode_t; + *mode = stat_buf.st_mode & ~S_IFMT; // Filter out file/path kind. + + return mode; +} + +// Restores the permission information for |path|, given the blob retrieved +// using |GetPermissionInfo()|. +// |info| is the pointer to the blob. +// |length| is the length of the blob. +// Either |info| or |length| may be NULL/0, in which case nothing happens. +bool RestorePermissionInfo(const FilePath& path, void* info, size_t length) { + if (!info || (length == 0)) + return false; + + DCHECK_EQ(sizeof(mode_t), length); + mode_t* mode = reinterpret_cast<mode_t*>(info); + + int rv = HANDLE_EINTR(chmod(path.value().c_str(), *mode)); + + delete mode; + + return rv == 0; +} + } // namespace bool DieFileDie(const FilePath& file, bool recurse) { @@ -140,4 +177,16 @@ bool MakeFileUnwritable(const FilePath& path) { return DenyFilePermission(path, S_IWUSR | S_IWGRP | S_IWOTH); } +PermissionRestorer::PermissionRestorer(const FilePath& path) + : path_(path), info_(NULL), length_(0) { + info_ = GetPermissionInfo(path_, &length_); + DCHECK(info_ != NULL); + DCHECK_NE(0u, length_); +} + +PermissionRestorer::~PermissionRestorer() { + if (!RestorePermissionInfo(path_, info_, length_)) + NOTREACHED(); +} + } // namespace file_util diff --git a/base/test/test_file_util_win.cc b/base/test/test_file_util_win.cc index f028080..41d69ea 100644 --- a/base/test/test_file_util_win.cc +++ b/base/test/test_file_util_win.cc @@ -23,6 +23,11 @@ static const ptrdiff_t kOneMB = 1024 * 1024; namespace { +struct PermissionInfo { + PSECURITY_DESCRIPTOR security_descriptor; + ACL dacl; +}; + // Deny |permission| on the file |path|, for the current user. bool DenyFilePermission(const FilePath& path, DWORD permission) { PACL old_dacl; @@ -59,6 +64,52 @@ bool DenyFilePermission(const FilePath& path, DWORD permission) { return rc == ERROR_SUCCESS; } +// Gets a blob indicating the permission information for |path|. +// |length| is the length of the blob. Zero on failure. +// Returns the blob pointer, or NULL on failure. +void* GetPermissionInfo(const FilePath& path, size_t* length) { + DCHECK(length != NULL); + *length = 0; + PACL dacl = NULL; + PSECURITY_DESCRIPTOR security_descriptor; + if (GetNamedSecurityInfo(const_cast<wchar_t*>(path.value().c_str()), + SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, NULL, NULL, &dacl, + NULL, &security_descriptor) != ERROR_SUCCESS) { + return NULL; + } + DCHECK(dacl != NULL); + + *length = sizeof(PSECURITY_DESCRIPTOR) + dacl->AclSize; + PermissionInfo* info = reinterpret_cast<PermissionInfo*>(new char[*length]); + info->security_descriptor = security_descriptor; + memcpy(&info->dacl, dacl, dacl->AclSize); + + return info; +} + +// Restores the permission information for |path|, given the blob retrieved +// using |GetPermissionInfo()|. +// |info| is the pointer to the blob. +// |length| is the length of the blob. +// Either |info| or |length| may be NULL/0, in which case nothing happens. +bool RestorePermissionInfo(const FilePath& path, void* info, size_t length) { + if (!info || !length) + return false; + + PermissionInfo* perm = reinterpret_cast<PermissionInfo*>(info); + + DWORD rc = SetNamedSecurityInfo(const_cast<wchar_t*>(path.value().c_str()), + SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, + NULL, NULL, &perm->dacl, NULL); + LocalFree(perm->security_descriptor); + + char* char_array = reinterpret_cast<char*>(info); + delete [] char_array; + + return rc == ERROR_SUCCESS; +} + } // namespace bool DieFileDie(const FilePath& file, bool recurse) { @@ -277,4 +328,16 @@ bool MakeFileUnwritable(const FilePath& path) { return DenyFilePermission(path, GENERIC_WRITE); } +PermissionRestorer::PermissionRestorer(const FilePath& path) + : path_(path), info_(NULL), length_(0) { + info_ = GetPermissionInfo(path_, &length_); + DCHECK(info_ != NULL); + DCHECK_NE(0u, length_); +} + +PermissionRestorer::~PermissionRestorer() { + if (!RestorePermissionInfo(path_, info_, length_)) + NOTREACHED(); +} + } // namespace file_util diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc index 5a4baf4..1e629e3 100644 --- a/chrome/browser/download/download_browsertest.cc +++ b/chrome/browser/download/download_browsertest.cc @@ -250,6 +250,7 @@ class DownloadTest : public InProcessBrowserTest { // Download interrupt reason (NONE is OK). content::DownloadInterruptReason reason; bool show_download_item; // True if the download item appears on the shelf. + bool should_redirect_to_documents; // True if we save it in "My Documents". }; DownloadTest() { @@ -641,6 +642,7 @@ class DownloadTest : public InProcessBrowserTest { } // Attempts to download a file, based on information in |download_info|. + // If a Select File dialog opens, will automatically choose the default. void DownloadFileCheckErrors(const DownloadInfo& download_info) { ASSERT_TRUE(test_server()->Start()); std::vector<DownloadItem*> download_items; @@ -654,6 +656,8 @@ class DownloadTest : public InProcessBrowserTest { GURL url = test_server()->GetURL(server_path); ASSERT_TRUE(url.is_valid()); + NullSelectFile(browser()); // Needed for read-only tests. + DownloadManager* download_manager = DownloadManagerForBrowser(browser()); scoped_ptr<DownloadTestObserver> observer( new DownloadTestObserverTerminal( @@ -717,9 +721,46 @@ class DownloadTest : public InProcessBrowserTest { ASSERT_EQ(url, item->GetOriginalUrl()); ASSERT_EQ(download_info.reason, item->GetLastReason()); + + if (item->GetState() == content::DownloadItem::COMPLETE) { + // Clean up the file, in case it ended up in the My Documents folder. + FilePath destination_folder = GetDownloadDirectory(browser()); + FilePath my_downloaded_file = item->GetTargetFilePath(); + EXPECT_TRUE(file_util::PathExists(my_downloaded_file)); + EXPECT_TRUE(file_util::Delete(my_downloaded_file, false)); + + EXPECT_EQ(download_info.should_redirect_to_documents ? + std::string::npos : + 0u, + my_downloaded_file.value().find(destination_folder.value())); + if (download_info.should_redirect_to_documents) { + // If it's not where we asked it to be, it should be in the + // My Documents folder. + FilePath my_docs_folder; + EXPECT_TRUE(PathService::Get(chrome::DIR_USER_DOCUMENTS, + &my_docs_folder)); + EXPECT_EQ(0u, + my_downloaded_file.value().find(my_docs_folder.value())); + } + } } } + // Attempts to download a file to a read-only folder, based on information + // in |download_info|. + void DownloadFileToReadonlyFolder(const DownloadInfo& download_info) { + ASSERT_TRUE(InitialSetup(false)); // Creates temporary download folder. + + // Make the test folder unwritable. + FilePath destination_folder = GetDownloadDirectory(browser()); + DVLOG(1) << " " << __FUNCTION__ << "()" + << " folder = '" << destination_folder.value() << "'"; + file_util::PermissionRestorer permission_restorer(destination_folder); + EXPECT_TRUE(file_util::MakeFileUnwritable(destination_folder)); + + DownloadFileCheckErrors(download_info); + } + bool EnsureNoPendingDownloads() { bool result = true; BrowserThread::PostTask( @@ -2028,7 +2069,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadNavigate) { "a_zip_file.zip", DOWNLOAD_NAVIGATE, content::DOWNLOAD_INTERRUPT_REASON_NONE, - true + true, + false }; // Do initial setup. @@ -2041,7 +2083,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadDirect) { "a_zip_file.zip", DOWNLOAD_DIRECT, content::DOWNLOAD_INTERRUPT_REASON_NONE, - true + true, + false }; // Do initial setup. @@ -2054,7 +2097,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadError404Direct) { "there_IS_no_spoon.zip", DOWNLOAD_DIRECT, content::DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT, - true + true, + false }; // Do initial setup. @@ -2067,6 +2111,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadError404Navigate) { "there_IS_no_spoon.zip", DOWNLOAD_NAVIGATE, content::DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT, + false, false }; @@ -2080,7 +2125,8 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadError400Direct) { "zip_file_not_found.zip", DOWNLOAD_DIRECT, content::DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED, - true + true, + false }; // Do initial setup. @@ -2093,6 +2139,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadError400Navigate) { "zip_file_not_found.zip", DOWNLOAD_NAVIGATE, content::DOWNLOAD_INTERRUPT_REASON_SERVER_FAILED, + false, false }; @@ -2100,3 +2147,29 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadError400Navigate) { ASSERT_TRUE(InitialSetup(false)); DownloadFileCheckErrors(download_info); } + +IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorReadonlyFolderDirect) { + DownloadInfo download_info = { + "a_zip_file.zip", + DOWNLOAD_DIRECT, + // This passes because we switch to the My Documents folder. + content::DOWNLOAD_INTERRUPT_REASON_NONE, + true, + true + }; + + DownloadFileToReadonlyFolder(download_info); +} + +IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadErrorReadonlyFolderNavigate) { + DownloadInfo download_info = { + "a_zip_file.zip", + DOWNLOAD_NAVIGATE, + // This passes because we switch to the My Documents folder. + content::DOWNLOAD_INTERRUPT_REASON_NONE, + true, + true + }; + + DownloadFileToReadonlyFolder(download_info); +} |