[zip] Add default args to ZipReader's methods

Added sensible default arguments to:
ZipReader::ExtractCurrentEntryToString()
ZipReader::ExtractCurrentEntry()

Updated tests.

BUG=chromium:1295127
TEST=autoninja -C out/Default zlib_unittests && out/Default/zlib_unittests
TEST=autoninja -C out/Default components_unittests && out/Default/components_unittests --gtest_filter='UnzipTest.*'

Change-Id: Iec8d047e5eb777774b1aaef1f3c66f38db72a593
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3469122
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: François Degros <fdegros@chromium.org>
Cr-Commit-Position: refs/heads/main@{#973621}
NOKEYCHECK=True
GitOrigin-RevId: bf5d8b34f48462995b919eb5428ef1b050ed0a00

4 files changed, 43 insertions, 28 deletions

diff --git a/google/zip.cc b/google/zip.cc
index 25e4fcb..88842c0 100644
--- a/google/zip.cc
+++ b/google/zip.cc
@@ -4,7 +4,6 @@
 
 #include "third_party/zlib/google/zip.h"
 
-#include <limits>
 #include <string>
 #include <vector>
 
@@ -216,8 +215,7 @@ bool Unzip(const base::PlatformFile& src_file,
 
     // It's a file.
     std::unique_ptr<WriterDelegate> writer = writer_factory.Run(entry->path);
-    if (!writer || !reader.ExtractCurrentEntry(
-                       writer.get(), std::numeric_limits<uint64_t>::max())) {
+    if (!writer || !reader.ExtractCurrentEntry(writer.get())) {
       DLOG(WARNING) << "Cannot extract " << entry->path;
       return false;
     }
diff --git a/google/zip_reader.cc b/google/zip_reader.cc
index b134cde..dcda7da 100644
--- a/google/zip_reader.cc
+++ b/google/zip_reader.cc
@@ -415,8 +415,8 @@ bool ZipReader::ExtractCurrentEntryToString(uint64_t max_read_bytes,
   // reallocations for the common case when the uncompressed size is correct.
   // However, we need to assume that the uncompressed size could be incorrect
   // therefore this function needs to read as much data as possible.
-  output->reserve(static_cast<size_t>(std::min(
-      base::checked_cast<int64_t>(max_read_bytes), entry_.original_size)));
+  output->reserve(base::checked_cast<size_t>(std::min<uint64_t>(
+      max_read_bytes, base::checked_cast<uint64_t>(entry_.original_size))));
 
   StringWriterDelegate writer(output);
   if (!ExtractCurrentEntry(&writer, max_read_bytes)) {
diff --git a/google/zip_reader.h b/google/zip_reader.h
index 1f309bd..0d81c21 100644
--- a/google/zip_reader.h
+++ b/google/zip_reader.h
@@ -7,6 +7,7 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <limits>
 #include <memory>
 #include <string>
 
@@ -15,6 +16,7 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/memory/weak_ptr.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/time/time.h"
 
 #if defined(USE_SYSTEM_MINIZIP)
@@ -60,8 +62,7 @@ class WriterDelegate {
 //
 //   while (const ZipReader::entry* entry = reader.Next()) {
 //     auto writer = CreateFilePathWriterDelegate(extract_dir, entry->path);
-//     if (!reader.ExtractCurrentEntry(
-//         writer, std::numeric_limits<uint64_t>::max())) {
+//     if (!reader.ExtractCurrentEntry(writer)) {
 //           // Cannot extract
 //           return;
 //     }
@@ -192,7 +193,8 @@ class ZipReader {
   //
   // Precondition: Next() returned a non-null Entry.
   bool ExtractCurrentEntry(WriterDelegate* delegate,
-                           uint64_t num_bytes_to_extract) const;
+                           uint64_t num_bytes_to_extract =
+                               std::numeric_limits<uint64_t>::max()) const;
 
   // Asynchronously extracts the current entry to the given output file path. If
   // the current entry is a directory it just creates the directory
@@ -230,6 +232,11 @@ class ZipReader {
   bool ExtractCurrentEntryToString(uint64_t max_read_bytes,
                                    std::string* output) const;
 
+  bool ExtractCurrentEntryToString(std::string* output) const {
+    return ExtractCurrentEntryToString(
+        base::checked_cast<uint64_t>(output->max_size()), output);
+  }
+
   // Returns the number of entries in the ZIP archive.
   //
   // Precondition: one of the Open() methods returned true.
diff --git a/google/zip_reader_unittest.cc b/google/zip_reader_unittest.cc
index 0680e5c..5cd878d 100644
--- a/google/zip_reader_unittest.cc
+++ b/google/zip_reader_unittest.cc
@@ -115,8 +115,7 @@ class MockWriterDelegate : public zip::WriterDelegate {
 bool ExtractCurrentEntryToFilePath(zip::ZipReader* reader,
                                    base::FilePath path) {
   zip::FilePathWriterDelegate writer(path);
-  return reader->ExtractCurrentEntry(&writer,
-                                     std::numeric_limits<uint64_t>::max());
+  return reader->ExtractCurrentEntry(&writer);
 }
 
 const zip::ZipReader::Entry* LocateAndOpenEntry(
@@ -426,7 +425,7 @@ TEST_F(ZipReaderTest, EncryptedFile_WrongPassword) {
     EXPECT_FALSE(entry->is_directory);
     EXPECT_FALSE(entry->is_encrypted);
     std::string contents = "dummy";
-    EXPECT_TRUE(reader.ExtractCurrentEntryToString(1000, &contents));
+    EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents));
     EXPECT_EQ("This is not encrypted.\n", contents);
   }
 
@@ -442,7 +441,7 @@ TEST_F(ZipReaderTest, EncryptedFile_WrongPassword) {
     EXPECT_FALSE(entry->is_directory);
     EXPECT_TRUE(entry->is_encrypted);
     std::string contents = "dummy";
-    EXPECT_FALSE(reader.ExtractCurrentEntryToString(1000, &contents));
+    EXPECT_FALSE(reader.ExtractCurrentEntryToString(&contents));
     EXPECT_EQ("", contents);
   }
 
@@ -462,7 +461,7 @@ TEST_F(ZipReaderTest, EncryptedFile_RightPassword) {
     EXPECT_FALSE(entry->is_directory);
     EXPECT_FALSE(entry->is_encrypted);
     std::string contents = "dummy";
-    EXPECT_TRUE(reader.ExtractCurrentEntryToString(1000, &contents));
+    EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents));
     EXPECT_EQ("This is not encrypted.\n", contents);
   }
 
@@ -478,7 +477,7 @@ TEST_F(ZipReaderTest, EncryptedFile_RightPassword) {
     EXPECT_FALSE(entry->is_directory);
     EXPECT_TRUE(entry->is_encrypted);
     std::string contents = "dummy";
-    EXPECT_FALSE(reader.ExtractCurrentEntryToString(1000, &contents));
+    EXPECT_FALSE(reader.ExtractCurrentEntryToString(&contents));
     EXPECT_EQ("", contents);
   }
 
@@ -490,7 +489,7 @@ TEST_F(ZipReaderTest, EncryptedFile_RightPassword) {
     EXPECT_FALSE(entry->is_directory);
     EXPECT_TRUE(entry->is_encrypted);
     std::string contents = "dummy";
-    EXPECT_TRUE(reader.ExtractCurrentEntryToString(1000, &contents));
+    EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents));
     EXPECT_EQ("This is encrypted with ZipCrypto.\n", contents);
   }
 
@@ -664,10 +663,7 @@ TEST_F(ZipReaderTest, ExtractToFileAsync_WrongCrc) {
   ASSERT_TRUE(base::ReadFileToString(target_path, &contents));
   EXPECT_EQ("This file has been changed after its CRC was computed.\n",
             contents);
-
-  int64_t file_size = 0;
-  ASSERT_TRUE(base::GetFileSize(target_path, &file_size));
-  EXPECT_EQ(file_size, listener.current_progress());
+  EXPECT_EQ(contents.size(), listener.current_progress());
 }
 
 // Verifies that the asynchronous extraction to a file works.
@@ -729,7 +725,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntryToString) {
     }
 
     // More than necessary byte read limit: must pass.
-    EXPECT_TRUE(reader.ExtractCurrentEntryToString(16, &contents));
+    EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents));
     EXPECT_EQ(std::string(base::StringPiece("0123456", i)), contents);
   }
   reader.Close();
