Commit 5fde5d40 authored by James Cook's avatar James Cook Committed by Chromium LUCI CQ

lacros: Allow downloading files into the MyFiles documents directory

Make the path sanitization code in DownloadPrefs more closely match
the version used by ash-chrome (existing chrome for chromeos).

There's still more refactoring work to do to make the behavior
match exactly, but this lets users have a bit more flexibility
with their download location.

Bug: 1148848
Test: added to unit_tests
Change-Id: I54048b60db3191f49187bf7645384f4d2a95218d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2566360Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832485}
parent 657a30ba
......@@ -556,7 +556,6 @@ base::FilePath DownloadPrefs::SanitizeDownloadTargetPath(
#if BUILDFLAG(IS_CHROMEOS_LACROS)
// TODO(https://crbug.com/1148848): Sort out path sanitization for Lacros.
// This will require refactoring the ash-only code below so it can be shared.
// For now, only allow downloads into the Downloads directory and children.
const base::FilePath default_downloads_path =
GetDefaultDownloadDirectoryForProfile();
// Relative paths might be unsafe, so use the default path.
......@@ -569,6 +568,12 @@ base::FilePath DownloadPrefs::SanitizeDownloadTargetPath(
if (default_downloads_path == path || default_downloads_path.IsParent(path))
return path;
// Allow documents directory ("MyFiles") and subdirectories.
base::FilePath documents_path =
base::PathService::CheckedGet(chrome::DIR_USER_DOCUMENTS);
if (documents_path == path || documents_path.IsParent(path))
return path;
// Otherwise, return the safe default.
return default_downloads_path;
#elif BUILDFLAG(IS_CHROMEOS_ASH)
......
......@@ -26,6 +26,11 @@
#include "components/drive/drive_pref_names.h"
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(IS_CHROMEOS_LACROS)
#include "base/path_service.h"
#include "chrome/common/chrome_paths.h"
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)
using safe_browsing::FileTypePolicies;
namespace {
......@@ -381,9 +386,19 @@ TEST(DownloadPrefsTest, DownloadDirSanitization) {
const base::FilePath default_dir =
prefs.GetDefaultDownloadDirectoryForProfile();
// Test a valid path.
// Test a valid subdirectory of downloads.
ExpectValidDownloadDir(&profile, &prefs, default_dir.AppendASCII("testdir"));
#if BUILDFLAG(IS_CHROMEOS_LACROS)
// Test a valid subdirectory of documents. This isn't tested for ash because
// these tests run on the linux "emulator", where ash uses ~/Documents, but
// the ash path sanitization code doesn't handle that path.
base::FilePath documents_path =
base::PathService::CheckedGet(chrome::DIR_USER_DOCUMENTS);
ExpectValidDownloadDir(&profile, &prefs,
documents_path.AppendASCII("testdir"));
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)
// Test with an invalid path outside the download directory.
profile.GetPrefs()->SetString(prefs::kDownloadDefaultDirectory,
"/home/chronos");
......
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