Propagate the SafeBrowsing download protection verdict to the DownloadItem.

chrome/browser/download/chrome_download_manager_delegate.cc

 // 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/chrome_download_manager_delegate.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/file_util.h"
@@ skipping 69 matching lines @@
   // insert the new DownloadItem into 'history_downloads_' or inform our
   // observers at this point. OnCreateDownloadEntryComplete() handles that
   // finalization of the the download creation as a callback from the history
   // thread.
   DownloadItem* download =
       download_manager_->GetActiveDownloadItem(download_id);
   if (!download)
     return false;
 
 #if defined(ENABLE_SAFE_BROWSING)
-  SafeBrowsingService* sb_service =
-      g_browser_process->safe_browsing_service();
-  if (sb_service && sb_service->download_protection_service() &&
-      profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
+  DownloadProtectionService* service = download_protection_service();
+  if (service) {
     VLOG(2) << __FUNCTION__ << "() Start SB URL check for download = "
             << download->DebugString(false);
-    sb_service->download_protection_service()->CheckDownloadUrl(
+    service->CheckDownloadUrl(
         DownloadProtectionService::DownloadInfo::FromDownloadItem(*download),
         base::Bind(
             &ChromeDownloadManagerDelegate::CheckDownloadUrlDone,
             this,
             download->id()));
     return false;
   }
 #endif
   CheckDownloadUrlDone(download_id, DownloadProtectionService::SAFE);
   return false;
 }
 
 void ChromeDownloadManagerDelegate::ChooseDownloadPath(
     TabContents* tab_contents,
     const FilePath& suggested_path,
     void* data) {
   // Deletes itself.
   new DownloadFilePicker(
       download_manager_, tab_contents, suggested_path, data);
 }
 
 bool ChromeDownloadManagerDelegate::OverrideIntermediatePath(
     DownloadItem* item,
     FilePath* intermediate_path) {
-  if (item->IsDangerous()) {
-    // The download is not safe.  It's name is already set to an intermediate
-    // name, so no need to override.
+  if (item->needs_quarantine_file()) {
+    // The download might not be safe.  It's name is already set to an
+    // intermediate name, so no need to override.
     return false;
   }
 
   // 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::OnDownloadCompleting.
   *intermediate_path = download_util::GetCrDownloadPath(item->full_path());
   return true;
 }
 
@@ skipping 23 matching lines @@
   // See if there is already a pending SafeBrowsing check for that download.
   SafeBrowsingStateMap::iterator it = safe_browsing_state_.find(item->id());
   if (it != safe_browsing_state_.end()) {
     SafeBrowsingState state = it->second;
     if (!state.pending) {
       safe_browsing_state_.erase(it);
     }
     return !state.pending;
   }
   // Begin the safe browsing download protection check.
-  SafeBrowsingService* sb_service =
-      g_browser_process->safe_browsing_service();
-  if (sb_service && sb_service->download_protection_service() &&
-      profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
+  DownloadProtectionService* service = download_protection_service();
+  if (service) {
     VLOG(2) << __FUNCTION__ << "() Start SB download check for download = "
             << item->DebugString(false);
-    sb_service->download_protection_service()->CheckClientDownload(
+    service->CheckClientDownload(
         DownloadProtectionService::DownloadInfo::FromDownloadItem(*item),
         base::Bind(
             &ChromeDownloadManagerDelegate::CheckClientDownloadDone,
             this,
             item->id()));
     SafeBrowsingState state;
     state.pending = true;
     state.verdict = DownloadProtectionService::SAFE;
     safe_browsing_state_[item->id()] = state;
     return false;
@@ skipping 108 matching lines @@
 
   float progress = 0;
   int download_count = 0;
   bool progress_known =
       g_browser_process->download_status_updater()->GetProgress(
           &progress, &download_count);
   download_util::UpdateAppIconDownloadProgress(
       download_count, progress_known, progress);
 }
 
+DownloadProtectionService*
+    ChromeDownloadManagerDelegate::download_protection_service() {

[asanka, 2011/11/16 03:23:48]

Nit: This is not a trivial accessor.

[noelutz, 2011/11/16 03:50:26]

Done.

+#if defined(ENABLE_SAFE_BROWSING)
+  SafeBrowsingService* sb_service = g_browser_process->safe_browsing_service();
+  if (sb_service && sb_service->download_protection_service() &&
+      profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
+    return sb_service->download_protection_service();
+  }
+#endif
+  return NULL;
+}
+
 void ChromeDownloadManagerDelegate::CheckDownloadUrlDone(
     int32 download_id,
     DownloadProtectionService::DownloadCheckResult result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* download =
       download_manager_->GetActiveDownloadItem(download_id);
   if (!download)
     return;
 
   VLOG(2) << __FUNCTION__ << "() download = " << download->DebugString(false)
@@ skipping 11 matching lines @@
     int32 download_id,
     DownloadProtectionService::DownloadCheckResult result) {
   DownloadItem* item = download_manager_->GetActiveDownloadItem(download_id);
   if (!item) {
     safe_browsing_state_.erase(download_id);  // Just in case.
     return;
   }
 
   VLOG(2) << __FUNCTION__ << "() download = " << item->DebugString(false)
           << " verdict = " << result;
-  // TODO(noelutz):
-  //  1) display a warning if the result is DANGEROUS.
-  //  2) make sure we haven't already displayed a warning for the URL.
-  //  3) disable the existing dangerous file warning for executables.
+  // TODO(noelutz): ensure that URL warning was not already shown.

[asanka, 2011/11/16 03:23:48]

Shall we, instead, ensure that the DownloadItem is only marked as (any kind of)
dangerous only once? Otherwise it would be difficult to determine what needs to
be done when an observer notices that a DI was marked as dangerous.

[noelutz, 2011/11/16 03:50:26]

Much easier. Done.

+  if (result == DownloadProtectionService::DANGEROUS)
+    item->MarkContentDangerous();
 
   SafeBrowsingStateMap::iterator it = safe_browsing_state_.find(item->id());
   DCHECK(it != safe_browsing_state_.end() && it->second.pending);
   if (it != safe_browsing_state_.end()) {
     it->second.pending = false;
     it->second.verdict = result;
   }
   download_manager_->MaybeCompleteDownload(item);
 }
 
@@ skipping 64 matching lines @@
       state.suggested_path = download_prefs_->download_path();
     }
     state.suggested_path = state.suggested_path.Append(generated_name);
   } else {
     state.suggested_path = state.force_file_name;
   }
 
   if (!state.prompt_user_for_save_location && state.force_file_name.empty()) {
     state.is_dangerous_file =
         IsDangerousFile(*download, state, visited_referrer_before);
+    state.needs_quarantine_file = true;
   }
 
+#if defined(ENABLE_SAFE_BROWSING)
+  DownloadProtectionService* service = download_protection_service();
+  // Return false if this type of files is handled by the enhanced SafeBrowsing
+  // download protection.
+  if (service && service->enabled() &&
+      service->IsSupportedFileType(state.suggested_path.BaseName())) {
+    state.is_dangerous_file = false;
+    state.needs_quarantine_file = true;
+  }
+#endif
+
   // We need to move over to the download thread because we don't want to stat
   // the suggested path on the UI thread.
   // We can only access preferences on the UI thread, so check the download path
   // now and pass the value to the FILE thread.
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&ChromeDownloadManagerDelegate::CheckIfSuggestedPathExists,
                  this, download->id(), state,
                  download_prefs_->download_path()));
 }
