Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick 933cc81c6bad from chromium (#36222)
* chore: cherry-pick 933cc81c6bad from chromium * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
- Loading branch information
1 parent
c678f7b
commit c5346bd
Showing
2 changed files
with
324 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,323 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Xiaocheng Hu <xiaochengh@chromium.org> | ||
Date: Sat, 10 Sep 2022 05:53:49 +0000 | ||
Subject: Remove symlinks from FileChooserImpl folder upload result | ||
|
||
FileChooserImpl is the browser-side implementation of | ||
<input type=file>. When uploading a whole folder, it | ||
currently uses DirectoryLister to list all the files in a | ||
directory. The result also includes resolved symbolic links | ||
(which may even hide deep in some subfolder), which is not a | ||
desired behavior. | ||
|
||
Therefore, this patch removes all symbolic links from the | ||
result by checking each file against `base::IsLink()`. Since | ||
the function needs blocking calls to access file data, the | ||
job is sent to a worker pool thread. | ||
|
||
Fixed: 1345275 | ||
Change-Id: I8ab58214c87944408c64b177e915247a7485925b | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3866767 | ||
Reviewed-by: Austin Sullivan <asully@chromium.org> | ||
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> | ||
Reviewed-by: Mason Freed <masonf@chromium.org> | ||
Reviewed-by: Alex Moshchuk <alexmos@chromium.org> | ||
Cr-Commit-Position: refs/heads/main@{#1045491} | ||
|
||
diff --git a/content/browser/web_contents/file_chooser_impl.cc b/content/browser/web_contents/file_chooser_impl.cc | ||
index 7a3ea45d32c97980c141662f6a071cc517a15ad8..1aa19f7a735b444f2c33d5084edcdd14e3c2f5c5 100644 | ||
--- a/content/browser/web_contents/file_chooser_impl.cc | ||
+++ b/content/browser/web_contents/file_chooser_impl.cc | ||
@@ -4,8 +4,11 @@ | ||
|
||
#include "content/browser/web_contents/file_chooser_impl.h" | ||
|
||
+#include "base/files/file_util.h" | ||
#include "base/logging.h" | ||
#include "base/memory/ptr_util.h" | ||
+#include "base/ranges/algorithm.h" | ||
+#include "base/task/thread_pool.h" | ||
#include "content/browser/child_process_security_policy_impl.h" | ||
#include "content/browser/renderer_host/back_forward_cache_disable.h" | ||
#include "content/browser/renderer_host/render_frame_host_delegate.h" | ||
@@ -18,6 +21,19 @@ | ||
|
||
namespace content { | ||
|
||
+namespace { | ||
+ | ||
+std::vector<blink::mojom::FileChooserFileInfoPtr> RemoveSymlinks( | ||
+ std::vector<blink::mojom::FileChooserFileInfoPtr> files) { | ||
+ auto new_end = base::ranges::remove_if( | ||
+ files, &base::IsLink, | ||
+ [](const auto& file) { return file->get_native_file()->file_path; }); | ||
+ files.erase(new_end, files.end()); | ||
+ return files; | ||
+} | ||
+ | ||
+} // namespace | ||
+ | ||
FileChooserImpl::FileSelectListenerImpl::~FileSelectListenerImpl() { | ||
#if DCHECK_IS_ON() | ||
if (!was_file_select_listener_function_called_) { | ||
@@ -51,8 +67,20 @@ void FileChooserImpl::FileSelectListenerImpl::FileSelected( | ||
"FileSelectListener::FileSelectionCanceled()"; | ||
was_file_select_listener_function_called_ = true; | ||
#endif | ||
- if (owner_) | ||
- owner_->FileSelected(std::move(files), base_dir, mode); | ||
+ if (!owner_) | ||
+ return; | ||
+ | ||
+ if (mode != blink::mojom::FileChooserParams::Mode::kUploadFolder) { | ||
+ owner_->FileSelected(base_dir, mode, std::move(files)); | ||
+ return; | ||
+ } | ||
+ | ||
+ base::ThreadPool::PostTaskAndReplyWithResult( | ||
+ FROM_HERE, | ||
+ {base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}, | ||
+ base::BindOnce(&RemoveSymlinks, std::move(files)), | ||
+ base::BindOnce(&FileChooserImpl::FileSelected, owner_->GetWeakPtr(), | ||
+ base_dir, mode)); | ||
} | ||
|
||
void FileChooserImpl::FileSelectListenerImpl::FileSelectionCanceled() { | ||
@@ -162,9 +190,9 @@ void FileChooserImpl::EnumerateChosenDirectory( | ||
} | ||
|
||
void FileChooserImpl::FileSelected( | ||
- std::vector<blink::mojom::FileChooserFileInfoPtr> files, | ||
const base::FilePath& base_dir, | ||
- blink::mojom::FileChooserParams::Mode mode) { | ||
+ blink::mojom::FileChooserParams::Mode mode, | ||
+ std::vector<blink::mojom::FileChooserFileInfoPtr> files) { | ||
listener_impl_ = nullptr; | ||
if (!render_frame_host_) { | ||
std::move(callback_).Run(nullptr); | ||
diff --git a/content/browser/web_contents/file_chooser_impl.h b/content/browser/web_contents/file_chooser_impl.h | ||
index b9f11f9e6a0b548cb5ab8ca721ae823e079ce6fa..b628b29a5f84264e62bb3fa9e92550787b8342de 100644 | ||
--- a/content/browser/web_contents/file_chooser_impl.h | ||
+++ b/content/browser/web_contents/file_chooser_impl.h | ||
@@ -37,6 +37,8 @@ class CONTENT_EXPORT FileChooserImpl : public blink::mojom::FileChooser, | ||
|
||
// FileSelectListener overrides: | ||
|
||
+ // TODO(xiaochengh): Move |file| to the end of the argument list to match | ||
+ // the argument ordering of FileChooserImpl::FileSelected(). | ||
void FileSelected(std::vector<blink::mojom::FileChooserFileInfoPtr> files, | ||
const base::FilePath& base_dir, | ||
blink::mojom::FileChooserParams::Mode mode) override; | ||
@@ -68,9 +70,9 @@ class CONTENT_EXPORT FileChooserImpl : public blink::mojom::FileChooser, | ||
|
||
~FileChooserImpl() override; | ||
|
||
- void FileSelected(std::vector<blink::mojom::FileChooserFileInfoPtr> files, | ||
- const base::FilePath& base_dir, | ||
- blink::mojom::FileChooserParams::Mode mode); | ||
+ void FileSelected(const base::FilePath& base_dir, | ||
+ blink::mojom::FileChooserParams::Mode mode, | ||
+ std::vector<blink::mojom::FileChooserFileInfoPtr> files); | ||
|
||
void FileSelectionCanceled(); | ||
|
||
@@ -82,6 +84,10 @@ class CONTENT_EXPORT FileChooserImpl : public blink::mojom::FileChooser, | ||
const base::FilePath& directory_path, | ||
EnumerateChosenDirectoryCallback callback) override; | ||
|
||
+ base::WeakPtr<FileChooserImpl> GetWeakPtr() { | ||
+ return weak_factory_.GetWeakPtr(); | ||
+ } | ||
+ | ||
private: | ||
explicit FileChooserImpl(RenderFrameHostImpl* render_frame_host); | ||
|
||
@@ -95,6 +101,8 @@ class CONTENT_EXPORT FileChooserImpl : public blink::mojom::FileChooser, | ||
raw_ptr<RenderFrameHostImpl> render_frame_host_; | ||
scoped_refptr<FileSelectListenerImpl> listener_impl_; | ||
base::OnceCallback<void(blink::mojom::FileChooserResultPtr)> callback_; | ||
+ | ||
+ base::WeakPtrFactory<FileChooserImpl> weak_factory_{this}; | ||
}; | ||
|
||
} // namespace content | ||
diff --git a/content/browser/web_contents/file_chooser_impl_browsertest.cc b/content/browser/web_contents/file_chooser_impl_browsertest.cc | ||
index 5aa120cd2e9fab1492f5d0993adbb2f80a4aa732..2acd2216331bd9be56eb9705f0e9c0d3bceb9e93 100644 | ||
--- a/content/browser/web_contents/file_chooser_impl_browsertest.cc | ||
+++ b/content/browser/web_contents/file_chooser_impl_browsertest.cc | ||
@@ -5,14 +5,18 @@ | ||
#include "content/browser/web_contents/file_chooser_impl.h" | ||
|
||
#include "base/bind.h" | ||
+#include "base/files/file_util.h" | ||
+#include "base/path_service.h" | ||
#include "base/run_loop.h" | ||
#include "content/browser/renderer_host/render_frame_host_impl.h" | ||
#include "content/public/browser/web_contents_delegate.h" | ||
+#include "content/public/common/content_paths.h" | ||
#include "content/public/test/browser_test.h" | ||
#include "content/public/test/browser_test_utils.h" | ||
#include "content/public/test/content_browser_test.h" | ||
#include "content/public/test/content_browser_test_utils.h" | ||
#include "content/shell/browser/shell.h" | ||
+#include "content/test/content_browser_test_utils_internal.h" | ||
#include "url/gurl.h" | ||
#include "url/url_constants.h" | ||
|
||
@@ -143,11 +147,52 @@ IN_PROC_BROWSER_TEST_F(FileChooserImplBrowserTest, | ||
->SetListenerFunctionCalledTrueForTesting(); | ||
std::vector<blink::mojom::FileChooserFileInfoPtr> files; | ||
files.emplace_back(blink::mojom::FileChooserFileInfoPtr(nullptr)); | ||
- chooser->FileSelected(std::move(files), base::FilePath(), | ||
- blink::mojom::FileChooserParams::Mode::kOpen); | ||
+ chooser->FileSelected(base::FilePath(), | ||
+ blink::mojom::FileChooserParams::Mode::kOpen, | ||
+ std::move(files)); | ||
|
||
// Test passes if this run_loop.Run() returns instead of timing out. | ||
run_loop.Run(); | ||
} | ||
|
||
+// https://crbug.com/1345275 | ||
+IN_PROC_BROWSER_TEST_F(FileChooserImplBrowserTest, UploadFolderWithSymlink) { | ||
+ EXPECT_TRUE(NavigateToURL( | ||
+ shell(), GetTestUrl(".", "file_input_webkitdirectory.html"))); | ||
+ | ||
+ // The folder contains a regular file and a symbolic link. | ||
+ // When uploading the folder, the symbolic link should be excluded. | ||
+ base::FilePath dir_test_data; | ||
+ ASSERT_TRUE(base::PathService::Get(DIR_TEST_DATA, &dir_test_data)); | ||
+ base::FilePath folder_to_upload = | ||
+ dir_test_data.AppendASCII("file_chooser").AppendASCII("dir_with_symlink"); | ||
+ | ||
+ base::FilePath text_file = folder_to_upload.AppendASCII("text_file.txt"); | ||
+ base::FilePath symlink_file = folder_to_upload.AppendASCII("symlink"); | ||
+ | ||
+ // Skip the test if symbolic links are not supported. | ||
+ { | ||
+ base::ScopedAllowBlockingForTesting allow_blocking; | ||
+ if (!base::IsLink(symlink_file)) | ||
+ return; | ||
+ } | ||
+ | ||
+ std::unique_ptr<FileChooserDelegate> delegate( | ||
+ new FileChooserDelegate({text_file, symlink_file}, base::OnceClosure())); | ||
+ shell()->web_contents()->SetDelegate(delegate.get()); | ||
+ EXPECT_TRUE(ExecJs(shell(), | ||
+ "(async () => {" | ||
+ " let listener = new Promise(" | ||
+ " resolve => fileinput.onchange = resolve);" | ||
+ " fileinput.click();" | ||
+ " await listener;" | ||
+ "})()")); | ||
+ | ||
+ EXPECT_EQ( | ||
+ 1, EvalJs(shell(), "document.getElementById('fileinput').files.length;")); | ||
+ EXPECT_EQ( | ||
+ "text_file.txt", | ||
+ EvalJs(shell(), "document.getElementById('fileinput').files[0].name;")); | ||
+} | ||
+ | ||
} // namespace content | ||
diff --git a/content/test/content_browser_test_utils_internal.cc b/content/test/content_browser_test_utils_internal.cc | ||
index 8183cb7e6547f42b27afc323fe136e2157e4dd03..dbdf244571956645c6494c4cdab514dd42dbb6c2 100644 | ||
--- a/content/test/content_browser_test_utils_internal.cc | ||
+++ b/content/test/content_browser_test_utils_internal.cc | ||
@@ -447,9 +447,14 @@ Shell* OpenPopup(const ToRenderFrameHost& opener, | ||
return new_shell_observer.GetShell(); | ||
} | ||
|
||
+FileChooserDelegate::FileChooserDelegate(std::vector<base::FilePath> files, | ||
+ base::OnceClosure callback) | ||
+ : files_(std::move(files)), callback_(std::move(callback)) {} | ||
+ | ||
FileChooserDelegate::FileChooserDelegate(const base::FilePath& file, | ||
base::OnceClosure callback) | ||
- : file_(file), callback_(std::move(callback)) {} | ||
+ : FileChooserDelegate(std::vector<base::FilePath>(1, file), | ||
+ std::move(callback)) {} | ||
|
||
FileChooserDelegate::~FileChooserDelegate() = default; | ||
|
||
@@ -457,16 +462,18 @@ void FileChooserDelegate::RunFileChooser( | ||
RenderFrameHost* render_frame_host, | ||
scoped_refptr<content::FileSelectListener> listener, | ||
const blink::mojom::FileChooserParams& params) { | ||
- // Send the selected file to the renderer process. | ||
- auto file_info = blink::mojom::FileChooserFileInfo::NewNativeFile( | ||
- blink::mojom::NativeFileInfo::New(file_, std::u16string())); | ||
+ // Send the selected files to the renderer process. | ||
std::vector<blink::mojom::FileChooserFileInfoPtr> files; | ||
- files.push_back(std::move(file_info)); | ||
- listener->FileSelected(std::move(files), base::FilePath(), | ||
- blink::mojom::FileChooserParams::Mode::kOpen); | ||
+ for (const auto& file : files_) { | ||
+ auto file_info = blink::mojom::FileChooserFileInfo::NewNativeFile( | ||
+ blink::mojom::NativeFileInfo::New(file, std::u16string())); | ||
+ files.push_back(std::move(file_info)); | ||
+ } | ||
+ listener->FileSelected(std::move(files), base::FilePath(), params.mode); | ||
|
||
params_ = params.Clone(); | ||
- std::move(callback_).Run(); | ||
+ if (callback_) | ||
+ std::move(callback_).Run(); | ||
} | ||
|
||
FrameTestNavigationManager::FrameTestNavigationManager( | ||
diff --git a/content/test/content_browser_test_utils_internal.h b/content/test/content_browser_test_utils_internal.h | ||
index 6ae5e8f81a189702d5ed9bef003696f34c525dd3..3bf1bb93e15b0a5270262837802e140ad72a9231 100644 | ||
--- a/content/test/content_browser_test_utils_internal.h | ||
+++ b/content/test/content_browser_test_utils_internal.h | ||
@@ -176,9 +176,11 @@ Shell* OpenPopup(const ToRenderFrameHost& opener, | ||
class FileChooserDelegate : public WebContentsDelegate { | ||
public: | ||
// Constructs a WebContentsDelegate that mocks a file dialog. | ||
- // The mocked file dialog will always reply that the user selected |file|. | ||
- // |callback| is invoked when RunFileChooser() is called. | ||
+ // The mocked file dialog will always reply that the user selected |file| or | ||
+ // |files|. |callback| is invoked when RunFileChooser() is called. | ||
FileChooserDelegate(const base::FilePath& file, base::OnceClosure callback); | ||
+ FileChooserDelegate(std::vector<base::FilePath> files, | ||
+ base::OnceClosure callback); | ||
~FileChooserDelegate() override; | ||
|
||
// Implementation of WebContentsDelegate::RunFileChooser. | ||
@@ -190,7 +192,7 @@ class FileChooserDelegate : public WebContentsDelegate { | ||
const blink::mojom::FileChooserParams& params() const { return *params_; } | ||
|
||
private: | ||
- base::FilePath file_; | ||
+ std::vector<base::FilePath> files_; | ||
base::OnceClosure callback_; | ||
blink::mojom::FileChooserParamsPtr params_; | ||
}; | ||
diff --git a/content/test/data/file_chooser/dir_with_symlink/symlink b/content/test/data/file_chooser/dir_with_symlink/symlink | ||
new file mode 120000 | ||
index 0000000000000000000000000000000000000000..7857c689f7043265b4e6d4dcdf6d40d0be2d3d60 | ||
--- /dev/null | ||
+++ b/content/test/data/file_chooser/dir_with_symlink/symlink | ||
@@ -0,0 +1 @@ | ||
+../linked_text_file.txt | ||
\ No newline at end of file | ||
diff --git a/content/test/data/file_chooser/dir_with_symlink/text_file.txt b/content/test/data/file_chooser/dir_with_symlink/text_file.txt | ||
new file mode 100644 | ||
index 0000000000000000000000000000000000000000..8e27be7d6154a1f68ea9160ef0e18691d20560dc | ||
--- /dev/null | ||
+++ b/content/test/data/file_chooser/dir_with_symlink/text_file.txt | ||
@@ -0,0 +1 @@ | ||
+text | ||
diff --git a/content/test/data/file_chooser/linked_text_file.txt b/content/test/data/file_chooser/linked_text_file.txt | ||
new file mode 100644 | ||
index 0000000000000000000000000000000000000000..9a1f4bc60917c014eac1464ad664a0271c288b84 | ||
--- /dev/null | ||
+++ b/content/test/data/file_chooser/linked_text_file.txt | ||
@@ -0,0 +1 @@ | ||
+linked text file | ||
diff --git a/content/test/data/file_input_webkitdirectory.html b/content/test/data/file_input_webkitdirectory.html | ||
new file mode 100644 | ||
index 0000000000000000000000000000000000000000..5b7bb501f7eb5d9f28751f36380e4ad01d2da0c7 | ||
--- /dev/null | ||
+++ b/content/test/data/file_input_webkitdirectory.html | ||
@@ -0,0 +1 @@ | ||
+<input type="file" id="fileinput" webkitdirectory /> |