@@ -787,7 +783,7 @@ TEST_F(ZipReaderTest, ExtractPosixPermissions) {
   for (auto entry : {"0.txt", "1.txt", "2.txt", "3.txt"}) {
     ASSERT_TRUE(LocateAndOpenEntry(&reader, base::FilePath::FromASCII(entry)));
     FilePathWriterDelegate delegate(temp_dir.GetPath().AppendASCII(entry));
-    ASSERT_TRUE(reader.ExtractCurrentEntry(&delegate, 10000));
+    ASSERT_TRUE(reader.ExtractCurrentEntry(&delegate));
   }
   reader.Close();
 
@@ -830,8 +826,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntryPrepareFailure) {
 
   ASSERT_TRUE(reader.Open(test_zip_file_));
   ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
-  ASSERT_FALSE(reader.ExtractCurrentEntry(
-      &mock_writer, std::numeric_limits<uint64_t>::max()));
+  ASSERT_FALSE(reader.ExtractCurrentEntry(&mock_writer));
 }
 
 // Test that when WriterDelegate::WriteBytes returns false, no other methods on
@@ -847,8 +842,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntryWriteBytesFailure) {
 
   ASSERT_TRUE(reader.Open(test_zip_file_));
   ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
-  ASSERT_FALSE(reader.ExtractCurrentEntry(
-      &mock_writer, std::numeric_limits<uint64_t>::max()));
+  ASSERT_FALSE(reader.ExtractCurrentEntry(&mock_writer));
 }
 
 // Test that extraction succeeds when the writer delegate reports all is well.