@@ skipping 14 matching lines @@
   FilePath dir = state.suggested_path.DirName();
   FilePath filename = state.suggested_path.BaseName();
   if (!file_util::PathIsWritable(dir)) {
     VLOG(1) << "Unable to write to directory \"" << dir.value() << "\"";
     state.prompt_user_for_save_location = true;
     PathService::Get(chrome::DIR_USER_DOCUMENTS, &state.suggested_path);
     state.suggested_path = state.suggested_path.Append(filename);
   }
 
   // If the download is deemed dangerous, we'll use a temporary name for it.
-  if (state.IsDangerous()) {
+  if (state.needs_quarantine_file) {
     state.target_name = FilePath(state.suggested_path).BaseName();
     // Create a temporary file to hold the file until the user approves its
     // download.
     FilePath::StringType file_name;
     FilePath path;
 #if defined(OS_WIN)
     string16 unconfirmed_prefix =
         l10n_util::GetStringUTF16(IDS_DOWNLOAD_UNCONFIRMED_PREFIX);
 #else
     std::string unconfirmed_prefix =
@@ skipping 31 matching lines @@
               << state.suggested_path.value() << "\"";
       state.prompt_user_for_save_location = true;
     }
   }
 
   // Create an empty file at the suggested path so that we don't allocate the
   // same "non-existant" path to multiple downloads.
   // See: http://code.google.com/p/chromium/issues/detail?id=3662
   if (!state.prompt_user_for_save_location &&
       state.force_file_name.empty()) {
-    if (state.IsDangerous())
+    if (state.needs_quarantine_file)
       file_util::WriteFile(state.suggested_path, "", 0);
     else
       file_util::WriteFile(download_util::GetCrDownloadPath(
           state.suggested_path), "", 0);
   }
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&ChromeDownloadManagerDelegate::OnPathExistenceAvailable,
                  this, download_id, state));
@@ skipping 14 matching lines @@
 bool ChromeDownloadManagerDelegate::IsDangerousFile(
     const DownloadItem& download,
     const DownloadStateInfo& state,
     bool visited_referrer_before) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Anything loaded directly from the address bar is OK.
   if (state.transition_type & content::PAGE_TRANSITION_FROM_ADDRESS_BAR)
     return false;
 
-  // Return false if this type of files is handled by the enhanced SafeBrowsing
-  // download protection.
-  // TODO(noelutz): implement that check.
-
   // Extensions that are not from the gallery are considered dangerous.
   if (IsExtensionDownload(&download)) {
     ExtensionService* service = profile_->GetExtensionService();
     if (!service || !service->IsDownloadFromGallery(download.GetURL(),
                                                     download.referrer_url()))
       return true;
   }
 
   // Anything the user has marked auto-open is OK if it's user-initiated.
   if (ShouldOpenFileBasedOnExtension(state.suggested_path) &&
@@ skipping 14 matching lines @@
     int32 download_id, int64 db_handle) {
   // It's not immediately obvious, but HistoryBackend::CreateDownload() can
   // call this function with an invalid |db_handle|. For instance, this can
   // happen when the history database is offline. We cannot have multiple
   // DownloadItems with the same invalid db_handle, so we need to assign a
   // unique |db_handle| here.
   if (db_handle == DownloadItem::kUninitializedHandle)
     db_handle = download_history_->GetNextFakeDbHandle();
   download_manager_->OnItemAddedToPersistentStore(download_id, db_handle);
 }