Skip to content

Commit

Permalink
chore: cherry-pick 0894af410c4e from chromium (#31545)
Browse files Browse the repository at this point in the history
* chore: cherry-pick 0894af410c4e from chromium

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
  • Loading branch information
ppontes and patchup[bot] committed Oct 25, 2021
1 parent f2c078e commit 51e4b47
Show file tree
Hide file tree
Showing 2 changed files with 386 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -160,3 +160,4 @@ speculative_fix_for_eye_dropper_getcolor_crash.patch
mas_gate_private_enterprise_APIs
cherry-pick-c69dddfe1cde.patch
cherry-pick-8af66de55aad.patch
cherry-pick-0894af410c4e.patch
385 changes: 385 additions & 0 deletions patches/chromium/cherry-pick-0894af410c4e.patch
@@ -0,0 +1,385 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Min Qin <qinmin@chromium.org>
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 <xingliu@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
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 b5e3997002f14208e84c0bab2f3fdee17a4962ef..ef21c3d4fc4c425666af4f6fbb6213fa8f79b002 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"
@@ -433,6 +434,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,
@@ -936,6 +941,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 69fcf9abbe975ea35a2869f3601958e88aeb5951..0deb3b7c7781a37b47a5a04169ecd2e0ceaed4c8 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<download::DownloadItem*>;
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<download::DownloadUrlParameters> params,
const GURL& site_url) override;
@@ -249,8 +253,6 @@ class CONTENT_EXPORT DownloadManagerImpl
void ReportBytesWasted(download::DownloadItemImpl* download) override;
void BindWakeLockProvider(
mojo::PendingReceiver<device::mojom::WakeLockProvider> receiver) override;
- download::QuarantineConnectionCallback GetQuarantineConnectionCallback()
- override;
std::unique_ptr<download::DownloadItemRenameHandler>
GetRenameHandlerForDownload(
download::DownloadItemImpl* download_item) override;
diff --git a/content/browser/download/save_file.cc b/content/browser/download/save_file.cc
index 72331e60fca942820b39580cee5a1890340401ae..110f66250e9608426b26333203e93045f17e9f99 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<quarantine::mojom::Quarantine> 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<quarantine::mojom::Quarantine> 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 91786d976f7f637d659468d0700a6c858284dd66..2489b47cf864af0ff184f9250208832c31496698 100644
--- a/content/browser/download/save_file_manager.cc
+++ b/content/browser/download/save_file_manager.cc
@@ -50,6 +50,7 @@ static SaveFileManager* g_save_file_manager = nullptr;
class SaveFileManager::SimpleURLLoaderHelper
: public network::SimpleURLLoaderStreamConsumer {
public:
+ using URLLoaderCompleteCallback = base::OnceCallback<void(bool success)>;
static std::unique_ptr<SimpleURLLoaderHelper> CreateAndStartDownload(
std::unique_ptr<network::ResourceRequest> resource_request,
SaveItemId save_item_id,
@@ -58,11 +59,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<SimpleURLLoaderHelper>(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;
@@ -76,10 +78,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);
@@ -124,9 +128,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 {
@@ -138,6 +140,7 @@ class SaveFileManager::SimpleURLLoaderHelper
SaveItemId save_item_id_;
SavePackageId save_package_id_;
std::unique_ptr<network::SimpleURLLoader> url_loader_;
+ URLLoaderCompleteCallback on_complete_cb_;

DISALLOW_COPY_AND_ASSIGN(SimpleURLLoaderHelper);
};
@@ -188,17 +191,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<quarantine::mojom::Quarantine> 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<void(bool /*success*/)> 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<SaveFileCreateInfo>(
@@ -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<quarantine::mojom::Quarantine> 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<quarantine::mojom::Quarantine> remote_quarantine);

// Notifications sent from the IO thread and run on the file thread:
void StartSave(std::unique_ptr<SaveFileCreateInfo> 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<quarantine::mojom::Quarantine> 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 4ceea290dcc9b886fb2c65be4ff684854a0f131f..c4653492c8332201f1f6eeb2ce7dbd7fb20c7cc3 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -843,6 +843,12 @@ void SavePackage::SaveNextFile(bool process_all_remaining_items) {
RenderFrameHostImpl* requester_frame =
requester_frame_tree_node->current_frame_host();

+ mojo::PendingRemote<quarantine::mojom::Quarantine> 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(),
@@ -854,8 +860,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());
}

0 comments on commit 51e4b47

Please sign in to comment.