[Downloads] Gracefully handle SafeBrowsing check failures.

chrome/browser/download/chrome_download_manager_delegate.cc

 // Copyright (c) 2012 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 <string>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 141 matching lines @@
 // Danger type is (in order of preference):
 //   * DANGEROUS_URL, if the URL is a known malware site.
 //   * MAYBE_DANGEROUS_CONTENT, if the content will be scanned for
 //         malware. I.e. |is_content_check_supported| is true.
 //   * NOT_DANGEROUS.
 void CheckDownloadUrlDone(
     const DownloadTargetDeterminerDelegate::CheckDownloadUrlCallback& callback,
     bool is_content_check_supported,
     DownloadProtectionService::DownloadCheckResult result) {
   content::DownloadDangerType danger_type;
-  if (result == DownloadProtectionService::SAFE) {
+  if (result == DownloadProtectionService::SAFE ||
+      result == DownloadProtectionService::UNKNOWN) {
     // If this type of files is handled by the enhanced SafeBrowsing download
     // protection, mark it as potentially dangerous content until we are done
     // with scanning it.
     if (is_content_check_supported)
       danger_type = content::DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT;
     else
       danger_type = content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS;
   } else {
     // If the URL is malicious, we'll use that as the danger type. The results
     // of the content check, if one is performed, will be ignored.
@@ skipping 127 matching lines @@
 bool ChromeDownloadManagerDelegate::IsDownloadReadyForCompletion(
     DownloadItem* item,
     const base::Closure& internal_complete_callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 #if defined(FULL_SAFE_BROWSING)
   SafeBrowsingState* state = static_cast<SafeBrowsingState*>(
       item->GetUserData(&kSafeBrowsingUserDataKey));
   if (!state) {
     // Begin the safe browsing download protection check.
     DownloadProtectionService* service = GetDownloadProtectionService();
     if (service) {

[mattm, 2014/09/12 02:52:39]

I think there may be a minor race here if the file danger_type is set to
DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT and then the user disables
safebrowsing before the download completes, the case in CheckClientDownloadDone
that resets it to DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE wouldn't happen.

Is that handled else where, or should it also check for
item.IsDangerousFileBasedOnType() if (!service)? Or is it too small an issue to
worry about?

       VLOG(2) << __FUNCTION__ << "() Start SB download check for download = "
               << item->DebugString(false);
       state = new SafeBrowsingState();
       state->set_callback(internal_complete_callback);
       item->SetUserData(&kSafeBrowsingUserDataKey, state);
       service->CheckClientDownload(
           item,
           base::Bind(
               &ChromeDownloadManagerDelegate::CheckClientDownloadDone,
               weak_ptr_factory_.GetWeakPtr(),
@@ skipping 308 matching lines @@
   VLOG(2) << __FUNCTION__ << "() download = " << item->DebugString(false)
           << " verdict = " << result;
   // We only mark the content as being dangerous if the download's safety state
   // has not been set to DANGEROUS yet.  We don't want to show two warnings.
   if (item->GetDangerType() == content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS ||
       item->GetDangerType() ==
       content::DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT) {
     content::DownloadDangerType danger_type =
         content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS;
     switch (result) {
+      case DownloadProtectionService::UNKNOWN:
+        // The check failed or was inconclusive.
+        if (DownloadItemModel(item).IsDangerousFileBasedOnType())
+          danger_type = content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE;
+        break;
       case DownloadProtectionService::SAFE:
         // Do nothing.
         break;
       case DownloadProtectionService::DANGEROUS:
         danger_type = content::DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT;
         break;
       case DownloadProtectionService::UNCOMMON:
         danger_type = content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT;
         break;
       case DownloadProtectionService::DANGEROUS_HOST:
@@ skipping 32 matching lines @@
   callback.Run(installer->did_handle_successfully());
 #endif
 }
 
 void ChromeDownloadManagerDelegate::OnDownloadTargetDetermined(
     int32 download_id,
     const content::DownloadTargetCallback& callback,
     scoped_ptr<DownloadTargetInfo> target_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* item = download_manager_->GetDownload(download_id);
-  if (!target_info->target_path.empty() && item &&
-      IsOpenInBrowserPreferreredForFile(target_info->target_path) &&
-      target_info->is_filetype_handled_safely)
-    DownloadItemModel(item).SetShouldPreferOpeningInBrowser(true);
+  if (item) {
+    if (!target_info->target_path.empty() &&
+        IsOpenInBrowserPreferreredForFile(target_info->target_path) &&
+        target_info->is_filetype_handled_safely)
+      DownloadItemModel(item).SetShouldPreferOpeningInBrowser(true);
+
+    if (target_info->is_dangerous_file)
+      DownloadItemModel(item).SetIsDangerousFileBasedOnType(true);
+  }
   callback.Run(target_info->target_path,
                target_info->target_disposition,
                target_info->danger_type,
                target_info->intermediate_path);
 }
 
 bool ChromeDownloadManagerDelegate::IsOpenInBrowserPreferreredForFile(
     const base::FilePath& path) {
   // On Windows, PDFs should open in Acrobat Reader if the user chooses.
 #if defined(OS_WIN)
@@ skipping 17 matching lines @@
       path.MatchesExtension(FILE_PATH_LITERAL(".xht")) ||
       path.MatchesExtension(FILE_PATH_LITERAL(".xhtm")) ||
       path.MatchesExtension(FILE_PATH_LITERAL(".xhtml")) ||
       path.MatchesExtension(FILE_PATH_LITERAL(".xsl")) ||
       path.MatchesExtension(FILE_PATH_LITERAL(".xslt"))) {
     return true;
   }
 #endif
   return false;
 }