Commit 9b246de6 authored by anthonyvd's avatar anthonyvd Committed by Chromium LUCI CQ

Fix ClearBrowsingDataJob remote command on Windows

Because paths are case-insensitive on Windows, the DM server
normalizes profile paths to lower-case when they are reported.
This breaks the Clear Browsing Data remote command because it uses
that path to find the profile into which it'll clear the data.

This CL makes it so the path is compared in a case-insensitive manner
on Windows in order to find the correct profile.

Bug: 1159450
Change-Id: I07ebb206049ee848e1aae9e2059c3c69a4b367d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2595728
Commit-Queue: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: default avatarNicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837754}
parent ef221127
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/values.h" #include "base/values.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h" #include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h"
#include "chrome/browser/profiles/profile_attributes_storage.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
namespace enterprise_commands { namespace enterprise_commands {
...@@ -77,6 +78,27 @@ bool ClearBrowsingDataJob::ParseCommandPayload( ...@@ -77,6 +78,27 @@ bool ClearBrowsingDataJob::ParseCommandPayload(
// path from UTF8, and ending up with an invalid path will fail later in // path from UTF8, and ending up with an invalid path will fail later in
// RunImpl when we attempt to get the profile from the path. // RunImpl when we attempt to get the profile from the path.
profile_path_ = base::FilePath::FromUTF8Unsafe(*path); profile_path_ = base::FilePath::FromUTF8Unsafe(*path);
#if defined(OS_WIN)
// For Windows machines, the path that Chrome reports for the profile is
// "Normalized" to all lower-case on the reporting server. This means that
// when the server sends the command, the path will be all lower case and
// the profile manager won't be able to use it as a key. To avoid this issue,
// This code will iterate over all profile paths and find the one that matches
// in a case-insensitive comparison. If this doesn't find one, RunImpl will
// fail in the same manner as if the profile didn't exist, which is the
// expected behavior.
ProfileAttributesStorage& storage =
profile_manager_->GetProfileAttributesStorage();
for (ProfileAttributesEntry* entry : storage.GetAllProfilesAttributes()) {
base::FilePath entry_path = entry->GetPath();
if (base::FilePath::CompareEqualIgnoreCase(profile_path_.value(),
entry_path.value())) {
profile_path_ = entry_path;
break;
}
}
#endif
// Not specifying these fields is equivalent to setting them to false. // Not specifying these fields is equivalent to setting them to false.
clear_cache_ = root->FindBoolKey(kClearCacheField).value_or(false); clear_cache_ = root->FindBoolKey(kClearCacheField).value_or(false);
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/bind.h" #include "base/test/bind.h"
#include "base/values.h" #include "base/values.h"
#include "build/build_config.h"
#include "chrome/browser/nacl_host/nacl_browser_delegate_impl.h" #include "chrome/browser/nacl_host/nacl_browser_delegate_impl.h"
#include "chrome/test/base/testing_browser_process.h" #include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
...@@ -20,10 +21,11 @@ ...@@ -20,10 +21,11 @@
namespace enterprise_commands { namespace enterprise_commands {
namespace { namespace {
const char kProfileName[] = "test"; const char kProfileName[] = "Test";
const char kProfileNameLowerCase[] = "test";
const policy::RemoteCommandJob::UniqueIDType kUniqueID = 123456789; const policy::RemoteCommandJob::UniqueIDType kUniqueID = 123456789;
base::FilePath::StringType kTestProfilePath = base::FilePath::StringType kTestProfilePath =
FILE_PATH_LITERAL("/path/to/profile"); FILE_PATH_LITERAL("/Path/To/Profile");
const char kProfilePathField[] = "profile_path"; const char kProfilePathField[] = "profile_path";
const char kClearCacheField[] = "clear_cache"; const char kClearCacheField[] = "clear_cache";
...@@ -106,6 +108,10 @@ class ClearBrowsingDataJobTest : public ::testing::Test { ...@@ -106,6 +108,10 @@ class ClearBrowsingDataJobTest : public ::testing::Test {
return profile_manager_->profiles_dir().AppendASCII(kProfileName); return profile_manager_->profiles_dir().AppendASCII(kProfileName);
} }
base::FilePath GetTestProfilePathLowerCase() {
return profile_manager_->profiles_dir().AppendASCII(kProfileNameLowerCase);
}
private: private:
std::unique_ptr<content::BrowserTaskEnvironment> task_environment_; std::unique_ptr<content::BrowserTaskEnvironment> task_environment_;
std::unique_ptr<TestingProfileManager> profile_manager_; std::unique_ptr<TestingProfileManager> profile_manager_;
...@@ -281,4 +287,41 @@ TEST_F(ClearBrowsingDataJobTest, SuccessClearNeither) { ...@@ -281,4 +287,41 @@ TEST_F(ClearBrowsingDataJobTest, SuccessClearNeither) {
EXPECT_TRUE(done); EXPECT_TRUE(done);
} }
// For Windows machines, the path that Chrome reports for the profile is
// "Normalized" to all lower-case on the reporting server. This means that
// when the server sends the command, the path will be all lower case and
// the profile manager won't be able to use it as a key.
// Because of that, the code for this job is slightly different on Windows,
// and this test verifies that behavior.
TEST_F(ClearBrowsingDataJobTest, HandleProfilPathCaseSensitivity) {
base::RunLoop run_loop;
AddTestingProfile();
auto job = CreateJob(CreateCommandProto(GetTestProfilePathLowerCase().value(),
/* clear_cache= */ false,
/* clear_cookies= */ true),
profile_manager());
bool done = false;
#if defined(OS_WIN)
// On windows, paths are case-insensitive so passing a lowercase path should
// still result in success.
auto expected = policy::RemoteCommandJob::SUCCEEDED;
#else
// On other platforms, paths are case-sensitive, so passing a lower case path
// will result in the profile not being found.
auto expected = policy::RemoteCommandJob::FAILED;
#endif // OS_WIN
EXPECT_TRUE(job->Run(base::Time::Now(), base::TimeTicks::Now(),
base::BindLambdaForTesting([&] {
EXPECT_EQ(expected, job->status());
done = true;
run_loop.Quit();
})));
run_loop.Run();
EXPECT_TRUE(done);
}
} // namespace enterprise_commands } // namespace enterprise_commands
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment