-
Notifications
You must be signed in to change notification settings - Fork 15.1k
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 (#36224)
* chore: cherry-pick 933cc81c6bad from chromium * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: electron-patch-conflict-fixer[bot] <83340002+electron-patch-conflict-fixer[bot]@users.noreply.github.com> Co-authored-by: John Kleinschmidt <jkleinsc@electronjs.org>
- Loading branch information
1 parent
aab45e6
commit 87b0a0d
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 ced9bfd8fe905acbb6ab5c3e52a9882fc23a7303..f7519189638ece437c4285ddd490be3ea59e638d 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 d8e63b56c8ac89a08cd7c40cabb156eb4d1923aa..c70d088bfd007e9a6cd9cfcb6f92b07b99653048 100644 | ||
--- a/content/test/content_browser_test_utils_internal.cc | ||
+++ b/content/test/content_browser_test_utils_internal.cc | ||
@@ -446,9 +446,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; | ||
|
||
@@ -456,16 +461,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 73be6e8a2f458128b08aae34d951c7a80139997d..43c6aabc5414d0cc12fb5a03142e8b20e2a7fab3 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 /> |