Don't prompt to save malicious downloads on exit

chrome/browser/lifetime/browser_close_manager_browsertest.cc

 // Copyright 2013 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 <vector>
 
 #include "base/command_line.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_shutdown.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ skipping 13 matching lines @@
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/download_test_observer.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "content/test/net/url_request_mock_http_job.h"
 #include "content/test/net/url_request_slow_download_job.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 
 namespace {
 
 class AppModalDialogObserver {
  public:
   AppModalDialogObserver() {}
 
   void Start() {
@@ skipping 70 matching lines @@
   base::RunLoop run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(RepeatedNotificationObserver);
 };
 
 class TestBrowserCloseManager : public BrowserCloseManager {
  public:
   enum UserChoice {
     USER_CHOICE_USER_CANCELS_CLOSE,
     USER_CHOICE_USER_ALLOWS_CLOSE,
+    NO_USER_CHOICE
   };
 
   static void AttemptClose(UserChoice user_choice) {
     scoped_refptr<BrowserCloseManager> browser_close_manager =
         new TestBrowserCloseManager(user_choice);
     browser_close_manager->StartClosingBrowsers();
   }
 
  protected:
   virtual ~TestBrowserCloseManager() {}
 
   virtual void ConfirmCloseWithPendingDownloads(
       int download_count,
       const base::Callback<void(bool)>& callback) OVERRIDE {
+    EXPECT_NE(NO_USER_CHOICE, user_choice_);
     switch (user_choice_) {
+      case NO_USER_CHOICE:
       case USER_CHOICE_USER_CANCELS_CLOSE: {
         callback.Run(false);
         break;
       }
       case USER_CHOICE_USER_ALLOWS_CLOSE: {
         callback.Run(true);
         break;
       }
     }
   }
@@ skipping 463 matching lines @@
   chrome::CloseAllBrowsers();
   ASSERT_FALSE(browsers_[0]->ShouldCloseWindow());
   ASSERT_NO_FATAL_FAILURE(dialogs_.AcceptClose());
   ASSERT_NO_FATAL_FAILURE(dialogs_.AcceptClose());
 
   close_observer.Wait();
   EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
   EXPECT_TRUE(chrome::BrowserIterator().done());
 }
 
+// Test shutdown with a DANGEROUS_URL download undecided.
+IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest,
+    TestWithDangerousUrlDownload) {
+  ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+
+  // Run a dangerous download, but the user doesn't make a decision.
+  // This test uses a .swf because it will trigger a dangerous download
+  // warning on every platform.
+  base::FilePath file(FILE_PATH_LITERAL("downloads/dangerous/dangerous.swf"));
+  GURL download_url(content::URLRequestMockHTTPJob::GetMockUrl(file));
+  content::DownloadTestObserverInterrupted observer(
+      content::BrowserContext::GetDownloadManager(browser()->profile()),
+      1,
+      content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_QUIT);
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      GURL(download_url),
+      NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_NONE);
+  observer.WaitForFinished();
+
+  // Change the type of the dangerous download to DANGEROUS_URL, since the
+  // browser still prompts for DANGEROUS_FILE.
+  std::vector<content::DownloadItem*> download_vector;
+  content::BrowserContext::GetDownloadManager(browser()->profile())->
+      GetAllDownloads(&download_vector);
+  EXPECT_EQ(1u, download_vector.size());
+  download_vector.at(0)->SetDangerType(
+      content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL);

[asanka, 2013/10/15 21:39:08]

It would be great if we didn't add a public interface to DownloadItem for this
test. A SetDangerType() did exist once and we removed it because allowing the
danger type to change at arbitrary points in time made it difficult to reason
about the state transitions for a download. So we now allow the danger type to
be set at two points in time: during file name determination using
content::DownloadTargetCallback, and after all the data has been downloaded
using DownloadItem::OnContentCheckCompleted().

It's more code, but the cleanest way to test this, IMO, is to inject a
ChromeDownloadManagerDelegate subclass that overrides
DownloadManagerDelegate::DetermineDownloadTarget() to:
- Wrap the incoming DownloadTargetCallback with one that sets the danger type to
DOWNLOAD_DANGER_TYPE_DANGEROUS_URL.
- invokes CDMD::DetermineDownloadTarget() with the wrapped callback.

You can then set this CDMD subclass as the DownloadManagerDelegate to use using
DownloadService::SetDownloadManagerDelegateForTesting().

This way, the download will behave the same as one which was flagged by
SafeBrowsing as DANGEROUS_URL.

Failing that, the SetDangerType could be renamed to SetDangerTypeForTesting().
However, calling it on all platforms in this case is only safe because the
download was already marked as DANGEROUS_FILE. Setting it to DANGEROUS_URL
doesn't change the IsDangerous() state.

+
+  // Check that the download manager has the expected state.
+  EXPECT_EQ(1, content::BrowserContext::GetDownloadManager(
+      browser()->profile())->InProgressCount());
+  EXPECT_EQ(0, content::BrowserContext::GetDownloadManager(
+      browser()->profile())->NonDangerousInProgressCount());
+
+  // Close the browser with no user action.
+  RepeatedNotificationObserver close_observer(
+      chrome::NOTIFICATION_BROWSER_CLOSED, 1);
+  TestBrowserCloseManager::AttemptClose(
+      TestBrowserCloseManager::NO_USER_CHOICE);
+  close_observer.Wait();
+  EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
+  EXPECT_TRUE(chrome::BrowserIterator().done());
+}
+
 // Test shutdown with a download in progress.
 IN_PROC_BROWSER_TEST_P(BrowserCloseManagerBrowserTest, TestWithDownloads) {
   ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
   ASSERT_NO_FATAL_FAILURE(CreateStalledDownload(browser()));
   content::TestNavigationObserver navigation_observer(
       browser()->tab_strip_model()->GetActiveWebContents(), 1);
   TestBrowserCloseManager::AttemptClose(
       TestBrowserCloseManager::USER_CHOICE_USER_CANCELS_CLOSE);
   EXPECT_FALSE(browser_shutdown::IsTryingToQuit());
   navigation_observer.Wait();
@@ skipping 79 matching lines @@
   TestBrowserCloseManager::AttemptClose(
       TestBrowserCloseManager::USER_CHOICE_USER_ALLOWS_CLOSE);
   close_observer.Wait();
   EXPECT_TRUE(browser_shutdown::IsTryingToQuit());
   EXPECT_TRUE(chrome::BrowserIterator().done());
 }
 
 INSTANTIATE_TEST_CASE_P(BrowserCloseManagerBrowserTest,
                         BrowserCloseManagerBrowserTest,
                         testing::Bool());