diff options
author | skerner@chromium.org <skerner@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-07-30 15:33:28 +0000 |
---|---|---|
committer | skerner@chromium.org <skerner@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-07-30 15:33:28 +0000 |
commit | bd781d897ea507f9abc028462d04224ca273875a (patch) | |
tree | bf14c6237ec30d3e1815e3978807d1943ad22f9f | |
parent | 5021691cce9e56fe0bb778df3ad92e41c55c6ed0 (diff) | |
download | chromium_src-bd781d897ea507f9abc028462d04224ca273875a.zip chromium_src-bd781d897ea507f9abc028462d04224ca273875a.tar.gz chromium_src-bd781d897ea507f9abc028462d04224ca273875a.tar.bz2 |
Remove some debug switches:
--issue35198-crxdir-browser
--issue35198-permission
They were used to understand issue 35198. They are no longer needed. There is one more switch, --issue35198-logging, that is not being removed in this CL. The logging it adds may still be useful.
BUG=50604
TEST=manual
Review URL: http://codereview.chromium.org/3052023
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54323 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | base/file_util.h | 5 | ||||
-rw-r--r-- | base/file_util_posix.cc | 8 | ||||
-rw-r--r-- | base/file_util_unittest.cc | 1 | ||||
-rw-r--r-- | base/file_util_win.cc | 71 | ||||
-rw-r--r-- | base/scoped_temp_dir.cc | 9 | ||||
-rw-r--r-- | base/scoped_temp_dir.h | 3 | ||||
-rw-r--r-- | base/scoped_temp_dir_unittest.cc | 2 | ||||
-rw-r--r-- | chrome/browser/extensions/convert_user_script.cc | 2 | ||||
-rw-r--r-- | chrome/browser/extensions/sandboxed_extension_unpacker.cc | 21 | ||||
-rw-r--r-- | chrome/common/chrome_switches.cc | 7 | ||||
-rw-r--r-- | chrome/common/chrome_switches.h | 2 |
11 files changed, 10 insertions, 121 deletions
diff --git a/base/file_util.h b/base/file_util.h index 66e450a..bd973b23 100644 --- a/base/file_util.h +++ b/base/file_util.h @@ -273,13 +273,8 @@ bool CreateNewTempDirectory(const FilePath::StringType& prefix, // Create a directory within another directory. // Extra characters will be appended to |prefix| to ensure that the // new directory does not have the same name as an existing directory. -// If |loosen_permissions| is true, the new directory will be readable -// and writable to all users on windows. It is ignored on other platforms. -// |loosen_permissions| exists to allow debugging of crbug/35198, and will -// be removed when the issue is understood. bool CreateTemporaryDirInDir(const FilePath& base_dir, const FilePath::StringType& prefix, - bool loosen_permissions, FilePath* new_dir); // Creates a directory, as well as creating any parent directories, if they diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc index 4d9cf42..13798d3 100644 --- a/base/file_util_posix.cc +++ b/base/file_util_posix.cc @@ -430,15 +430,7 @@ static bool CreateTemporaryDirInDirImpl(const FilePath& base_dir, bool CreateTemporaryDirInDir(const FilePath& base_dir, const FilePath::StringType& prefix, - bool loosen_permissions, FilePath* new_dir) { - // To understand crbug/35198, the ability to call this - // this function on windows while giving loose permissions - // to the resulting directory has been temporarily added. - // It should not be possible to call this function with - // loosen_permissions == true on non-windows platforms. - DCHECK(!loosen_permissions); - FilePath::StringType mkdtemp_template = prefix; mkdtemp_template.append(FILE_PATH_LITERAL("XXXXXX")); return CreateTemporaryDirInDirImpl(base_dir, mkdtemp_template, new_dir); diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc index 7860111..4a1417a 100644 --- a/base/file_util_unittest.cc +++ b/base/file_util_unittest.cc @@ -1577,7 +1577,6 @@ TEST_F(FileUtilTest, CreateNewTemporaryDirInDirTest) { ASSERT_TRUE(file_util::CreateTemporaryDirInDir( test_dir_, FILE_PATH_LITERAL("CreateNewTemporaryDirInDirTest"), - false, &new_dir)); EXPECT_TRUE(file_util::PathExists(new_dir)); EXPECT_TRUE(test_dir_.IsParent(new_dir)); diff --git a/base/file_util_win.cc b/base/file_util_win.cc index 26955f2..b6a741b 100644 --- a/base/file_util_win.cc +++ b/base/file_util_win.cc @@ -64,56 +64,6 @@ bool DevicePathToDriveLetterPath(const FilePath& device_path, return true; } -// Build a security descriptor with the weakest possible file permissions. -bool InitLooseSecurityDescriptor(SECURITY_ATTRIBUTES *sa, - SECURITY_DESCRIPTOR *sd) { - DWORD last_error; - - if (!InitializeSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION)) { - last_error = GetLastError(); - LOG(ERROR) << "InitializeSecurityDescriptor failed: GetLastError() = " - << last_error; - return false; - } - - if (!SetSecurityDescriptorDacl(sd, - TRUE, // bDaclPresent: Add one to |sd|. - NULL, // pDacl: NULL means allow all access. - FALSE // bDaclDefaulted: Not defaulted. - )) { - last_error = GetLastError(); - LOG(ERROR) << "SetSecurityDescriptorDacl() failed: GetLastError() = " - << last_error; - return false; - } - - if (!SetSecurityDescriptorGroup(sd, - NULL, // pGroup: No no primary group. - FALSE // bGroupDefaulted: Not defaulted. - )) { - last_error = GetLastError(); - LOG(ERROR) << "SetSecurityDescriptorGroup() failed: GetLastError() = " - << last_error; - return false; - } - - if (!SetSecurityDescriptorSacl(sd, - FALSE, // bSaclPresent: No SACL. - NULL, - FALSE - )) { - last_error = GetLastError(); - LOG(ERROR) << "SetSecurityDescriptorSacl() failed: GetLastError() = " - << last_error; - return false; - } - - sa->nLength = sizeof(SECURITY_ATTRIBUTES); - sa->lpSecurityDescriptor = sd; - sa->bInheritHandle = TRUE; - return true; -} - } // namespace std::wstring GetDirectoryFromPath(const std::wstring& path) { @@ -601,19 +551,7 @@ bool CreateTemporaryFileInDir(const FilePath& dir, bool CreateTemporaryDirInDir(const FilePath& base_dir, const FilePath::StringType& prefix, - bool loosen_permissions, FilePath* new_dir) { - SECURITY_ATTRIBUTES sa; - SECURITY_DESCRIPTOR sd; - - LPSECURITY_ATTRIBUTES directory_security_attributes = NULL; - if (loosen_permissions) { - if (InitLooseSecurityDescriptor(&sa, &sd)) - directory_security_attributes = &sa; - else - LOG(ERROR) << "Failed to init security attributes, fall back to NULL."; - } - FilePath path_to_create; srand(static_cast<uint32>(time(NULL))); @@ -628,8 +566,7 @@ bool CreateTemporaryDirInDir(const FilePath& base_dir, new_dir_name.append(IntToWString(rand() % kint16max)); path_to_create = path_to_create.Append(new_dir_name); - if (::CreateDirectory(path_to_create.value().c_str(), - directory_security_attributes)) + if (::CreateDirectory(path_to_create.value().c_str(), NULL)) break; count++; } @@ -639,7 +576,6 @@ bool CreateTemporaryDirInDir(const FilePath& base_dir, } *new_dir = path_to_create; - return true; } @@ -649,10 +585,7 @@ bool CreateNewTempDirectory(const FilePath::StringType& prefix, if (!GetTempDir(&system_temp_dir)) return false; - return CreateTemporaryDirInDir(system_temp_dir, - prefix, - false, - new_temp_path); + return CreateTemporaryDirInDir(system_temp_dir, prefix, new_temp_path); } bool CreateDirectory(const FilePath& full_path) { diff --git a/base/scoped_temp_dir.cc b/base/scoped_temp_dir.cc index 28f59bf..53452f1 100644 --- a/base/scoped_temp_dir.cc +++ b/base/scoped_temp_dir.cc @@ -4,7 +4,6 @@ #include "base/scoped_temp_dir.h" -#include "base/command_line.h" #include "base/file_util.h" #include "base/logging.h" #include "base/string_util.h" @@ -27,19 +26,15 @@ bool ScopedTempDir::CreateUniqueTempDir() { return true; } -bool ScopedTempDir::CreateUniqueTempDirUnderPath(const FilePath& base_path, - bool loose_permissions) { +bool ScopedTempDir::CreateUniqueTempDirUnderPath(const FilePath& base_path) { // If |path| does not exist, create it. - if (!file_util::CreateDirectory(base_path)) { - LOG(ERROR) << "Failed to create base directory " << base_path.value(); + if (!file_util::CreateDirectory(base_path)) return false; - } // Create a new, uniquely named directory under |base_path|. if (!file_util::CreateTemporaryDirInDir( base_path, FILE_PATH_LITERAL("scoped_dir_"), - loose_permissions, &path_)) { return false; } diff --git a/base/scoped_temp_dir.h b/base/scoped_temp_dir.h index c24891d..702c2bc 100644 --- a/base/scoped_temp_dir.h +++ b/base/scoped_temp_dir.h @@ -27,8 +27,7 @@ class ScopedTempDir { bool CreateUniqueTempDir(); // Creates a unique directory under a given path, and takes ownership of it. - bool CreateUniqueTempDirUnderPath(const FilePath& path, - bool loose_permissions); + bool CreateUniqueTempDirUnderPath(const FilePath& path); // Takes ownership of directory at |path|, creating it if necessary. // Don't call multiple times unless Take() has been called first. diff --git a/base/scoped_temp_dir_unittest.cc b/base/scoped_temp_dir_unittest.cc index e180119..4be0d07 100644 --- a/base/scoped_temp_dir_unittest.cc +++ b/base/scoped_temp_dir_unittest.cc @@ -65,7 +65,7 @@ TEST(ScopedTempDir, UniqueTempDirUnderPath) { FilePath test_path; { ScopedTempDir dir; - EXPECT_TRUE(dir.CreateUniqueTempDirUnderPath(base_path, false)); + EXPECT_TRUE(dir.CreateUniqueTempDirUnderPath(base_path)); test_path = dir.path(); EXPECT_TRUE(file_util::DirectoryExists(test_path)); EXPECT_TRUE(base_path.IsParent(test_path)); diff --git a/chrome/browser/extensions/convert_user_script.cc b/chrome/browser/extensions/convert_user_script.cc index b06553a..d50bec1 100644 --- a/chrome/browser/extensions/convert_user_script.cc +++ b/chrome/browser/extensions/convert_user_script.cc @@ -45,7 +45,7 @@ Extension* ConvertUserScriptToExtension(const FilePath& user_script_path, CHECK(PathService::Get(chrome::DIR_USER_DATA_TEMP, &user_data_temp_dir)); ScopedTempDir temp_dir; - if (!temp_dir.CreateUniqueTempDirUnderPath(user_data_temp_dir, false)) { + if (!temp_dir.CreateUniqueTempDirUnderPath(user_data_temp_dir)) { *error = "Could not create temporary directory."; return NULL; } diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc index c087649..2de1c09 100644 --- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc +++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc @@ -42,18 +42,8 @@ void SandboxedExtensionUnpacker::Start() { // file IO on. CHECK(ChromeThread::GetCurrentThreadIdentifier(&thread_identifier_)); - // To understand crbug/35198, allow users who can reproduce the bug - // to loosen permissions on the scoped directory. - bool loosen_permissions = false; -#if defined (OS_WIN) - loosen_permissions = CommandLine::ForCurrentProcess()->HasSwitch( - switches::kIssue35198Permission); - LOG(INFO) << "loosen_permissions = " << loosen_permissions; -#endif - // Create a temporary directory to work in. - if (!temp_dir_.CreateUniqueTempDirUnderPath(temp_path_, - loosen_permissions)) { + if (!temp_dir_.CreateUniqueTempDirUnderPath(temp_path_)) { ReportFailure("Could not create temporary directory."); return; } @@ -62,15 +52,6 @@ void SandboxedExtensionUnpacker::Start() { extension_root_ = temp_dir_.path().AppendASCII( extension_filenames::kTempExtensionName); - // To understand crbug/35198, allow users who can reproduce the bug to - // create the unpack directory in the browser process. - bool crxdir_in_browser = CommandLine::ForCurrentProcess()->HasSwitch( - switches::kIssue35198CrxDirBrowser); - LOG(INFO) << "crxdir_in_browser = " << crxdir_in_browser; - if (crxdir_in_browser && !file_util::CreateDirectory(extension_root_)) { - LOG(ERROR) << "Failed to create directory " << extension_root_.value(); - } - // Extract the public key and validate the package. if (!ValidateSignature()) return; // ValidateSignature() already reported the error. diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc index 6016cda..d31eb6d 100644 --- a/chrome/common/chrome_switches.cc +++ b/chrome/common/chrome_switches.cc @@ -566,12 +566,9 @@ const char kInternalNaCl[] = "internal-nacl"; // Runs a trusted Pepper plugin inside the renderer process. const char kInternalPepper[] = "internal-pepper"; -// The following flags allow users who can reproduce crbug/35198 -// to enable extra logging and behaviors. They will be removed once -// the issue is fixed. -const char kIssue35198CrxDirBrowser[] = "issue35198-crxdir-browser"; +// The following flag allows users who can reproduce crbug/35198 +// to enable extra logging. It will be removed once the issue is fixed. const char kIssue35198ExtraLogging[] = "issue35198-logging"; -const char kIssue35198Permission[] = "issue35198-permission"; // Specifies the flags passed to JS engine const char kJavaScriptFlags[] = "js-flags"; diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h index e822cb5..3d42124 100644 --- a/chrome/common/chrome_switches.h +++ b/chrome/common/chrome_switches.h @@ -171,9 +171,7 @@ extern const char kInstallerTestClean[]; extern const char kInstallerTestForce[]; extern const char kInternalNaCl[]; extern const char kInternalPepper[]; -extern const char kIssue35198CrxDirBrowser[]; extern const char kIssue35198ExtraLogging[]; -extern const char kIssue35198Permission[]; extern const char kJavaScriptFlags[]; extern const char kKeepAliveForTest[]; extern const char kLoadExtension[]; |