[Downloads] Rework how hashes are calculated for download files.

content/browser/download/base_file.h

Index: content/browser/download/base_file.h
diff --git a/content/browser/download/base_file.h b/content/browser/download/base_file.h
index cc87ae6ce6713a569ed2c418e0e2d87ff61e80b1..811dd7bf839f999af3cb014f73bf855a02af9ebe 100644
--- a/content/browser/download/base_file.h
+++ b/content/browser/download/base_file.h
@@ -20,40 +20,72 @@
 #include "base/time/time.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/download_interrupt_reasons.h"
-#include "crypto/sha2.h"
+#include "crypto/secure_hash.h"
 #include "net/base/net_errors.h"
 #include "net/log/net_log.h"
 #include "url/gurl.h"
 
-namespace crypto {
-class SecureHash;
-}
-
 namespace content {
 
 // File being downloaded and saved to disk. This is a base class
-// for DownloadFile and SaveFile, which keep more state information.
+// for DownloadFile and SaveFile, which keep more state information. BaseFile
+// considers itself the owner of the physical file and will delete it when the
+// BaseFile object is destroyed unless the ownership is revoked via a call to
+// Detach().
 class CONTENT_EXPORT BaseFile {
  public:
   // May be constructed on any thread.  All other routines (including
   // destruction) must occur on the FILE thread.
-  BaseFile(const base::FilePath& full_path,
-           const GURL& source_url,
-           const GURL& referrer_url,
-           int64_t received_bytes,
-           bool calculate_hash,
-           const std::string& hash_state,
-           base::File file,
-           const net::BoundNetLog& bound_net_log);
-  virtual ~BaseFile();
+  BaseFile(const net::BoundNetLog& bound_net_log);
+  ~BaseFile();
 
   // Returns DOWNLOAD_INTERRUPT_REASON_NONE on success, or a
-  // DownloadInterruptReason on failure.  |default_directory| specifies the
-  // directory to create the temporary file in if |full_path()| is empty. If
-  // |default_directory| and |full_path()| are empty, then a temporary file will
-  // be created in the default download location as determined by
-  // ContentBrowserClient.
-  DownloadInterruptReason Initialize(const base::FilePath& default_directory);
+  // DownloadInterruptReason on failure. Upon success, the file at |full_path()|
+  // is assumed to be owned by the BaseFile. It will be deleted when the
+  // BaseFile object is destroyed unless Detach() is called before destroying
+  // the BaseFile instance.
+  //
+  // |full_path|: Full path to the download file. Can be empty, in which case
+  //     the rules described in |default_directory| will be used to generate a
+  //     temporary filename.
+  //
+  // |default_directory|: specifies the directory to create the temporary file
+  //     in if |full_path| is empty. If |default_directory| and |full_path| are
+  //     empty, then a temporary file will be created in the default download
+  //     location as determined by ContentBrowserClient.
+  //
+  // |file|: The base::File handle to use. If specified, BaseFile will not open
+  //     a file and will use this handle. The file should be opened for both
+  //     read and write. Only makes sense if |full_path| is non-empty since it
+  //     implies that the caller already knows the path to the file.  There's no
+  //     perfect way to come up with a canonical path for a file. So BaseFile
+  //     will not attempt to determine the |full_path|.
+  //
+  // |bytes_so_far|: If a file is provided (via |full_path| or |file|), then
+  //     this argument specifies the size of the file to expect. It is legal for
+  //     the file to be larger, in which case the file will be truncated down to
+  //     this size. However, if the file is shorter, then the operation will
+  //     fail with DOWNLOAD_INTERRUPT_REASON_FILE_TOO_SHORT.
+  //
+  // |hash_so_far|: If |bytes_so_far| is non-zero, this specifies the SHA-256
+  //     hash of the first |bytes_so_far| bytes of the target file. If
+  //     specified, BaseFile will read the first |bytes_so_far| of the target
+  //     file in order to calculate the hash and verify that the file matches.
+  //     If there's a mismatch, then the operation fails with
+  //     DOWNLOAD_INTERRUPT_REASON_FILE_HASH_MISMATCH. Not used if |hash_state|
+  //     is also specified.
+  //
+  // |hash_state|: The partial hash object to use. Only meaningful if there's a
+  //     preexisting target file and it is non-empty (i.e. bytes_so_far is
+  //     non-zero). If specified, BaseFile will assume that the bytes up to
+  //     |bytes_so_far| has been accurately hashed into |hash_state| and will
+  //     ignore |hash_so_far|.
+  DownloadInterruptReason Initialize(const base::FilePath& full_path,
+                                     const base::FilePath& default_directory,
+                                     base::File file,
+                                     int64_t bytes_so_far,
+                                     const std::string& hash_so_far,
+                                     scoped_ptr<crypto::SecureHash> hash_state);
 
   // Write a new chunk of data to the file. Returns a DownloadInterruptReason
   // indicating the result of the operation.
@@ -63,33 +95,39 @@ class CONTENT_EXPORT BaseFile {
   // result of the operation. A return code of NONE indicates that the rename
   // was successful. After a failure, the full_path() and in_progress() can be
   // used to determine the last known filename and whether the file is available
-  // for writing or retrying the rename.
-  virtual DownloadInterruptReason Rename(const base::FilePath& full_path);
-
-  // Detach the file so it is not deleted on destruction.
-  virtual void Detach();
-
-  // Abort the download and automatically close the file.
+  // for writing or retrying the rename. Call Finish() to obtain the last known
+  // hash state.
+  DownloadInterruptReason Rename(const base::FilePath& full_path);
+
+  // Mark the file as detached. Up until this method is called, BaseFile assumes
+  // ownership of the file and hence will delete the file if the BaseFile object
+  // is destroyed. Calling Detach() causes BaseFile to assume that it no longer
+  // owns the file. Detach() can be called at any time. Close() must still be
+  // called to close the file if it is open.
+  void Detach();
+
+  // Abort the download and automatically close and delete the file.
   void Cancel();
 
   // Indicate that the download has finished. No new data will be received.
-  void Finish();
-
-  // Indicate that the download is being aborted due to an error. This is
-  // identical to Finish() with the exception that the hash state will not be
-  // finalized.
-  void FinishWithError();
-
-  // Set the client guid which will be used to identify the app to the
-  // system AV scanning function. Should be called before
-  // AnnotateWithSourceInformation() to take effect.
-  void SetClientGuid(const std::string& guid);
+  // Returns the SecureHash object representing the state of the hash function
+  // at the end of the operation.
+  scoped_ptr<crypto::SecureHash> Finish();
 
   // Informs the OS that this file came from the internet. Returns a
   // DownloadInterruptReason indicating the result of the operation.
-  // Note: SetClientGuid() should be called before this function on
-  // Windows to ensure the correct app client ID is available.
-  DownloadInterruptReason AnnotateWithSourceInformation();
+  //
+  // |client_guid|: The client GUID which will be used to identify the caller to
+  //     the system AV scanning function.
+  //
+  // |source_url| / |referrer_url|: Source and referrer for the network request
+  //     that originated this download. Will be used to annotate source
+  //     information and also to determine the relative danger level of the
+  //     file.
+  DownloadInterruptReason AnnotateWithSourceInformation(
+      const std::string& client_guid,
+      const GURL& source_url,
+      const GURL& referrer_url);
 
   // Returns the last known path to the download file. Can be empty if there's
   // no file.
@@ -102,26 +140,14 @@ class CONTENT_EXPORT BaseFile {
   // Returns the number of bytes in the file pointed to by full_path().
   int64_t bytes_so_far() const { return bytes_so_far_; }
 
-  // Fills |hash| with the hash digest for the file.
-  // Returns true if digest is successfully calculated.
-  virtual bool GetHash(std::string* hash);
-
-  // Returns the current (intermediate) state of the hash as a byte string.
-  virtual std::string GetHashState();
-
-  // Returns true if the given hash is considered empty.  An empty hash is
-  // a string of size crypto::kSHA256Length that contains only zeros (initial
-  // value for the hash).
-  static bool IsEmptyHash(const std::string& hash);
-
-  virtual std::string DebugString() const;
+  std::string DebugString() const;
 
  private:
   friend class BaseFileTest;
   FRIEND_TEST_ALL_PREFIXES(BaseFileTest, IsEmptyHash);
 
   // Creates and opens the file_ if it is NULL.
-  DownloadInterruptReason Open();

[svaldez, 2016/03/09 19:27:33]

Add comment about hash_so_far.

[asanka, 2016/03/10 16:48:08]

Done.

+  DownloadInterruptReason Open(const std::string& hash_so_far);
 
   // Closes and resets file_.
   void Close();
@@ -138,6 +164,17 @@ class CONTENT_EXPORT BaseFile {
   // Split out from CurrentSpeed to enable testing.
   int64_t CurrentSpeedAtTime(base::TimeTicks current_time) const;
 
+  // Verifies that:
+  // * Size of the file represented by |file_| is at least |bytes_so_far_|.
+  //
+  // * If |hash_to_expect| is not empty, then the result of hashing the first
+  //   |bytes_so_far_| bytes of |file_| matches |hash_to_expect|.
+  //
+  // If the result is REASON_NONE, then on return |secure_hash_| is valid and
+  // is ready to hash bytes from offset |bytes_so_far_| + 1.
+  DownloadInterruptReason CalculatePartialHash(
+      const std::string& hash_to_expect);
+
   // Log a TYPE_DOWNLOAD_FILE_ERROR NetLog event with |error| and passes error
   // on through, converting to a |DownloadInterruptReason|.
   DownloadInterruptReason LogNetError(const char* operation, net::Error error);
@@ -153,40 +190,24 @@ class CONTENT_EXPORT BaseFile {
       const char* operation, int os_error,
       DownloadInterruptReason reason);
 
-  static const unsigned char kEmptySha256Hash[crypto::kSHA256Length];
-
   // Full path to the file including the file name.
   base::FilePath full_path_;
 
-  // Source URL for the file being downloaded.
-  GURL source_url_;
-
-  // The URL where the download was initiated.
-  GURL referrer_url_;
-
-  std::string client_guid_;
-
   // OS file for writing
   base::File file_;
 
   // Amount of data received up so far, in bytes.
-  int64_t bytes_so_far_;
+  int64_t bytes_so_far_ = 0;
 
-  // Start time for calculating speed.
-  base::TimeTicks start_tick_;
-
-  // Indicates if hash should be calculated for the file.
-  bool calculate_hash_;
-
-  // Used to calculate hash for the file when calculate_hash_
-  // is set.
+  // Used to calculate hash for the file when calculate_hash_ is set.
   scoped_ptr<crypto::SecureHash> secure_hash_;
 
-  unsigned char sha256_hash_[crypto::kSHA256Length];
+  // Start time for calculating speed.
+  base::TimeTicks start_tick_;
 
   // Indicates that this class no longer owns the associated file, and so
   // won't delete it on destruction.
-  bool detached_;
+  bool detached_ = false;
 
   net::BoundNetLog bound_net_log_;