Use base::Optional for passphrase, and add a unit test.

BUG=889703

Change-Id: I06284ea5d72359ea9d6950b65aa891e86b20550e
Reviewed-on: https://chromium-review.googlesource.com/c/1307016Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603799}

chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_archive_minizip_unittest.cc

@@ -112,7 +112,7 @@ class InMemoryVolumeReader : public VolumeReader {
     return file_offset_;
   }
 
-  std::unique_ptr<std::string> Passphrase() override { return nullptr; }
+  base::Optional<std::string> Passphrase() override { return {}; }
 
   int64_t offset() override { return file_offset_; }
 

chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.cc

@@ -389,20 +389,20 @@ bool VolumeArchiveMinizip::SeekHeader(const std::string& path_name) {
   // Directories cannot be encrypted with the basic zip encrytion algorithm.
   if (((raw_file_info.flag & 1) != 0) && !is_directory) {
     do {
-      if (password_cache_ == nullptr) {
+      if (!password_cache_) {
         // Save passphrase for upcoming file requests.
         password_cache_ = reader()->Passphrase();
-        // check if |password_cache_| is nullptr in case when user clicks Cancel
-        if (password_cache_ == nullptr) {
+        // check if |password_cache_| is empty in case when user clicks Cancel
+        if (!password_cache_) {
           return false;
         }
       }
 
-      open_result =
-          unzOpenCurrentFilePassword(zip_file_, password_cache_.get()->c_str());
+      open_result = unzOpenCurrentFilePassword(zip_file_,
+                                               password_cache_.value().c_str());
 
       // If password is incorrect then password cache ought to be reseted.
-      if (open_result == UNZ_BADPASSWORD && password_cache_ != nullptr)
+      if (open_result == UNZ_BADPASSWORD)
         password_cache_.reset();
 
     } while (open_result == UNZ_BADPASSWORD);

chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.h

@@ -9,11 +9,11 @@
 #include <memory>
 #include <string>
 
+#include "base/optional.h"
+#include "chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive.h"
 #include "third_party/minizip/src/unzip.h"
 #include "third_party/minizip/src/zip.h"
 
-#include "chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive.h"
-
 // Defines an implementation of VolumeArchive that wraps all minizip
 // operations.
 class VolumeArchiveMinizip : public VolumeArchive {
@@ -149,7 +149,7 @@ class VolumeArchiveMinizip : public VolumeArchive {
   bool decompressed_error_;
 
   // The password cache to access password protected files.
-  std::unique_ptr<std::string> password_cache_;
+  base::Optional<std::string> password_cache_;
 };
 
 #endif  // CHROME_BROWSER_RESOURCES_CHROMEOS_ZIP_ARCHIVER_CPP_VOLUME_ARCHIVE_MINIZIP_H_

chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/md5.h"
+#include "base/optional.h"
 #include "base/path_service.h"
 #include "base/strings/string_piece.h"
 #include "chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive.h"
@@ -24,6 +25,8 @@
 
 namespace {
 
+constexpr char kEncryptedZipPassphrase[] = "test123";
+
 class TestVolumeReader : public VolumeReader {
  public:
   explicit TestVolumeReader(base::FilePath path)
@@ -40,15 +43,20 @@ class TestVolumeReader : public VolumeReader {
     return file_.Seek(whence, offset);
   }
 
-  std::unique_ptr<std::string> Passphrase() override { return nullptr; }
+  base::Optional<std::string> Passphrase() override { return passphrase_; }
 
   int64_t offset() override { return Seek(0, base::File::FROM_CURRENT); }
 
   int64_t archive_size() override { return file_.GetLength(); }
 
+  void set_passphrase(base::Optional<std::string> passphrase) {
+    passphrase_ = std::move(passphrase);
+  }
+
  private:
   base::File file_;
   std::vector<char> buffer_;
+  base::Optional<std::string> passphrase_;
 };
 
 class VolumeArchiveMinizipTest : public testing::Test {
@@ -88,6 +96,13 @@ const std::map<std::string, FileInfo> kSmallZipFiles = {
     {"dir/file3", {56, false, 1407913020, "bffbca4992b32db8ed72bfc2c88e7f11"}},
 };
 
+const std::map<std::string, FileInfo> kEncryptedZipFiles = {
+    {"file1", {15, false, 1407912954, "b4d9b82bb1cd97aa6191843149df18e6"}},
+    {"file2", {33, false, 1407912974, "b864e9456deb246b018c49ef831f7ca7"}},
+    {"dir/", {0, true, 1407913020, nullptr}},
+    {"dir/file3", {56, false, 1407913020, "bffbca4992b32db8ed72bfc2c88e7f11"}},
+};
+
 TEST_F(VolumeArchiveMinizipTest, Basic) {
   std::unique_ptr<TestVolumeReader> reader =
       std::make_unique<TestVolumeReader>(GetTestZipPath("small_zip.zip"));
@@ -185,4 +200,51 @@ TEST_F(VolumeArchiveMinizipTest, Read) {
   }
 }
 
+TEST_F(VolumeArchiveMinizipTest, Encrypted) {
+  std::unique_ptr<TestVolumeReader> reader =
+      std::make_unique<TestVolumeReader>(GetTestZipPath("encrypted.zip"));
+  reader->set_passphrase(
+      base::make_optional<std::string>(kEncryptedZipPassphrase));
+  VolumeArchiveMinizip archive(std::move(reader));
+  ASSERT_TRUE(archive.Init(""));
+
+  for (auto it : kEncryptedZipFiles) {
+    EXPECT_TRUE(archive.SeekHeader(it.first));
+    std::string file_path;
+    bool is_utf8 = false;
+    int64_t size = -1;
+    bool is_directory = false;
+    time_t mod_time = 0;
+    auto result = archive.GetCurrentFileInfo(&file_path, &is_utf8, &size,
+                                             &is_directory, &mod_time);
+    EXPECT_EQ(result, VolumeArchive::RESULT_SUCCESS);
+    EXPECT_EQ(file_path, it.first);
+    EXPECT_EQ(size, it.second.size);
+    EXPECT_EQ(is_directory, it.second.is_directory);
+    EXPECT_EQ(mod_time, it.second.mod_time);
+
+    if (it.second.is_directory)
+      continue;
+
+    base::MD5Context ctx;
+    base::MD5Init(&ctx);
+    const char* buffer = nullptr;
+    int64_t offset = 0;
+    while (offset < it.second.size) {
+      int64_t read = archive.ReadData(offset, it.second.size - offset, &buffer);
+      ASSERT_GT(read, 0);
+      EXPECT_LE(read, it.second.size - offset);
+      base::MD5Update(&ctx, base::StringPiece(buffer, read));
+      offset += read;
+    }
+    EXPECT_EQ(it.second.size, offset);
+    EXPECT_EQ(0, archive.ReadData(offset, 1, &buffer));
+
+    base::MD5Digest digest;
+    base::MD5Final(&digest, &ctx);
+    std::string md5_sum = base::MD5DigestToBase16(digest);
+    EXPECT_EQ(md5_sum, it.second.md5_sum);
+  }
+}
+
 }  // namespace

chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader.h

@@ -10,6 +10,7 @@
 #include <string>
 
 #include "base/files/file.h"
+#include "base/optional.h"
 
 // Defines a reader for archive volumes. This class is used by minizip
 // for custom reads.
@@ -34,9 +35,9 @@ class VolumeReader {
   // http://www.cplusplus.com/reference/cstdio/fseek/
   virtual int64_t Seek(int64_t offset, base::File::Whence whence) = 0;
 
-  // Fetches a passphrase for reading. If the passphrase is not available it
-  // returns nullptr.
-  virtual std::unique_ptr<std::string> Passphrase() = 0;
+  // Fetches a passphrase for reading. If the passphrase is not available, the
+  // returned Optional will have no value.
+  virtual base::Optional<std::string> Passphrase() = 0;
 
   virtual int64_t offset() = 0;
 

chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.cc

@@ -212,15 +212,14 @@ void VolumeReaderJavaScriptStream::SetRequestId(const std::string& request_id) {
   request_id_ = request_id;
 }
 
-std::unique_ptr<std::string> VolumeReaderJavaScriptStream::Passphrase() {
-  std::unique_ptr<std::string> result;
+base::Optional<std::string> VolumeReaderJavaScriptStream::Passphrase() {
   // The error is not recoverable. Once passphrase fails to be provided, it is
   // never asked again. Note, that still users are able to retry entering the
   // password, unless they click Cancel.
   {
     base::AutoLock al(shared_state_lock_);
     if (passphrase_error_) {
-      return result;
+      return {};
     }
   }
 
@@ -232,10 +231,10 @@ std::unique_ptr<std::string> VolumeReaderJavaScriptStream::Passphrase() {
   // TODO(amistry): Handle spurious wakeups.
   available_passphrase_cond_.Wait();
 
-  if (!passphrase_error_)
-    result.reset(new std::string(available_passphrase_));
+  if (passphrase_error_)
+    return {};
 
-  return result;
+  return base::make_optional<std::string>(available_passphrase_);
 }
 
 void VolumeReaderJavaScriptStream::RequestChunk(int64_t length) {

chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.h

@@ -71,7 +71,7 @@ class VolumeReaderJavaScriptStream : public VolumeReader {
   // See volume_reader.h for description. The method blocks on
   // available_passphrase_cond_. SetPassphraseAndSignal should unblock it from
   // another thread.
-  std::unique_ptr<std::string> Passphrase() override;
+  base::Optional<std::string> Passphrase() override;
 
   int64_t offset() override;