[SafeBrowsing] Send pings for Zip files that contain other archives.

chrome/browser/safe_browsing/download_protection_service_unittest.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/safe_browsing/download_protection_service.h"
 
 #include <stdint.h>
 #include <map>
 #include <string>
 
@@ skipping 1060 matching lines @@
   MessageLoop::current()->Run();
 
 #if defined(OS_WIN) || defined(OS_MACOSX)
   EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
   EXPECT_TRUE(HasClientDownloadRequest());
   const ClientDownloadRequest& request = *GetClientDownloadRequest();
   EXPECT_EQ(1, request.archived_binary_size());
   const ClientDownloadRequest_ArchivedBinary* archived_binary =
       GetRequestArchivedBinary(request, "file.exe");
   ASSERT_NE(nullptr, archived_binary);
   EXPECT_EQ(ClientDownloadRequest_DownloadType_WIN_EXECUTABLE,

[mattm, 2015/07/29 22:53:25]

Seems like this case should also be checking request->download_type() ==
ZIPPED_EXECUTABLE.

[asanka, 2015/07/31 01:06:33]

This is looking at the download type of the binary inside the archive. That's
going to be WIN_EXECUTABLE. The download type for the outer zip file is
ZIPPED_EXECUTABLE.

             archived_binary->download_type());
   EXPECT_EQ(static_cast<int64_t>(file_contents.size()),
             archived_binary->length());
   ClearClientDownloadRequest();
 #else
   // For !(OS_WIN || OS_MACOSX), no file types are currently supported. Hence
   // the resulting verdict is UNKNOWN.
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
   EXPECT_FALSE(HasClientDownloadRequest());
 #endif
@@ skipping 15 matching lines @@
 
 #if defined(OS_WIN) || defined(OS_MACOSX)
   EXPECT_TRUE(IsResult(DownloadProtectionService::DANGEROUS));
   EXPECT_TRUE(HasClientDownloadRequest());
   ClearClientDownloadRequest();
 #else
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
   EXPECT_FALSE(HasClientDownloadRequest());
 #endif
   Mock::VerifyAndClearExpectations(binary_feature_extractor_.get());
+
+  // Repeat the test with an archive inside the zip file.
+  ASSERT_EQ(static_cast<int>(file_contents.size()), base::WriteFile(
+      zip_source_dir.path().Append(FILE_PATH_LITERAL("file.rar")),
+      file_contents.data(), file_contents.size()));
+  ASSERT_TRUE(zip::Zip(zip_source_dir.path(), a_tmp, false));

[mattm, 2015/07/29 22:53:25]

Isn't this creating a zip with file.txt, file.exe, and file.rar? Shouldn't the
result below still be ZIPPED_EXECUTABLE since it contains both an exe and a rar
and the exe takes precedence?

[asanka, 2015/07/31 01:04:42]

Indeed. Fixed by testing both with file.exe and without.

+
+  EXPECT_CALL(*sb_service_->mock_database_manager(),
+              MatchDownloadWhitelistUrl(_))
+      .WillRepeatedly(Return(false));
+
+  download_service_->CheckClientDownload(
+      &item,
+      base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
+                 base::Unretained(this)));
+  MessageLoop::current()->Run();
+
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
+  EXPECT_TRUE(HasClientDownloadRequest());
+  EXPECT_EQ(0, GetClientDownloadRequest()->archived_binary_size());
+  EXPECT_TRUE(GetClientDownloadRequest()->has_download_type());
+  EXPECT_EQ(ClientDownloadRequest_DownloadType_ZIPPED_ARCHIVE,
+            GetClientDownloadRequest()->download_type());
+  ClearClientDownloadRequest();
+#else
+  // For !(OS_WIN || OS_MACOSX), no file types are currently supported. Hence
+  // the resulting verdict is UNKNOWN.
+  EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
+  Mock::VerifyAndClearExpectations(binary_feature_extractor_.get());
 }
 
 TEST_F(DownloadProtectionServiceTest, CheckClientDownloadCorruptZip) {
   base::ScopedTempDir download_dir;
   ASSERT_TRUE(download_dir.CreateUniqueTempDir());
 
   base::FilePath a_tmp(download_dir.path().Append(FILE_PATH_LITERAL("a.tmp")));
   base::FilePath a_zip(FILE_PATH_LITERAL("a.zip"));
   std::vector<GURL> url_chain;
   url_chain.push_back(GURL("http://www.evil.com/a.zip"));
@@ skipping 762 matching lines @@
 
   EXPECT_CALL(mock_download_item, GetDangerType())
       .WillOnce(Return(content::DOWNLOAD_DANGER_TYPE_DANGEROUS_HOST));
   EXPECT_CALL(mock_page_navigator, OpenURL(OpenURLParamsWithContextValue("7")));
 
   download_service_->ShowDetailsForDownload(mock_download_item,
                                             &mock_page_navigator);
 }
 
 }  // namespace safe_browsing