From cb206b8a9fa8e2a020577007d1be7026f6cfb6b5 Mon Sep 17 00:00:00 2001 From: Pedro Pontes Date: Tue, 26 Oct 2021 10:45:28 +0200 Subject: [PATCH] chore: cherry-pick 0894af410c4e from chromium (#31544) * chore: cherry-pick 0894af410c4e from chromium * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: Electron Bot --- patches/chromium/.patches | 1 + .../chromium/cherry-pick-0894af410c4e.patch | 385 ++++++++++++++++++ 2 files changed, 386 insertions(+) create mode 100644 patches/chromium/cherry-pick-0894af410c4e.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 1fc3e7511e294..2d593ec8f1619 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -115,5 +115,6 @@ check_direction_of_rtcencodedframes.patch mas_gate_private_enterprise_APIs cherry-pick-c69dddfe1cde.patch speculative_fix_for_eye_dropper_getcolor_crash.patch +cherry-pick-0894af410c4e.patch move_networkstateobserver_from_document_to_window.patch cherry-pick-8af66de55aad.patch diff --git a/patches/chromium/cherry-pick-0894af410c4e.patch b/patches/chromium/cherry-pick-0894af410c4e.patch new file mode 100644 index 0000000000000..d9c59df238750 --- /dev/null +++ b/patches/chromium/cherry-pick-0894af410c4e.patch @@ -0,0 +1,385 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Min Qin +Date: Tue, 31 Aug 2021 23:03:03 +0000 +Subject: Quarantine save package items that's downloaded from network + +Currently quarantine is not performed for save page downloads. This CL +fixes the issue. + +BUG=1243020, 811161 + +Change-Id: I85d03cc324b0b90a45bd8b3429e4e9eec1aaf857 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3126709 +Reviewed-by: Xing Liu +Commit-Queue: Min Qin +Cr-Commit-Position: refs/heads/main@{#917013} + +diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc +index df2e3571636144acfefd1b79f157fd176db46ea0..f0a2d2bd58314420cc356400acc46ab412d49aca 100644 +--- a/chrome/browser/download/save_page_browsertest.cc ++++ b/chrome/browser/download/save_page_browsertest.cc +@@ -49,6 +49,7 @@ + #include "components/prefs/pref_member.h" + #include "components/prefs/pref_service.h" + #include "components/security_state/core/security_state.h" ++#include "components/services/quarantine/test_support.h" + #include "content/public/browser/download_manager.h" + #include "content/public/browser/notification_service.h" + #include "content/public/browser/notification_types.h" +@@ -431,6 +432,10 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveFileURL) { + EXPECT_TRUE(base::PathExists(full_file_name)); + EXPECT_FALSE(base::PathExists(dir)); + EXPECT_TRUE(base::ContentsEqual(GetTestDirFile("text.txt"), full_file_name)); ++#if defined(OS_WIN) ++ // Local file URL will not be quarantined. ++ EXPECT_FALSE(quarantine::IsFileQuarantined(full_file_name, GURL(), GURL())); ++#endif + } + + IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, +@@ -964,6 +969,25 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveUnauthorizedResource) { + EXPECT_FALSE(base::PathExists(dir.AppendASCII("should-not-save.jpg"))); + } + ++#if defined(OS_WIN) ++// Save a file and confirm that the file is correctly quarantined. ++IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveURLQuarantine) { ++ GURL url = embedded_test_server()->GetURL("/save_page/text.txt"); ++ ui_test_utils::NavigateToURL(browser(), url); ++ ++ base::FilePath full_file_name, dir; ++ SaveCurrentTab(url, content::SAVE_PAGE_TYPE_AS_ONLY_HTML, "test", 1, &dir, ++ &full_file_name); ++ ASSERT_FALSE(HasFailure()); ++ ++ base::ScopedAllowBlockingForTesting allow_blocking; ++ EXPECT_TRUE(base::PathExists(full_file_name)); ++ EXPECT_FALSE(base::PathExists(dir)); ++ EXPECT_TRUE(base::ContentsEqual(GetTestDirFile("text.txt"), full_file_name)); ++ EXPECT_TRUE(quarantine::IsFileQuarantined(full_file_name, url, GURL())); ++} ++#endif ++ + // Test suite that allows testing --site-per-process against cross-site frames. + // See http://dev.chromium.org/developers/design-documents/site-isolation. + class SavePageSitePerProcessBrowserTest : public SavePageBrowserTest { +diff --git a/content/browser/download/download_manager_impl.h b/content/browser/download/download_manager_impl.h +index b7e0163ab27e1b0e42d81776ae9242f72bd43e87..b29407e76d4e6f1f7fbbe62cea445d33c789fcc3 100644 +--- a/content/browser/download/download_manager_impl.h ++++ b/content/browser/download/download_manager_impl.h +@@ -170,6 +170,11 @@ class CONTENT_EXPORT DownloadManagerImpl + int frame_tree_node_id, + bool from_download_cross_origin_redirect); + ++ // DownloadItemImplDelegate overrides. ++ download::QuarantineConnectionCallback GetQuarantineConnectionCallback() ++ override; ++ std::string GetApplicationClientIdForFileScanning() const override; ++ + private: + using DownloadSet = std::set; + using DownloadGuidMap = +@@ -237,7 +242,6 @@ class CONTENT_EXPORT DownloadManagerImpl + bool ShouldOpenDownload(download::DownloadItemImpl* item, + ShouldOpenDownloadCallback callback) override; + void CheckForFileRemoval(download::DownloadItemImpl* download_item) override; +- std::string GetApplicationClientIdForFileScanning() const override; + void ResumeInterruptedDownload( + std::unique_ptr params, + const GURL& site_url) override; +@@ -249,8 +253,6 @@ class CONTENT_EXPORT DownloadManagerImpl + void ReportBytesWasted(download::DownloadItemImpl* download) override; + void BindWakeLockProvider( + mojo::PendingReceiver receiver) override; +- download::QuarantineConnectionCallback GetQuarantineConnectionCallback() +- override; + std::unique_ptr + GetRenameHandlerForDownload( + download::DownloadItemImpl* download_item) override; +diff --git a/content/browser/download/save_file.cc b/content/browser/download/save_file.cc +index 6e0cf6ed1df5db407e31be0e3a6db2e919d8e1bf..296c0cf79c9f5ee5e4ab9835394fb34ae7d92acc 100644 +--- a/content/browser/download/save_file.cc ++++ b/content/browser/download/save_file.cc +@@ -63,10 +63,15 @@ void SaveFile::Finish() { + file_.Finish(); + } + +-void SaveFile::AnnotateWithSourceInformation() { +- // TODO(gbillock): If this method is called, it should set the +- // file_.SetClientGuid() method first. +- NOTREACHED(); ++void SaveFile::AnnotateWithSourceInformation( ++ const std::string& client_guid, ++ const GURL& source_url, ++ const GURL& referrer_url, ++ mojo::PendingRemote remote_quarantine, ++ download::BaseFile::OnAnnotationDoneCallback on_annotation_done_callback) { ++ file_.AnnotateWithSourceInformation(client_guid, source_url, referrer_url, ++ std::move(remote_quarantine), ++ std::move(on_annotation_done_callback)); + } + + base::FilePath SaveFile::FullPath() const { +diff --git a/content/browser/download/save_file.h b/content/browser/download/save_file.h +index 688574b07f9374e75a25caaaa13bdb405aea7b0d..1893a0031f4c6642c6c806577da2246e55e49091 100644 +--- a/content/browser/download/save_file.h ++++ b/content/browser/download/save_file.h +@@ -34,7 +34,12 @@ class SaveFile { + void Detach(); + void Cancel(); + void Finish(); +- void AnnotateWithSourceInformation(); ++ void AnnotateWithSourceInformation( ++ const std::string& client_guid, ++ const GURL& source_url, ++ const GURL& referrer_url, ++ mojo::PendingRemote remote_quarantine, ++ download::BaseFile::OnAnnotationDoneCallback on_annotation_done_callback); + base::FilePath FullPath() const; + bool InProgress() const; + int64_t BytesSoFar() const; +diff --git a/content/browser/download/save_file_manager.cc b/content/browser/download/save_file_manager.cc +index bb322825a449920001b3c09fce33d3ed7e4d82da..98ce67d548d0880ed187779a0e41e7995c83543b 100644 +--- a/content/browser/download/save_file_manager.cc ++++ b/content/browser/download/save_file_manager.cc +@@ -51,6 +51,7 @@ static SaveFileManager* g_save_file_manager = nullptr; + class SaveFileManager::SimpleURLLoaderHelper + : public network::SimpleURLLoaderStreamConsumer { + public: ++ using URLLoaderCompleteCallback = base::OnceCallback; + static std::unique_ptr CreateAndStartDownload( + std::unique_ptr resource_request, + SaveItemId save_item_id, +@@ -59,11 +60,12 @@ class SaveFileManager::SimpleURLLoaderHelper + int render_frame_routing_id, + const net::NetworkTrafficAnnotationTag& annotation_tag, + network::mojom::URLLoaderFactory* url_loader_factory, +- SaveFileManager* save_file_manager) { ++ SaveFileManager* save_file_manager, ++ URLLoaderCompleteCallback on_complete_cb) { + return std::unique_ptr(new SimpleURLLoaderHelper( + std::move(resource_request), save_item_id, save_package_id, + render_process_id, render_frame_routing_id, annotation_tag, +- url_loader_factory, save_file_manager)); ++ url_loader_factory, save_file_manager, std::move(on_complete_cb))); + } + + ~SimpleURLLoaderHelper() override = default; +@@ -77,10 +79,12 @@ class SaveFileManager::SimpleURLLoaderHelper + int render_frame_routing_id, + const net::NetworkTrafficAnnotationTag& annotation_tag, + network::mojom::URLLoaderFactory* url_loader_factory, +- SaveFileManager* save_file_manager) ++ SaveFileManager* save_file_manager, ++ URLLoaderCompleteCallback on_complete_cb) + : save_file_manager_(save_file_manager), + save_item_id_(save_item_id), +- save_package_id_(save_package_id) { ++ save_package_id_(save_package_id), ++ on_complete_cb_(std::move(on_complete_cb)) { + GURL url = resource_request->url; + url_loader_ = network::SimpleURLLoader::Create(std::move(resource_request), + annotation_tag); +@@ -125,9 +129,7 @@ class SaveFileManager::SimpleURLLoaderHelper + + void OnComplete(bool success) override { + download::GetDownloadTaskRunner()->PostTask( +- FROM_HERE, +- base::BindOnce(&SaveFileManager::SaveFinished, save_file_manager_, +- save_item_id_, save_package_id_, success)); ++ FROM_HERE, base::BindOnce(std::move(on_complete_cb_), success)); + } + + void OnRetry(base::OnceClosure start_retry) override { +@@ -139,6 +141,7 @@ class SaveFileManager::SimpleURLLoaderHelper + SaveItemId save_item_id_; + SavePackageId save_package_id_; + std::unique_ptr url_loader_; ++ URLLoaderCompleteCallback on_complete_cb_; + + DISALLOW_COPY_AND_ASSIGN(SimpleURLLoaderHelper); + }; +@@ -189,17 +192,20 @@ SavePackage* SaveFileManager::LookupPackage(SaveItemId save_item_id) { + } + + // Call from SavePackage for starting a saving job +-void SaveFileManager::SaveURL(SaveItemId save_item_id, +- const GURL& url, +- const Referrer& referrer, +- int render_process_host_id, +- int render_view_routing_id, +- int render_frame_routing_id, +- SaveFileCreateInfo::SaveFileSource save_source, +- const base::FilePath& file_full_path, +- BrowserContext* context, +- StoragePartition* storage_partition, +- SavePackage* save_package) { ++void SaveFileManager::SaveURL( ++ SaveItemId save_item_id, ++ const GURL& url, ++ const Referrer& referrer, ++ int render_process_host_id, ++ int render_view_routing_id, ++ int render_frame_routing_id, ++ SaveFileCreateInfo::SaveFileSource save_source, ++ const base::FilePath& file_full_path, ++ BrowserContext* context, ++ StoragePartition* storage_partition, ++ SavePackage* save_package, ++ const std::string& client_guid, ++ mojo::PendingRemote remote_quarantine) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + + // Insert started saving job to tracking list. +@@ -285,11 +291,18 @@ void SaveFileManager::SaveURL(SaveItemId save_item_id, + factory = storage_partition->GetURLLoaderFactoryForBrowserProcess().get(); + } + ++ base::OnceCallback save_finished_cb = ++ base::BindOnce(&SaveFileManager::OnURLLoaderComplete, this, ++ save_item_id, save_package->id(), ++ context->IsOffTheRecord() ? GURL() : url, ++ context->IsOffTheRecord() ? GURL() : referrer.url, ++ client_guid, std::move(remote_quarantine)); ++ + url_loader_helpers_[save_item_id] = + SimpleURLLoaderHelper::CreateAndStartDownload( + std::move(request), save_item_id, save_package->id(), + render_process_host_id, render_frame_routing_id, traffic_annotation, +- factory, this); ++ factory, this, std::move(save_finished_cb)); + } else { + // We manually start the save job. + auto info = std::make_unique( +@@ -344,6 +357,36 @@ void SaveFileManager::SendCancelRequest(SaveItemId save_item_id) { + base::BindOnce(&SaveFileManager::CancelSave, this, save_item_id)); + } + ++void SaveFileManager::OnURLLoaderComplete( ++ SaveItemId save_item_id, ++ SavePackageId save_package_id, ++ const GURL& url, ++ const GURL& referrer_url, ++ const std::string& client_guid, ++ mojo::PendingRemote remote_quarantine, ++ bool is_success) { ++ DCHECK(download::GetDownloadTaskRunner()->RunsTasksInCurrentSequence()); ++ SaveFile* save_file = LookupSaveFile(save_item_id); ++ if (!is_success || !save_file) { ++ SaveFinished(save_item_id, save_package_id, is_success); ++ return; ++ } ++ ++ save_file->AnnotateWithSourceInformation( ++ client_guid, url, referrer_url, std::move(remote_quarantine), ++ base::BindOnce(&SaveFileManager::OnQuarantineComplete, this, save_item_id, ++ save_package_id)); ++} ++ ++void SaveFileManager::OnQuarantineComplete( ++ SaveItemId save_item_id, ++ SavePackageId save_package_id, ++ download::DownloadInterruptReason result) { ++ DCHECK(download::GetDownloadTaskRunner()->RunsTasksInCurrentSequence()); ++ SaveFinished(save_item_id, save_package_id, ++ result == download::DOWNLOAD_INTERRUPT_REASON_NONE); ++} ++ + // Notifications sent from the IO thread and run on the file thread: + + // The IO thread created |info|, but the file thread (this method) uses it +diff --git a/content/browser/download/save_file_manager.h b/content/browser/download/save_file_manager.h +index 51eb63a9b189be388e4dff48e04644956e968345..0d4290b273ba4f150bc9a49418e54b709a601581 100644 +--- a/content/browser/download/save_file_manager.h ++++ b/content/browser/download/save_file_manager.h +@@ -61,6 +61,8 @@ + + #include "base/macros.h" + #include "base/memory/ref_counted.h" ++#include "components/download/public/common/download_interrupt_reasons.h" ++#include "components/services/quarantine/quarantine.h" + #include "content/browser/download/save_types.h" + #include "content/common/content_export.h" + +@@ -90,17 +92,20 @@ class CONTENT_EXPORT SaveFileManager + + // Saves the specified URL |url|. |save_package| must not be deleted before + // the call to RemoveSaveFile. Should be called on the UI thread, +- void SaveURL(SaveItemId save_item_id, +- const GURL& url, +- const Referrer& referrer, +- int render_process_host_id, +- int render_view_routing_id, +- int render_frame_routing_id, +- SaveFileCreateInfo::SaveFileSource save_source, +- const base::FilePath& file_full_path, +- BrowserContext* context, +- StoragePartition* storage_partition, +- SavePackage* save_package); ++ void SaveURL( ++ SaveItemId save_item_id, ++ const GURL& url, ++ const Referrer& referrer, ++ int render_process_host_id, ++ int render_view_routing_id, ++ int render_frame_routing_id, ++ SaveFileCreateInfo::SaveFileSource save_source, ++ const base::FilePath& file_full_path, ++ BrowserContext* context, ++ StoragePartition* storage_partition, ++ SavePackage* save_package, ++ const std::string& client_guid, ++ mojo::PendingRemote remote_quarantine); + + // Notifications sent from the IO thread and run on the file thread: + void StartSave(std::unique_ptr info); +@@ -159,6 +164,21 @@ class CONTENT_EXPORT SaveFileManager + // Help function for sending notification of canceling specific request. + void SendCancelRequest(SaveItemId save_item_id); + ++ // Called on the file thread when the URLLoader completes saving a SaveItem. ++ void OnURLLoaderComplete( ++ SaveItemId save_item_id, ++ SavePackageId save_package_id, ++ const GURL& url, ++ const GURL& referrer_url, ++ const std::string& client_guid, ++ mojo::PendingRemote remote_quarantine, ++ bool is_success); ++ ++ // Called on the file thread when file quarantine finishes on a SaveItem. ++ void OnQuarantineComplete(SaveItemId save_item_id, ++ SavePackageId save_package_id, ++ download::DownloadInterruptReason result); ++ + // Notifications sent from the file thread and run on the UI thread. + + // Lookup the SaveManager for this WebContents' saving browser context and +diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc +index 7a5b64845a0e733467e3607cfa7feb75d1f3cb4c..0193ec702ffe5ba0fd3e87cbb65aa3b0e0bf3add 100644 +--- a/content/browser/download/save_package.cc ++++ b/content/browser/download/save_package.cc +@@ -859,6 +859,12 @@ void SavePackage::SaveNextFile(bool process_all_remaining_items) { + RenderFrameHostImpl* requester_frame = + requester_frame_tree_node->current_frame_host(); + ++ mojo::PendingRemote quarantine; ++ auto quarantine_callback = ++ download_manager_->GetQuarantineConnectionCallback(); ++ if (quarantine_callback) ++ quarantine_callback.Run(quarantine.InitWithNewPipeAndPassReceiver()); ++ + file_manager_->SaveURL( + save_item_ptr->id(), save_item_ptr->url(), save_item_ptr->referrer(), + requester_frame->GetProcess()->GetID(), +@@ -870,8 +876,8 @@ void SavePackage::SaveNextFile(bool process_all_remaining_items) { + .GetRenderViewHost() + ->GetProcess() + ->GetStoragePartition(), +- this); +- ++ this, download_manager_->GetApplicationClientIdForFileScanning(), ++ std::move(quarantine)); + } while (process_all_remaining_items && !waiting_item_queue_.empty()); + } +