Add test coverage to DeepScanningDialogDelegate::FileContents usage

Done through 2 refactors:
- FileContents is now declared in the header to be used by tests.
- GetSha256Blocking is now in the header as a static function.

Bug: 1041890
Change-Id: I15e6ecf6938324271e2bf018db57d0071d7d0072
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2078973Reviewed-by: Daniel Rubery <drubery@chromium.org>
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745983}

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate.cc

@@ -71,25 +71,11 @@ bool WaitForVerdict() {
   return state == DELAY_UPLOADS || state == DELAY_UPLOADS_AND_DOWNLOADS;
 }
 
-struct FileContents {
-  FileContents() : result(BinaryUploadService::Result::UNKNOWN) {}
-  explicit FileContents(BinaryUploadService::Result result) : result(result) {}
-  FileContents(FileContents&&) = default;
-  FileContents& operator=(FileContents&&) = default;
-
-  BinaryUploadService::Result result;
-  BinaryUploadService::Request::Data data;
-
-  // Store the file size separately instead of using data.contents.size() to
-  // keep track of size for large files.
-  int64_t size = 0;
-  std::string sha256;
-};
-
-FileContents GetFileContentsForLargeFile(const base::FilePath& path,
-                                         base::File* file) {
+DeepScanningDialogDelegate::FileContents GetFileContentsForLargeFile(
+    const base::FilePath& path,
+    base::File* file) {
   size_t file_size = file->GetLength();
-  FileContents file_contents;
+  DeepScanningDialogDelegate::FileContents file_contents;
   file_contents.result = BinaryUploadService::Result::FILE_TOO_LARGE;
   file_contents.size = file_size;
 
@@ -104,7 +90,7 @@ FileContents GetFileContentsForLargeFile(const base::FilePath& path,
         &buf[0], BinaryUploadService::kMaxUploadSizeBytes);
 
     if (bytes_currently_read == -1)
-      return FileContents();
+      return DeepScanningDialogDelegate::FileContents();
 
     secure_hash->Update(buf.data(), bytes_currently_read);
 
@@ -116,10 +102,11 @@ FileContents GetFileContentsForLargeFile(const base::FilePath& path,
   return file_contents;
 }
 
-FileContents GetFileContentsForNormalFile(const base::FilePath& path,
-                                          base::File* file) {
+DeepScanningDialogDelegate::FileContents GetFileContentsForNormalFile(
+    const base::FilePath& path,
+    base::File* file) {
   size_t file_size = file->GetLength();
-  FileContents file_contents;
+  DeepScanningDialogDelegate::FileContents file_contents;
   file_contents.result = BinaryUploadService::Result::SUCCESS;
   file_contents.size = file_size;
   file_contents.data.contents.resize(file_size);
@@ -128,7 +115,7 @@ FileContents GetFileContentsForNormalFile(const base::FilePath& path,
       file->ReadAtCurrentPos(&file_contents.data.contents[0], file_size);
 
   if (bytes_currently_read == -1)
-    return FileContents();
+    return DeepScanningDialogDelegate::FileContents();
 
   DCHECK_EQ(static_cast<size_t>(bytes_currently_read), file_size);
 
@@ -136,18 +123,6 @@ FileContents GetFileContentsForNormalFile(const base::FilePath& path,
   return file_contents;
 }
 
-// Callback used by FileSourceRequest to read file data on a blocking thread.
-FileContents GetFileContentsSHA256Blocking(const base::FilePath& path) {
-  base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ);
-  if (!file.IsValid())
-    return FileContents();
-
-  return static_cast<size_t>(file.GetLength()) >
-                 BinaryUploadService::kMaxUploadSizeBytes
-             ? GetFileContentsForLargeFile(path, &file)
-             : GetFileContentsForNormalFile(path, &file);
-}
-
 // A BinaryUploadService::Request implementation that gets the data to scan
 // from a string.
 class StringSourceRequest : public BinaryUploadService::Request {
@@ -269,7 +244,8 @@ void DeepScanningDialogDelegate::FileSourceRequest::GetRequestData(
     DataCallback callback) {
   base::ThreadPool::PostTaskAndReplyWithResult(
       FROM_HERE, {base::TaskPriority::USER_VISIBLE, base::MayBlock()},
-      base::BindOnce(&GetFileContentsSHA256Blocking, path_),
+      base::BindOnce(&DeepScanningDialogDelegate::GetFileContentsSHA256Blocking,
+                     path_),
       base::BindOnce(&FileSourceRequest::OnGotFileContents,
                      weakptr_factory_.GetWeakPtr(), std::move(callback)));
 }
@@ -296,6 +272,16 @@ DeepScanningDialogDelegate::FileInfo::FileInfo() = default;
 DeepScanningDialogDelegate::FileInfo::FileInfo(FileInfo&& other) = default;
 DeepScanningDialogDelegate::FileInfo::~FileInfo() = default;
 
+DeepScanningDialogDelegate::FileContents::FileContents() = default;
+DeepScanningDialogDelegate::FileContents::FileContents(
+    BinaryUploadService::Result result)
+    : result(result) {}
+
+DeepScanningDialogDelegate::FileContents::FileContents(FileContents&& other) =
+    default;
+DeepScanningDialogDelegate::FileContents&
+DeepScanningDialogDelegate::FileContents::operator=(
+    DeepScanningDialogDelegate::FileContents&& other) = default;
 DeepScanningDialogDelegate::~DeepScanningDialogDelegate() = default;
 
 void DeepScanningDialogDelegate::Cancel() {
@@ -339,6 +325,20 @@ bool DeepScanningDialogDelegate::ResultShouldAllowDataUse(
   }
 }
 
+// static
+DeepScanningDialogDelegate::FileContents
+DeepScanningDialogDelegate::GetFileContentsSHA256Blocking(
+    const base::FilePath& path) {
+  base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ);
+  if (!file.IsValid())
+    return FileContents();
+
+  return static_cast<size_t>(file.GetLength()) >
+                 BinaryUploadService::kMaxUploadSizeBytes
+             ? GetFileContentsForLargeFile(path, &file)
+             : GetFileContentsForNormalFile(path, &file);
+}
+
 // static
 bool DeepScanningDialogDelegate::IsEnabled(Profile* profile,
                                            GURL url,

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate.h

@@ -119,6 +119,22 @@ class DeepScanningDialogDelegate {
     uint64_t size = 0;
   };
 
+  // File contents used as input for |file_info_| and the BinaryUploadService.
+  struct FileContents {
+    FileContents();
+    explicit FileContents(BinaryUploadService::Result result);
+    FileContents(FileContents&&);
+    FileContents& operator=(FileContents&&);
+
+    BinaryUploadService::Result result = BinaryUploadService::Result::UNKNOWN;
+    BinaryUploadService::Request::Data data;
+
+    // Store the file size separately instead of using data.contents.size() to
+    // keep track of size for large files.
+    int64_t size = 0;
+    std::string sha256;
+  };
+
   // Enum to identify special cases when deep scanning doesn't happen either
   // because the file is too large or encrypted. This is used to show
   // appropriate messages in the upload UI dialog to inform the user of why
@@ -190,6 +206,9 @@ class DeepScanningDialogDelegate {
   // block it.
   static bool ResultShouldAllowDataUse(BinaryUploadService::Result result);
 
+  // Callback used by FileSourceRequest to read file data on a blocking thread.
+  static FileContents GetFileContentsSHA256Blocking(const base::FilePath& path);
+
  protected:
   DeepScanningDialogDelegate(content::WebContents* web_contents,
                              Data data,

chrome/browser/safe_browsing/cloud_content_scanning/deep_scanning_dialog_delegate_unittest.cc

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/files/scoped_temp_dir.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
@@ -1528,4 +1529,98 @@ TEST_F(DeepScanningDialogDelegatePolicyResultsTest,
       BinaryUploadService::Result::FILE_ENCRYPTED));
 }
 
+class DeepScanningFileContentsTest : public testing::Test {
+ public:
+  void TestFile(const std::string& file_contents,
+                const std::string& expected_sha256,
+                BinaryUploadService::Result expected_result) {
+    base::ScopedTempDir temp_dir;
+    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+    base::FilePath file_path = temp_dir.GetPath().AppendASCII("foo.doc");
+
+    // Create the file.
+    base::File file(file_path,
+                    base::File::FLAG_CREATE | base::File::FLAG_WRITE);
+    file.WriteAtCurrentPos(file_contents.data(), file_contents.size());
+
+    // Obtain its contents and validate the returned value.
+    DeepScanningDialogDelegate::FileContents contents =
+        DeepScanningDialogDelegate::GetFileContentsSHA256Blocking(file_path);
+    ASSERT_EQ(contents.result, expected_result);
+    ASSERT_EQ(static_cast<size_t>(contents.size), file_contents.size());
+    if (expected_result == BinaryUploadService::Result::FILE_TOO_LARGE) {
+      ASSERT_TRUE(contents.data.contents.empty());
+    } else {
+      ASSERT_EQ(contents.data.contents, file_contents);
+    }
+    ASSERT_EQ(contents.sha256, expected_sha256);
+  }
+
+  static constexpr int kLargeFileThreshold =
+      BinaryUploadService::kMaxUploadSizeBytes;
+};
+
+TEST_F(DeepScanningFileContentsTest, InvalidFiles) {
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  // Non-existent files should return UNKNOWN and have no information set.
+  DeepScanningDialogDelegate::FileContents contents =
+      DeepScanningDialogDelegate::GetFileContentsSHA256Blocking(
+          temp_dir.GetPath().AppendASCII("not_a_real.doc"));
+  ASSERT_EQ(contents.result, BinaryUploadService::Result::UNKNOWN);
+  ASSERT_EQ(contents.size, 0u);
+  ASSERT_TRUE(contents.data.contents.empty());
+  ASSERT_TRUE(contents.sha256.empty());
+
+  // Directories should not be used as paths passed to GetFileSHA256Blocking, so
+  // they should return UNKNOWN and have no information set.
+  contents = DeepScanningDialogDelegate::GetFileContentsSHA256Blocking(
+      temp_dir.GetPath());
+  ASSERT_EQ(contents.result, BinaryUploadService::Result::UNKNOWN);
+  ASSERT_EQ(contents.size, 0u);
+  ASSERT_TRUE(contents.data.contents.empty());
+  ASSERT_TRUE(contents.sha256.empty());
+}
+
+TEST_F(DeepScanningFileContentsTest, NormalFiles) {
+  std::string file_contents = "Normal file contents";
+  // printf "Normal file contents" | sha256sum
+  std::string expected_sha256 = {
+      0x29, 0x64, 0x4c, 0x10, 0xbd, 0x03, 0x68, 0x66, 0xfc, 0xfd, 0x2b,
+      0xda, 0xcf, 0xf3, 0x40, 0xdb, 0x5d, 0xe4, 0x7a, 0x90, 0x00, 0x2d,
+      0x6a, 0xb0, 0xc4, 0x2d, 0xe6, 0xa2, 0x2c, 0x26, 0x15, 0x8b};
+  TestFile(file_contents, expected_sha256,
+           BinaryUploadService::Result::SUCCESS);
+
+  std::string almost_large_file_contents(kLargeFileThreshold, 'a');
+  // python3 -c "print('a' * (50 * 1024 * 1024), end='')" | sha256sum
+  std::string almost_large_expected_sha256 = {
+      0x4f, 0x0e, 0x9c, 0x6a, 0x1a, 0x9a, 0x90, 0xf3, 0x5b, 0x88, 0x4d,
+      0x0f, 0x0e, 0x73, 0x43, 0x45, 0x9c, 0x21, 0x06, 0x0e, 0xef, 0xec,
+      0x6c, 0x0f, 0x2f, 0xa9, 0xdc, 0x11, 0x18, 0xdb, 0xe5, 0xbe};
+  TestFile(almost_large_file_contents, almost_large_expected_sha256,
+           BinaryUploadService::Result::SUCCESS);
+}
+
+TEST_F(DeepScanningFileContentsTest, LargeFiles) {
+  std::string large_file_contents(kLargeFileThreshold + 1, 'a');
+  // python3 -c "print('a' * (50 * 1024 * 1024 + 1), end='')" | sha256sum
+  std::string large_expected_sha256 = {
+      0x9e, 0xb5, 0x6d, 0xb3, 0x0c, 0x49, 0xe1, 0x31, 0x45, 0x9f, 0xe7,
+      0x35, 0xba, 0x6b, 0x9d, 0x38, 0x32, 0x73, 0x76, 0x22, 0x4e, 0xc8,
+      0xd5, 0xa1, 0x23, 0x3f, 0x43, 0xa5, 0xb4, 0xa2, 0x59, 0x42};
+  TestFile(large_file_contents, large_expected_sha256,
+           BinaryUploadService::Result::FILE_TOO_LARGE);
+
+  std::string very_large_file_contents(2 * kLargeFileThreshold, 'a');
+  // python3 -c "print('a' * (100 * 1024 * 1024), end='')" | sha256sum
+  std::string very_large_expected_sha256 = {
+      0xce, 0xe4, 0x1e, 0x98, 0xd0, 0xa6, 0xad, 0x65, 0xcc, 0x0e, 0xc7,
+      0x7a, 0x2b, 0xa5, 0x0b, 0xf2, 0x6d, 0x64, 0xdc, 0x90, 0x07, 0xf7,
+      0xf1, 0xc7, 0xd7, 0xdf, 0x68, 0xb8, 0xb7, 0x12, 0x91, 0xa6};
+  TestFile(very_large_file_contents, very_large_expected_sha256,
+           BinaryUploadService::Result::FILE_TOO_LARGE);
+}
+
 }  // namespace safe_browsing