@@ -865,8 +859,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntrySuccess) {
 
   ASSERT_TRUE(reader.Open(test_zip_file_));
   ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path));
-  ASSERT_TRUE(reader.ExtractCurrentEntry(&mock_writer,
-                                         std::numeric_limits<uint64_t>::max()));
+  ASSERT_TRUE(reader.ExtractCurrentEntry(&mock_writer));
 }
 
 TEST_F(ZipReaderTest, WrongCrc) {
@@ -876,9 +869,26 @@ TEST_F(ZipReaderTest, WrongCrc) {
   const ZipReader::Entry* const entry =
       LocateAndOpenEntry(&reader, base::FilePath::FromASCII("Corrupted.txt"));
   ASSERT_TRUE(entry);
+
   std::string contents = "dummy";
-  EXPECT_FALSE(reader.ExtractCurrentEntryToString(1000, &contents));
+  EXPECT_FALSE(reader.ExtractCurrentEntryToString(&contents));
   EXPECT_EQ("", contents);
+
+  contents = "dummy";
+  EXPECT_FALSE(
+      reader.ExtractCurrentEntryToString(entry->original_size + 1, &contents));
+  EXPECT_EQ("", contents);
+
+  contents = "dummy";
+  EXPECT_FALSE(
+      reader.ExtractCurrentEntryToString(entry->original_size, &contents));
+  EXPECT_EQ("This file has been changed after its CRC was computed.\n",
+            contents);
+
+  contents = "dummy";
+  EXPECT_FALSE(
+      reader.ExtractCurrentEntryToString(entry->original_size - 1, &contents));
+  EXPECT_EQ("This file has been changed after its CRC was computed.", contents);
 }
 
 class FileWriterDelegateTest : public ::testing::Test {