diff options
21 files changed, 492 insertions, 224 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index e9148d3..e86ccfc 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -2110,6 +2110,10 @@ each locale. --> desc="Message shown to the user to validate the download of an extension file."> Extensions, apps, and themes can harm your computer. Are you sure you want to continue? </message> + <message name="IDS_PROMPT_UNSAFE_DOWNLOAD_URL" + desc="Message shown to the user to validate the download when the download url is classified to lead a malware by safebrowsing database."> + This file appears to be malicious. Are you sure you want to continue? + </message> <message name="IDS_SAVE_DOWNLOAD" desc="Text for the button used to validate the downloading of a dangerous download."> Save diff --git a/chrome/app/theme/theme_resources.grd b/chrome/app/theme/theme_resources.grd index e5a534a..59c7114 100644 --- a/chrome/app/theme/theme_resources.grd +++ b/chrome/app/theme/theme_resources.grd @@ -293,6 +293,7 @@ <include name="IDR_RESTORE_BUTTON_MASK" file="restore_button_mask.png" type="BINDATA" /> <include name="IDR_SAD_FAVICON" file="sadfavicon.png" type="BINDATA" /> <include name="IDR_SAD_TAB" file="sadtab.png" type="BINDATA" /> + <include name="IDR_SAFEBROWSING_WARNING" file="safebrowsing_warning.png" type="BINDATA" /> <include name="IDR_SEARCH_ENGINE_DIALOG_TOP" file="search_engine_top.png" type="BINDATA" /> <if expr="pp_ifdef('_google_chrome')"> <include name="IDR_SEARCH_ENGINE_LOGO_ABCSOK" file="google_chrome/search_abcsok.png" type="BINDATA" /> diff --git a/chrome/browser/download/download_item.cc b/chrome/browser/download/download_item.cc index c48a5a8..6b2dae9 100644 --- a/chrome/browser/download/download_item.cc +++ b/chrome/browser/download/download_item.cc @@ -87,6 +87,26 @@ const char* DebugDownloadStateString(DownloadItem::DownloadState state) { }; } +DownloadItem::SafetyState GetSafetyState(bool dangerous_file, + bool dangerous_url) { + return (dangerous_url || dangerous_file) ? + DownloadItem::DANGEROUS : DownloadItem::SAFE; +} + +// Note: When a download has both |dangerous_file| and |dangerous_url| set, +// danger type is set to DANGEROUS_URL since the risk of dangerous URL +// overweights that of dangerous file type. +DownloadItem::DangerType GetDangerType(bool dangerous_file, + bool dangerous_url) { + if (dangerous_url) { + // dangerous URL overweights dangerous file. We check dangerous URL first. + return DownloadItem::DANGEROUS_URL; + } else if (dangerous_file) { + return DownloadItem::DANGEROUS_FILE; + } + return DownloadItem::NOT_DANGEROUS; +} + } // namespace // Constructor for reading from the history service. @@ -110,6 +130,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, is_paused_(false), open_when_complete_(false), safety_state_(SAFE), + danger_type_(NOT_DANGEROUS), auto_opened_(false), target_name_(info.original_name), render_process_id_(-1), @@ -149,7 +170,10 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, download_manager_(download_manager), is_paused_(false), open_when_complete_(false), - safety_state_(info.is_dangerous ? DANGEROUS : SAFE), + safety_state_(GetSafetyState(info.is_dangerous_file, + info.is_dangerous_url)), + danger_type_(GetDangerType(info.is_dangerous_file, + info.is_dangerous_url)), auto_opened_(false), target_name_(info.original_name), render_process_id_(info.child_id), @@ -187,6 +211,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager, is_paused_(false), open_when_complete_(false), safety_state_(SAFE), + danger_type_(NOT_DANGEROUS), auto_opened_(false), render_process_id_(-1), request_id_(-1), @@ -497,14 +522,16 @@ bool DownloadItem::MatchesQuery(const string16& query) const { } void DownloadItem::SetFileCheckResults(const FilePath& path, - bool is_dangerous, + bool is_dangerous_file, + bool is_dangerous_url, int path_uniquifier, bool prompt, bool is_extension_install, const FilePath& original_name) { VLOG(20) << " " << __FUNCTION__ << "()" << " path = \"" << path.value() << "\"" - << " is_dangerous = " << is_dangerous + << " is_dangerous_file = " << is_dangerous_file + << " is_dangerous_url = " << is_dangerous_url << " path_uniquifier = " << path_uniquifier << " prompt = " << prompt << " is_extension_install = " << is_extension_install @@ -516,7 +543,8 @@ void DownloadItem::SetFileCheckResults(const FilePath& path, DCHECK(!path.empty()); full_path_ = path; - safety_state_ = is_dangerous ? DANGEROUS : SAFE; + safety_state_ = GetSafetyState(is_dangerous_file, is_dangerous_url); + danger_type_ = GetDangerType(is_dangerous_file, is_dangerous_url); path_uniquifier_ = path_uniquifier; save_as_ = prompt; is_extension_install_ = is_extension_install; diff --git a/chrome/browser/download/download_item.h b/chrome/browser/download/download_item.h index eaa9a55..5eadea8 100644 --- a/chrome/browser/download/download_item.h +++ b/chrome/browser/download/download_item.h @@ -62,6 +62,17 @@ class DownloadItem { DANGEROUS_BUT_VALIDATED // Dangerous but the user confirmed the download. }; + enum DangerType { + NOT_DANGEROUS = 0, + + // A dangerous file to the system (e.g.: an executable or extension from + // places other than gallery). + DANGEROUS_FILE, + + // Safebrowsing service shows this URL leads to malicious file download. + DANGEROUS_URL + }; + // Interface that observers of a particular download must implement in order // to receive updates to the download's status. class Observer { @@ -174,7 +185,8 @@ class DownloadItem { // result of analyzing the file and figuring out its type, location, etc. // May only be called once. void SetFileCheckResults(const FilePath& path, - bool is_dangerous, + bool is_dangerous_file, + bool is_dangerous_url, int path_uniquifier, bool prompt, bool is_extension_install, @@ -226,6 +238,7 @@ class DownloadItem { void set_safety_state(SafetyState safety_state) { safety_state_ = safety_state; } + DangerType danger_type() { return danger_type_;} bool auto_opened() { return auto_opened_; } FilePath target_name() const { return target_name_; } bool save_as() const { return save_as_; } @@ -329,6 +342,9 @@ class DownloadItem { // (executable files are typically considered dangerous). SafetyState safety_state_; + // Why |safety_state_| is not SAFE. + DangerType danger_type_; + // Whether the download was auto-opened. We set this rather than using // an observer as it's frequently possible for the download to be auto opened // before the observer is added. diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc index 52b2e78..697684d 100644 --- a/chrome/browser/download/download_manager.cc +++ b/chrome/browser/download/download_manager.cc @@ -23,6 +23,7 @@ #include "chrome/browser/download/download_history.h" #include "chrome/browser/download/download_item.h" #include "chrome/browser/download/download_prefs.h" +#include "chrome/browser/download/download_safe_browsing_client.h" #include "chrome/browser/download/download_status_updater.h" #include "chrome/browser/download/download_util.h" #include "chrome/browser/extensions/extension_service.h" @@ -246,8 +247,21 @@ bool DownloadManager::Init(Profile* profile) { // history thread. void DownloadManager::StartDownload(DownloadCreateInfo* info) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + // Create a client to verify download URL with safebrowsing. + // It deletes itself after the callback. + scoped_refptr<DownloadSBClient> sb_client = new DownloadSBClient(info); + sb_client->CheckDownloadUrl( + NewCallback(this, &DownloadManager::CheckDownloadUrlDone)); +} + +void DownloadManager::CheckDownloadUrlDone(DownloadCreateInfo* info, + bool is_dangerous_url) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(info); + info->is_dangerous_url = is_dangerous_url; + // Check whether this download is for an extension install or not. // Allow extensions to be explicitly saved. if (!info->prompt_user_for_save_location) { @@ -293,7 +307,7 @@ void DownloadManager::StartDownload(DownloadCreateInfo* info) { if (!info->prompt_user_for_save_location && info->save_info.file_path.empty()) { - info->is_dangerous = download_util::IsDangerous( + info->is_dangerous_file = download_util::IsDangerous( info, profile(), ShouldOpenFileBasedOnExtension(info->suggested_path)); } @@ -332,7 +346,7 @@ void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info, } // If the download is deemed dangerous, we'll use a temporary name for it. - if (info->is_dangerous) { + if (info->IsDangerous()) { info->original_name = FilePath(info->suggested_path).BaseName(); // Create a temporary file to hold the file until the user approves its // download. @@ -374,7 +388,7 @@ void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info, } else if (info->path_uniquifier == -1) { // We failed to find a unique path. We have to prompt the user. VLOG(1) << "Unable to find a unique path for suggested path \"" - << info->suggested_path.value() << "\""; + << info->suggested_path.value() << "\""; info->prompt_user_for_save_location = true; } } @@ -384,7 +398,7 @@ void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info, // See: http://code.google.com/p/chromium/issues/detail?id=3662 if (!info->prompt_user_for_save_location && info->save_info.file_path.empty()) { - if (info->is_dangerous) + if (info->IsDangerous()) file_util::WriteFile(info->suggested_path, "", 0); else file_util::WriteFile(download_util::GetCrDownloadPath( @@ -426,7 +440,8 @@ void DownloadManager::OnPathExistenceAvailable(DownloadCreateInfo* info) { FOR_EACH_OBSERVER(Observer, observers_, SelectFileDialogDisplayed()); } else { // No prompting for download, just continue with the suggested name. - AttachDownloadItem(info, info->suggested_path); + info->path = info->suggested_path; + AttachDownloadItem(info); } } @@ -441,14 +456,13 @@ void DownloadManager::CreateDownloadItem(DownloadCreateInfo* info) { active_downloads_[info->download_id] = download; } -void DownloadManager::AttachDownloadItem(DownloadCreateInfo* info, - const FilePath& target_path) { +void DownloadManager::AttachDownloadItem(DownloadCreateInfo* info) { VLOG(20) << __FUNCTION__ << "()" << " info = " << info->DebugString(); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + // Life of |info| ends here. No more references to it after this method. scoped_ptr<DownloadCreateInfo> infop(info); - info->path = target_path; // NOTE(ahendrickson) Eventually |active_downloads_| will replace // |in_progress_|, but we don't want to change the semantics yet. @@ -459,7 +473,8 @@ void DownloadManager::AttachDownloadItem(DownloadCreateInfo* info, DCHECK(ContainsKey(downloads_, download)); download->SetFileCheckResults(info->path, - info->is_dangerous, + info->is_dangerous_file, + info->is_dangerous_url, info->path_uniquifier, info->prompt_user_for_save_location, info->is_extension_install, @@ -468,7 +483,7 @@ void DownloadManager::AttachDownloadItem(DownloadCreateInfo* info, UpdateAppIcon(); // Reflect entry into in_progress_. // Rename to intermediate name. - if (info->is_dangerous) { + if (info->IsDangerous()) { // The download is not safe. We can now rename the file to its // tentative name using OnFinalDownloadName (the actual final // name after user confirmation will be set in @@ -477,13 +492,13 @@ void DownloadManager::AttachDownloadItem(DownloadCreateInfo* info, BrowserThread::FILE, FROM_HERE, NewRunnableMethod( file_manager_, &DownloadFileManager::OnFinalDownloadName, - download->id(), target_path, make_scoped_refptr(this))); + download->id(), info->path, make_scoped_refptr(this))); } else { // The download is a safe download. We need to // rename it to its intermediate '.crdownload' path. The final // name after user confirmation will be set from // DownloadItem::OnSafeDownloadFinished. - FilePath download_path = download_util::GetCrDownloadPath(target_path); + FilePath download_path = download_util::GetCrDownloadPath(info->path); BrowserThread::PostTask( BrowserThread::FILE, FROM_HERE, NewRunnableMethod( @@ -935,7 +950,9 @@ void DownloadManager::FileSelected(const FilePath& path, DownloadCreateInfo* info = reinterpret_cast<DownloadCreateInfo*>(params); if (info->prompt_user_for_save_location) last_download_path_ = path.DirName(); - AttachDownloadItem(info, path); + + info->path = path; + AttachDownloadItem(info); } void DownloadManager::FileSelectionCanceled(void* params) { diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h index e103a79..b1cc695 100644 --- a/chrome/browser/download/download_manager.h +++ b/chrome/browser/download/download_manager.h @@ -221,6 +221,9 @@ class DownloadManager // Called when the user has validated the download of a dangerous file. void DangerousDownloadValidated(DownloadItem* download); + // Callback function after url is checked with safebrowsing service. + void CheckDownloadUrlDone(DownloadCreateInfo* info, bool is_dangerous_url); + private: // For testing. friend class DownloadManagerTest; @@ -266,8 +269,7 @@ class DownloadManager // Called back after a target path for the file to be downloaded to has been // determined, either automatically based on the suggested file name, or by // the user in a Save As dialog box. - void AttachDownloadItem(DownloadCreateInfo* info, - const FilePath& target_path); + void AttachDownloadItem(DownloadCreateInfo* info); // Download cancel helper function. void DownloadCancelledInternal(int download_id, diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc index f9f5f4d..8923faf 100644 --- a/chrome/browser/download/download_manager_unittest.cc +++ b/chrome/browser/download/download_manager_unittest.cc @@ -9,6 +9,7 @@ #include "chrome/browser/browser_thread.h" #include "chrome/browser/download/download_file.h" #include "chrome/browser/download/download_file_manager.h" +#include "chrome/browser/download/download_item.h" #include "chrome/browser/download/download_manager.h" #include "chrome/browser/download/download_prefs.h" #include "chrome/browser/download/download_status_updater.h" @@ -60,6 +61,17 @@ class DownloadManagerTest : public testing::Test { return file_manager_; } + // Make sure download item |id| was set with correct safety state for + // given |is_dangerous_file| and |is_dangerous_url|. + bool VerifySafetyState(bool is_dangerous_file, + bool is_dangerous_url, + int id) { + DownloadItem::SafetyState safety_state = + download_manager_->GetDownloadItem(id)->safety_state(); + return (is_dangerous_file || is_dangerous_url) ? + safety_state != DownloadItem::SAFE : safety_state == DownloadItem::SAFE; + } + DISALLOW_COPY_AND_ASSIGN(DownloadManagerTest); }; @@ -117,6 +129,7 @@ const struct { } // namespace TEST_F(DownloadManagerTest, StartDownload) { + BrowserThread io_thread(BrowserThread::IO, &message_loop_); PrefService* prefs = profile_->GetPrefs(); prefs->SetFilePath(prefs::kDownloadDefaultDirectory, FilePath()); download_manager_->download_prefs()->EnableAutoOpenBasedOnExtension( @@ -132,6 +145,7 @@ TEST_F(DownloadManagerTest, StartDownload) { info.mime_type = kStartDownloadCases[i].mime_type; download_manager_->StartDownload(&info); + message_loop_.RunAllPending(); EXPECT_EQ(kStartDownloadCases[i].expected_save_as, info.prompt_user_for_save_location); @@ -142,7 +156,8 @@ namespace { const struct { FilePath::StringType suggested_path; - bool is_dangerous; + bool is_dangerous_file; + bool is_dangerous_url; bool finish_before_rename; int expected_rename_count; } kDownloadRenameCases[] = { @@ -150,27 +165,28 @@ const struct { // Renamed twice (linear path through UI). Crdownload file does not need // to be deleted. { FILE_PATH_LITERAL("foo.zip"), - false, - true, - 2, }, - // Dangerous download, download finishes BEFORE file name determined. - // Needs to be renamed only once. + false, false, true, 2, }, + // Dangerous download (file is dangerous or download URL is not safe or both), + // download finishes BEFORE file name determined. Needs to be renamed only + // once. { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), - true, - true, - 1, }, + true, false, true, 1, }, + { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), + false, true, true, 1, }, + { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), + true, true, true, 1, }, // Safe download, download finishes AFTER file name determined. // Needs to be renamed twice. { FILE_PATH_LITERAL("foo.zip"), - false, - false, - 2, }, + false, false, false, 2, }, // Dangerous download, download finishes AFTER file name determined. // Needs to be renamed only once. { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), - true, - false, - 1, }, + true, false, false, 1, }, + { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), + false, true, false, 1, }, + { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), + true, true, false, 1, }, }; class MockDownloadFile : public DownloadFile { @@ -209,7 +225,8 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) { DownloadCreateInfo* info(new DownloadCreateInfo); info->download_id = static_cast<int>(i); info->prompt_user_for_save_location = false; - info->is_dangerous = kDownloadRenameCases[i].is_dangerous; + info->is_dangerous_file = kDownloadRenameCases[i].is_dangerous_file; + info->is_dangerous_url = kDownloadRenameCases[i].is_dangerous_url; FilePath new_path(kDownloadRenameCases[i].suggested_path); MockDownloadFile* download(new MockDownloadFile(info)); @@ -243,5 +260,8 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) { } message_loop_.RunAllPending(); + EXPECT_TRUE(VerifySafetyState(kDownloadRenameCases[i].is_dangerous_file, + kDownloadRenameCases[i].is_dangerous_url, + i)); } } diff --git a/chrome/browser/download/download_safe_browsing_client.cc b/chrome/browser/download/download_safe_browsing_client.cc new file mode 100644 index 0000000..e833707 --- /dev/null +++ b/chrome/browser/download/download_safe_browsing_client.cc @@ -0,0 +1,90 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +// + +#include "chrome/browser/download/download_safe_browsing_client.h" + +#include "base/logging.h" +#include "base/metrics/histogram.h" +#include "base/metrics/stats_counters.h" +#include "chrome/browser/browser_process.h" +#include "chrome/browser/browser_thread.h" +#include "chrome/browser/download/download_manager.h" +#include "chrome/browser/history/download_create_info.h" +#include "chrome/browser/renderer_host/resource_dispatcher_host.h" + +// TODO(lzheng): Get rid of the AddRef and Release after +// SafeBrowsingService::Client is changed to RefCountedThreadSafe<>. + +DownloadSBClient::DownloadSBClient(DownloadCreateInfo* info) : info_(info) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + CHECK(info_); + ResourceDispatcherHost* rdh = g_browser_process->resource_dispatcher_host(); + if (rdh) + sb_service_ = rdh->safe_browsing_service(); +} + +void DownloadSBClient::CheckDownloadUrl(DoneCallback* callback) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + // It is not allowed to call this method twice. + CHECK(!done_callback_.get()); + CHECK(callback); + + start_time_ = base::TimeTicks::Now(); + done_callback_.reset(callback); + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + NewRunnableMethod(this, + &DownloadSBClient::CheckDownloadUrlOnIOThread)); + UpdateDownloadUrlCheckStats(DOWNLOAD_URL_CHECKS_TOTAL); +} + +void DownloadSBClient::CheckDownloadUrlOnIOThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + + // Will be released in OnDownloadUrlCheckResult. + AddRef(); + if (sb_service_.get() && !sb_service_->CheckDownloadUrl(info_->url, this)) { + // Wait for SafeBrowsingService to call back OnDownloadUrlCheckResult. + return; + } + OnDownloadUrlCheckResult(info_->url, SafeBrowsingService::SAFE); +} + +// The callback interface for SafeBrowsingService::Client. +// Called when the result of checking a download URL is known. +void DownloadSBClient::OnDownloadUrlCheckResult( + const GURL& url, SafeBrowsingService::UrlCheckResult result) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + NewRunnableMethod(this, + &DownloadSBClient::SafeBrowsingCheckUrlDone, + result)); + Release(); +} + +void DownloadSBClient::SafeBrowsingCheckUrlDone( + SafeBrowsingService::UrlCheckResult result) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + bool is_dangerous = result != SafeBrowsingService::SAFE; + done_callback_->Run(info_, is_dangerous); + UMA_HISTOGRAM_TIMES("SB2.DownloadUrlCheckDuration", + base::TimeTicks::Now() - start_time_); + if (is_dangerous) { + UpdateDownloadUrlCheckStats(DOWNLOAD_URL_CHECKS_MALWARE); + + // TODO(lzheng): we need to collect page_url and referrer_url to report. + sb_service_->ReportSafeBrowsingHit(info_->url, + info_->url /* page_url */, + info_->url /* referrer_url */, + true, + result); + } +} + +void DownloadSBClient::UpdateDownloadUrlCheckStats(SBStatsType stat_type) { + UMA_HISTOGRAM_ENUMERATION("SB2.DownloadUrlChecks", + stat_type, + DOWNLOAD_URL_CHECKS_MAX); +} diff --git a/chrome/browser/download/download_safe_browsing_client.h b/chrome/browser/download/download_safe_browsing_client.h new file mode 100644 index 0000000..fd50e9f --- /dev/null +++ b/chrome/browser/download/download_safe_browsing_client.h @@ -0,0 +1,79 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_SAFE_BROWSING_CLIENT_H_ +#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_SAFE_BROWSING_CLIENT_H_ +#pragma once + +#include "base/callback.h" +#include "base/ref_counted.h" +#include "base/time.h" +#include "chrome/browser/safe_browsing/safe_browsing_service.h" + +struct DownloadCreateInfo; + +// This is a helper class used by DownloadManager to check a download URL with +// SafeBrowsingService. The client is refcounted and will be released once +// there is no reference to it. +// Usage: +// { +// scoped_refptr<DownloadSBClient> client_ = new DownloadSBClient(...); +// client_->CheckDownloadUrl(NewCallback(this, &DownloadManager::CallBack)); +// } +// DownloadManager::CallBack(...) { +// // After this, the |client_| is gone. +// } +class DownloadSBClient + : public SafeBrowsingService::Client, + public base::RefCountedThreadSafe<DownloadSBClient> { + public: + typedef Callback2<DownloadCreateInfo*, bool>::Type DoneCallback; + + explicit DownloadSBClient(DownloadCreateInfo* info); + + // Note: This method can only be called once per DownloadSBClient instance. + void CheckDownloadUrl(DoneCallback* callback); + + protected: + // Call SafeBrowsingService on IO thread to verify the download URL. + void CheckDownloadUrlOnIOThread(); + + // The callback interface for SafeBrowsingService::Client. + // Called when the result of checking a download URL is known. + virtual void OnDownloadUrlCheckResult( + const GURL& url, SafeBrowsingService::UrlCheckResult result); + + private: + // Enumerate for histogramming purposes. + // DO NOT CHANGE THE ORDERING OF THESE VALUES (different histogram data will + // be mixed together based on their values). + enum SBStatsType { + DOWNLOAD_URL_CHECKS_TOTAL, + DOWNLOAD_URL_CHECKS_CANCELED, + DOWNLOAD_URL_CHECKS_MALWARE, + + // Memory space for histograms is determined by the max. + // ALWAYS ADD NEW VALUES BEFORE THIS ONE. + DOWNLOAD_URL_CHECKS_MAX + }; + + // Call DownloadManager on UI thread. + void SafeBrowsingCheckUrlDone(SafeBrowsingService::UrlCheckResult result); + + // Update the UMA stats. + void UpdateDownloadUrlCheckStats(SBStatsType stat_type); + + scoped_ptr<DoneCallback> done_callback_; + + // Not owned by this class. + DownloadCreateInfo* info_; + scoped_refptr<SafeBrowsingService> sb_service_; + + // When a safebrowsing URL check starts, for stats purpose. + base::TimeTicks start_time_; + + DISALLOW_COPY_AND_ASSIGN(DownloadSBClient); +}; + +#endif // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_SAFE_BROWSING_CLIENT_H_ diff --git a/chrome/browser/history/download_create_info.cc b/chrome/browser/history/download_create_info.cc index d1ea28e..eaa007a 100644 --- a/chrome/browser/history/download_create_info.cc +++ b/chrome/browser/history/download_create_info.cc @@ -31,7 +31,8 @@ DownloadCreateInfo::DownloadCreateInfo(const FilePath& path, request_id(-1), db_handle(0), prompt_user_for_save_location(false), - is_dangerous(false), + is_dangerous_file(false), + is_dangerous_url(false), is_extension_install(false) { } @@ -47,13 +48,17 @@ DownloadCreateInfo::DownloadCreateInfo() request_id(-1), db_handle(0), prompt_user_for_save_location(false), - is_dangerous(false), + is_dangerous_file(false), + is_dangerous_url(false), is_extension_install(false) { } DownloadCreateInfo::~DownloadCreateInfo() { } +bool DownloadCreateInfo::IsDangerous() { + return is_dangerous_url || is_dangerous_file; +} std::string DownloadCreateInfo::DebugString() const { return base::StringPrintf("{" " url_ = \"%s\"" @@ -76,4 +81,3 @@ std::string DownloadCreateInfo::DebugString() const { download_id, prompt_user_for_save_location ? 'T' : 'F'); } - diff --git a/chrome/browser/history/download_create_info.h b/chrome/browser/history/download_create_info.h index 0814350..455e77a 100644 --- a/chrome/browser/history/download_create_info.h +++ b/chrome/browser/history/download_create_info.h @@ -32,6 +32,9 @@ struct DownloadCreateInfo { DownloadCreateInfo(); ~DownloadCreateInfo(); + // Indicates if the download is dangerous. + bool IsDangerous(); + std::string DebugString() const; // DownloadItem fields @@ -68,8 +71,10 @@ struct DownloadCreateInfo { // False if the UI should be supressed and the download performed to the // default location. bool prompt_user_for_save_location; - // Whether this download is potentially dangerous (ex: exe, dll, ...). - bool is_dangerous; + // Whether this download file is potentially dangerous (ex: exe, dll, ...). + bool is_dangerous_file; + // If safebrowsing believes this URL leads to malware. + bool is_dangerous_url; // The original name for a dangerous download. FilePath original_name; // Whether this download is for extension install or not. diff --git a/chrome/browser/renderer_host/download_resource_handler.cc b/chrome/browser/renderer_host/download_resource_handler.cc index 246b995..193c3e5 100644 --- a/chrome/browser/renderer_host/download_resource_handler.cc +++ b/chrome/browser/renderer_host/download_resource_handler.cc @@ -44,8 +44,7 @@ DownloadResourceHandler::DownloadResourceHandler( save_info_(save_info), buffer_(new DownloadBuffer), rdh_(rdh), - is_paused_(false), - url_check_pending_(false) { + is_paused_(false) { } bool DownloadResourceHandler::OnUploadProgress(int request_id, @@ -63,55 +62,12 @@ bool DownloadResourceHandler::OnRequestRedirected(int request_id, return true; } -// Callback when the result of checking a download URL is known. -// TODO(lzheng): We should create a information bar with buttons to ask -// if users want to proceed when the download is malicious. -void DownloadResourceHandler::OnDownloadUrlCheckResult( - const GURL& url, SafeBrowsingService::UrlCheckResult result) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - DCHECK(url_check_pending_); - - UMA_HISTOGRAM_TIMES("SB2.DownloadUrlCheckDuration", - base::TimeTicks::Now() - download_start_time_); - - if (result == SafeBrowsingService::BINARY_MALWARE_URL) { - // TODO(lzheng): More UI work to show warnings properly on download shelf. - DLOG(WARNING) << "This url leads to a malware downloading: " - << url.spec(); - UpdateDownloadUrlCheckStats(DOWNLOAD_URL_CHECKS_MALWARE); - } - - url_check_pending_ = false; - // Note: Release() should be the last line in this call. It is for - // the AddRef in CheckSafeBrowsing. - Release(); -} - -// Send the download creation information to the download thread. -void DownloadResourceHandler::StartDownloadUrlCheck() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - AddRef(); - if (!rdh_->safe_browsing_service()->CheckDownloadUrl(url_, this)) { - url_check_pending_ = true; - UpdateDownloadUrlCheckStats(DOWNLOAD_URL_CHECKS_TOTAL); - // Note: in this case, the AddRef will be balanced in - // "OnDownloadUrlCheckResult" or "OnRequestClosed". - } else { - // Immediately release the AddRef() at the beginning of this function - // since no more callbacks will happen. - Release(); - DVLOG(1) << "url: " << url_.spec() << " is safe to download."; - } -} - // Send the download creation information to the download thread. bool DownloadResourceHandler::OnResponseStarted(int request_id, ResourceResponse* response) { VLOG(20) << __FUNCTION__ << "()" << DebugString() << " request_id = " << request_id; - DCHECK(!url_check_pending_); download_start_time_ = base::TimeTicks::Now(); - StartDownloadUrlCheck(); std::string content_disposition; request_->GetResponseHeaderByName("content-disposition", &content_disposition); @@ -122,7 +78,8 @@ bool DownloadResourceHandler::OnResponseStarted(int request_id, ResourceDispatcherHost::InfoForRequest(request_); download_id_ = download_file_manager_->GetNextId(); - // |download_file_manager_| consumes (deletes): + + // Deleted in DownloadManager. DownloadCreateInfo* info = new DownloadCreateInfo; info->url = url_; info->original_url = original_url_; @@ -147,7 +104,8 @@ bool DownloadResourceHandler::OnResponseStarted(int request_id, info->prompt_user_for_save_location = save_as_ && save_info_.file_path.empty(); - info->is_dangerous = false; + info->is_dangerous_file = false; + info->is_dangerous_url = false; info->referrer_charset = request_->context()->referrer_charset(); info->save_info = save_info_; BrowserThread::PostTask( @@ -234,15 +192,6 @@ bool DownloadResourceHandler::OnResponseCompleted( void DownloadResourceHandler::OnRequestClosed() { UMA_HISTOGRAM_TIMES("SB2.DownloadDuration", base::TimeTicks::Now() - download_start_time_); - if (url_check_pending_) { - DVLOG(1) << "Cancel pending download url checking request: " << this; - rdh_->safe_browsing_service()->CancelCheck(this); - UpdateDownloadUrlCheckStats(DOWNLOAD_URL_CHECKS_CANCELED); - url_check_pending_ = false; - // Balance the AddRef() from StartDownloadUrlCheck() which would usually be - // balanced by OnDownloadUrlCheckResult(). - Release(); - } } // If the content-length header is not present (or contains something other @@ -309,10 +258,3 @@ std::string DownloadResourceHandler::DebugString() const { render_view_id_, save_info_.file_path.value().c_str()); } - -void DownloadResourceHandler::UpdateDownloadUrlCheckStats( - SBStatsType stat_type) { - UMA_HISTOGRAM_ENUMERATION("SB2.DownloadUrlChecks", - stat_type, - DOWNLOAD_URL_CHECKS_MAX); -} diff --git a/chrome/browser/renderer_host/download_resource_handler.h b/chrome/browser/renderer_host/download_resource_handler.h index f416ee2..219961c 100644 --- a/chrome/browser/renderer_host/download_resource_handler.h +++ b/chrome/browser/renderer_host/download_resource_handler.h @@ -12,7 +12,6 @@ #include "chrome/browser/download/download_file.h" #include "chrome/browser/renderer_host/global_request_id.h" #include "chrome/browser/renderer_host/resource_handler.h" -#include "chrome/browser/safe_browsing/safe_browsing_service.h" class DownloadFileManager; class ResourceDispatcherHost; @@ -23,8 +22,7 @@ class URLRequest; } // namespace net // Forwards data to the download thread. -class DownloadResourceHandler : public ResourceHandler, - public SafeBrowsingService::Client { +class DownloadResourceHandler : public ResourceHandler { public: DownloadResourceHandler(ResourceDispatcherHost* rdh, int render_process_host_id, @@ -72,32 +70,10 @@ class DownloadResourceHandler : public ResourceHandler, std::string DebugString() const; private: - // Enumerate for histogramming purposes. DO NOT CHANGE THE - // ORDERING OF THESE VALUES. - enum SBStatsType { - DOWNLOAD_URL_CHECKS_TOTAL, - DOWNLOAD_URL_CHECKS_CANCELED, - DOWNLOAD_URL_CHECKS_MALWARE, - - // Memory space for histograms is determined by the max. ALWAYS - // ADD NEW VALUES BEFORE THIS ONE. - DOWNLOAD_URL_CHECKS_MAX - }; - ~DownloadResourceHandler(); void StartPauseTimer(); - void StartDownloadUrlCheck(); - - // Called when the result of checking a download URL is known. - virtual void OnDownloadUrlCheckResult( - const GURL& url, - SafeBrowsingService::UrlCheckResult result); - - // A helper function that updates UMA for download url checks. - static void UpdateDownloadUrlCheckStats(SBStatsType stat_type); - int download_id_; GlobalRequestID global_id_; int render_view_id_; @@ -114,9 +90,7 @@ class DownloadResourceHandler : public ResourceHandler, ResourceDispatcherHost* rdh_; bool is_paused_; base::OneShotTimer<DownloadResourceHandler> pause_timer_; - bool url_check_pending_; base::TimeTicks download_start_time_; // used to collect stats. - static const int kReadBufSize = 32768; // bytes static const size_t kLoadsToWrite = 100; // number of data buffers queued static const int kThrottleTimeMs = 200; // milliseconds diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc index 5e61eca..77e0a8a 100644 --- a/chrome/browser/safe_browsing/protocol_manager.cc +++ b/chrome/browser/safe_browsing/protocol_manager.cc @@ -762,14 +762,27 @@ GURL SafeBrowsingProtocolManager::SafeBrowsingHitUrl( const GURL& referrer_url, bool is_subresource, SafeBrowsingService::UrlCheckResult threat_type) const { DCHECK(threat_type == SafeBrowsingService::URL_MALWARE || - threat_type == SafeBrowsingService::URL_PHISHING); + threat_type == SafeBrowsingService::URL_PHISHING || + threat_type == SafeBrowsingService::BINARY_MALWARE_URL); // The malware and phishing hits go over HTTP. std::string url = ComposeUrl(http_url_prefix_, "report", client_name_, version_, additional_query_); + std::string threat_list = "none"; + switch (threat_type) { + case SafeBrowsingService::URL_MALWARE: + threat_list = "malblhit"; + break; + case SafeBrowsingService::URL_PHISHING: + threat_list = "phishblhit"; + break; + case SafeBrowsingService::BINARY_MALWARE_URL: + threat_list = "binurlhit"; + break; + default: + NOTREACHED(); + } return GURL(StringPrintf("%s&evts=%s&evtd=%s&evtr=%s&evhr=%s&evtb=%d", - url.c_str(), - (threat_type == SafeBrowsingService::URL_MALWARE) ? - "malblhit" : "phishblhit", + url.c_str(), threat_list.c_str(), EscapeQueryParamValue(malicious_url.spec(), true).c_str(), EscapeQueryParamValue(page_url.spec(), true).c_str(), EscapeQueryParamValue(referrer_url.spec(), true).c_str(), diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc index 70f8a3f..e7248c9 100644 --- a/chrome/browser/safe_browsing/safe_browsing_service.cc +++ b/chrome/browser/safe_browsing/safe_browsing_service.cc @@ -1064,30 +1064,43 @@ void SafeBrowsingService::DoDisplayBlockingPage( referrer_url = page_url; page_url = resource.original_url; } - - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - NewRunnableMethod( - this, - &SafeBrowsingService::ReportSafeBrowsingHit, - resource.url, - page_url, - referrer_url, - is_subresource, - resource.threat_type)); + ReportSafeBrowsingHit(resource.url, page_url, referrer_url, is_subresource, + resource.threat_type); } SafeBrowsingBlockingPage::ShowBlockingPage(this, resource); } -// A safebrowsing hit is sent right after we create a blocking page, -// only for UMA users. +// A safebrowsing hit is sent after a blocking page for malware/phishing +// or after the warning dialog for download urls, only for UMA users. void SafeBrowsingService::ReportSafeBrowsingHit( const GURL& malicious_url, const GURL& page_url, const GURL& referrer_url, bool is_subresource, SafeBrowsingService::UrlCheckResult threat_type) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (!CanReportStats()) + return; + + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, + NewRunnableMethod( + this, + &SafeBrowsingService::ReportSafeBrowsingHitOnIOThread, + malicious_url, + page_url, + referrer_url, + is_subresource, + threat_type)); +} + +void SafeBrowsingService::ReportSafeBrowsingHitOnIOThread( + const GURL& malicious_url, + const GURL& page_url, + const GURL& referrer_url, + bool is_subresource, + SafeBrowsingService::UrlCheckResult threat_type) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); if (!enabled_) return; diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h index 396df13..cd0d5e2 100644 --- a/chrome/browser/safe_browsing/safe_browsing_service.h +++ b/chrome/browser/safe_browsing/safe_browsing_service.h @@ -224,6 +224,14 @@ class SafeBrowsingService // so the service can serialize and send MalwareDetails. virtual void ReportMalwareDetails(scoped_refptr<MalwareDetails> details); + // Report hits to the unsafe contents (malware, phishing, unsafe download URL) + // to the server. Can only be called on UI thread. + void ReportSafeBrowsingHit(const GURL& malicious_url, + const GURL& page_url, + const GURL& referrer_url, + bool is_subresource, + UrlCheckResult threat_type); + protected: // Creates the safe browsing service. Need to initialize before using. SafeBrowsingService(); @@ -331,13 +339,12 @@ class SafeBrowsingService // Invoked on the UI thread to show the blocking page. void DoDisplayBlockingPage(const UnsafeResource& resource); - // As soon as we create a blocking page, we schedule this method to - // report hits to the malware or phishing list to the server. - void ReportSafeBrowsingHit(const GURL& malicious_url, - const GURL& page_url, - const GURL& referrer_url, - bool is_subresource, - UrlCheckResult threat_type); + // Call protocol manager on IO thread to report hits of unsafe contents. + void ReportSafeBrowsingHitOnIOThread(const GURL& malicious_url, + const GURL& page_url, + const GURL& referrer_url, + bool is_subresource, + UrlCheckResult threat_type); // Checks the download hash on safe_browsing_thread_. void CheckDownloadHashOnSBThread(SafeBrowsingCheck* check); diff --git a/chrome/browser/ui/cocoa/download/download_item_controller.mm b/chrome/browser/ui/cocoa/download/download_item_controller.mm index 4af42f2..60eb845 100644 --- a/chrome/browser/ui/cocoa/download/download_item_controller.mm +++ b/chrome/browser/ui/cocoa/download/download_item_controller.mm @@ -153,11 +153,6 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu { frameOrigin.x += widthChange; [buttonTweaker_ setFrameOrigin:frameOrigin]; - ResourceBundle& rb = ResourceBundle::GetSharedInstance(); - NSImage* alertIcon = rb.GetNativeImageNamed(IDR_WARNING); - DCHECK(alertIcon); - [image_ setImage:alertIcon]; - bridge_->LoadIcon(); [self updateToolTip]; } @@ -169,47 +164,68 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu { if (downloadModel->download()->safety_state() == DownloadItem::DANGEROUS) { [self setState:kDangerous]; + ResourceBundle& rb = ResourceBundle::GetSharedInstance(); NSString* dangerousWarning; NSString* confirmButtonTitle; - // The dangerous download label and button text are different for an - // extension file. - if (downloadModel->download()->is_extension_install()) { + NSImage* alertIcon; + + // The dangerous download label, button text and icon are different under + // different cases. + if (downloadModel->download()->danger_type() == + DownloadItem::DANGEROUS_URL) { + // Safebrowsing shows the download URL leads to malicious file. + alertIcon = rb.GetNativeImageNamed(IDR_SAFEBROWSING_WARNING); dangerousWarning = l10n_util::GetNSStringWithFixup( - IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION); - confirmButtonTitle = l10n_util::GetNSStringWithFixup( - IDS_CONTINUE_EXTENSION_DOWNLOAD); + IDS_PROMPT_UNSAFE_DOWNLOAD_URL); + confirmButtonTitle = l10n_util::GetNSStringWithFixup(IDS_SAVE_DOWNLOAD); } else { - // This basic fixup copies Windows DownloadItemView::DownloadItemView(). - - // Extract the file extension (if any). - FilePath filename(downloadModel->download()->target_name()); - FilePath::StringType extension = filename.Extension(); - - // Remove leading '.' from the extension - if (extension.length() > 0) - extension = extension.substr(1); - - // Elide giant extensions. - if (extension.length() > kFileNameMaxLength / 2) { - string16 utf16_extension; - ui::ElideString(UTF8ToUTF16(extension), kFileNameMaxLength / 2, - &utf16_extension); - extension = UTF16ToUTF8(utf16_extension); + // It's a dangerous file type (e.g.: an executable). + DCHECK_EQ(downloadModel->download()->danger_type(), + DownloadItem::DANGEROUS_FILE); + alertIcon = rb.GetNativeImageNamed(IDR_WARNING); + if (downloadModel->download()->is_extension_install()) { + dangerousWarning = l10n_util::GetNSStringWithFixup( + IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION); + confirmButtonTitle = l10n_util::GetNSStringWithFixup( + IDS_CONTINUE_EXTENSION_DOWNLOAD); + } else { + // This basic fixup copies Windows DownloadItemView::DownloadItemView(). + + // Extract the file extension (if any). + FilePath filename(downloadModel->download()->target_name()); + FilePath::StringType extension = filename.Extension(); + + // Remove leading '.' from the extension + if (extension.length() > 0) + extension = extension.substr(1); + + // Elide giant extensions. + if (extension.length() > kFileNameMaxLength / 2) { + string16 utf16_extension; + ui::ElideString(UTF8ToUTF16(extension), kFileNameMaxLength / 2, + &utf16_extension); + extension = UTF16ToUTF8(utf16_extension); + } + + // Rebuild the filename.extension. + string16 rootname = UTF8ToUTF16(filename.RemoveExtension().value()); + ui::ElideString(rootname, kFileNameMaxLength - extension.length(), + &rootname); + std::string new_filename = UTF16ToUTF8(rootname); + if (extension.length()) + new_filename += std::string(".") + extension; + + dangerousWarning = l10n_util::GetNSStringFWithFixup( + IDS_PROMPT_DANGEROUS_DOWNLOAD, UTF8ToUTF16(new_filename)); + confirmButtonTitle = + l10n_util::GetNSStringWithFixup(IDS_SAVE_DOWNLOAD); } - - // Rebuild the filename.extension. - string16 rootname = UTF8ToUTF16(filename.RemoveExtension().value()); - ui::ElideString(rootname, kFileNameMaxLength - extension.length(), - &rootname); - std::string new_filename = UTF16ToUTF8(rootname); - if (extension.length()) - new_filename += std::string(".") + extension; - - dangerousWarning = l10n_util::GetNSStringFWithFixup( - IDS_PROMPT_DANGEROUS_DOWNLOAD, UTF8ToUTF16(new_filename)); - confirmButtonTitle = l10n_util::GetNSStringWithFixup(IDS_SAVE_DOWNLOAD); } + DCHECK(alertIcon); + [image_ setImage:alertIcon]; + DCHECK(dangerousWarning); [dangerousDownloadLabel_ setStringValue:dangerousWarning]; + DCHECK(confirmButtonTitle); [dangerousDownloadConfirmButton_ setTitle:confirmButtonTitle]; return; } diff --git a/chrome/browser/ui/gtk/download_item_gtk.cc b/chrome/browser/ui/gtk/download_item_gtk.cc index 27a2e06..a1d56f4 100644 --- a/chrome/browser/ui/gtk/download_item_gtk.cc +++ b/chrome/browser/ui/gtk/download_item_gtk.cc @@ -575,32 +575,35 @@ void DownloadItemGtk::UpdateStatusLabel(const std::string& status_text) { void DownloadItemGtk::UpdateDangerWarning() { if (dangerous_prompt_) { + UpdateDangerIcon(); + // We create |dangerous_warning| as a wide string so we can more easily // calculate its length in characters. string16 dangerous_warning; - if (get_download()->is_extension_install()) { - dangerous_warning = - l10n_util::GetStringUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION); - } else { - string16 elided_filename = ui::ElideFilename( - get_download()->target_name(), gfx::Font(), kTextWidth); + // The dangerous download label text is different for different cases. + if (get_download()->danger_type() == DownloadItem::DANGEROUS_URL) { + // Safebrowsing shows the download URL leads to malicious file. dangerous_warning = - l10n_util::GetStringFUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD, - elided_filename); + l10n_util::GetStringUTF16(IDS_PROMPT_UNSAFE_DOWNLOAD_URL); + } else { + // It's a dangerous file type (e.g.: an executable). + DCHECK(get_download()->danger_type() == DownloadItem::DANGEROUS_FILE); + if (get_download()->is_extension_install()) { + dangerous_warning = + l10n_util::GetStringUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION); + } else { + string16 elided_filename = ui::ElideFilename( + get_download()->target_name(), gfx::Font(), kTextWidth); + dangerous_warning = + l10n_util::GetStringFUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD, + elided_filename); + } } if (theme_provider_->UseGtkTheme()) { - gtk_image_set_from_stock(GTK_IMAGE(dangerous_image_), - GTK_STOCK_DIALOG_WARNING, GTK_ICON_SIZE_SMALL_TOOLBAR); - gtk_util::SetLabelColor(dangerous_label_, NULL); } else { - // Set the warning icon. - ResourceBundle& rb = ResourceBundle::GetSharedInstance(); - GdkPixbuf* download_pixbuf = rb.GetPixbufNamed(IDR_WARNING); - gtk_image_set_from_pixbuf(GTK_IMAGE(dangerous_image_), download_pixbuf); - GdkColor color = theme_provider_->GetGdkColor( BrowserThemeProvider::COLOR_BOOKMARK_TEXT); gtk_util::SetLabelColor(dangerous_label_, &color); @@ -632,6 +635,24 @@ void DownloadItemGtk::UpdateDangerWarning() { } } +void DownloadItemGtk::UpdateDangerIcon() { + if (theme_provider_->UseGtkTheme()) { + const char* stock = + get_download()->danger_type() == DownloadItem::DANGEROUS_URL ? + GTK_STOCK_DIALOG_ERROR : GTK_STOCK_DIALOG_WARNING; + gtk_image_set_from_stock( + GTK_IMAGE(dangerous_image_), stock, GTK_ICON_SIZE_SMALL_TOOLBAR); + } else { + // Set the warning icon. + ResourceBundle& rb = ResourceBundle::GetSharedInstance(); + int pixbuf_id = + get_download()->danger_type() == DownloadItem::DANGEROUS_URL ? + IDR_SAFEBROWSING_WARNING : IDR_WARNING; + GdkPixbuf* download_pixbuf = rb.GetPixbufNamed(pixbuf_id); + gtk_image_set_from_pixbuf(GTK_IMAGE(dangerous_image_), download_pixbuf); + } +} + // static void DownloadItemGtk::InitNineBoxes() { if (body_nine_box_normal_) diff --git a/chrome/browser/ui/gtk/download_item_gtk.h b/chrome/browser/ui/gtk/download_item_gtk.h index d8fd8ae..68d605c 100644 --- a/chrome/browser/ui/gtk/download_item_gtk.h +++ b/chrome/browser/ui/gtk/download_item_gtk.h @@ -96,6 +96,9 @@ class DownloadItemGtk : public DownloadItem::Observer, // Sets the components of the danger warning. void UpdateDangerWarning(); + // Sets the icon for the danger warning dialog. + void UpdateDangerIcon(); + static void InitNineBoxes(); // Draws everything in GTK rendering mode. diff --git a/chrome/browser/ui/views/download_item_view.cc b/chrome/browser/ui/views/download_item_view.cc index c7f2106..8cdc4f4 100644 --- a/chrome/browser/ui/views/download_item_view.cc +++ b/chrome/browser/ui/views/download_item_view.cc @@ -244,8 +244,6 @@ DownloadItemView::DownloadItemView(DownloadItem* download, tooltip_text_.clear(); body_state_ = DANGEROUS; drop_down_state_ = DANGEROUS; - - warning_icon_ = rb.GetBitmapNamed(IDR_WARNING); save_button_ = new views::NativeButton(this, UTF16ToWide(l10n_util::GetStringUTF16( download->is_extension_install() ? @@ -283,20 +281,33 @@ DownloadItemView::DownloadItemView(DownloadItem* download, if (extension.length() > kFileNameMaxLength / 2) ui::ElideString(extension, kFileNameMaxLength / 2, &extension); - // The dangerous download label text is different for an extension file. - if (download->is_extension_install()) { - dangerous_download_label_ = new views::Label(UTF16ToWide( - l10n_util::GetStringUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION))); + // The dangerous download label text and icon are different + // under different cases. + string16 dangerous_label; + if (download->danger_type() == DownloadItem::DANGEROUS_URL) { + // Safebrowsing shows the download URL leads to malicious file. + warning_icon_ = rb.GetBitmapNamed(IDR_SAFEBROWSING_WARNING); + dangerous_label = + l10n_util::GetStringUTF16(IDS_PROMPT_UNSAFE_DOWNLOAD_URL); } else { - ui::ElideString(rootname, - kFileNameMaxLength - extension.length(), - &rootname); - string16 filename = rootname + ASCIIToUTF16(".") + extension; - filename = base::i18n::GetDisplayStringInLTRDirectionality(filename); - dangerous_download_label_ = new views::Label(UTF16ToWide( - l10n_util::GetStringFUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD, - filename))); + // The download file has dangerous file type (e.g.: an executable). + DCHECK(download->danger_type() == DownloadItem::DANGEROUS_FILE); + warning_icon_ = rb.GetBitmapNamed(IDR_WARNING); + if (download->is_extension_install()) { + dangerous_label = + l10n_util::GetStringUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION); + } else { + ui::ElideString(rootname, + kFileNameMaxLength - extension.length(), + &rootname); + string16 filename = rootname + ASCIIToUTF16(".") + extension; + filename = base::i18n::GetDisplayStringInLTRDirectionality(filename); + dangerous_label = + l10n_util::GetStringFUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD, filename); + } } + + dangerous_download_label_ = new views::Label(UTF16ToWide(dangerous_label)); dangerous_download_label_->SetMultiLine(true); dangerous_download_label_->SetHorizontalAlignment( views::Label::ALIGN_LEFT); diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index a9d0897..fdaaf89 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -850,6 +850,8 @@ 'browser/download/download_request_infobar_delegate.h', 'browser/download/download_request_limiter.cc', 'browser/download/download_request_limiter.h', + 'browser/download/download_safe_browsing_client.cc', + 'browser/download/download_safe_browsing_client.h', 'browser/download/download_shelf.cc', 'browser/download/download_shelf.h', 'browser/download/download_started_animation.h', |