[zip] Simplify ZipReader::ExtractCurrentEntryToString()

BUG=chromium:1295127
TEST=autoninja -C out/Default zlib_unittests && out/Default/zlib_unittests

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

2 files changed, 30 insertions, 74 deletions

diff --git a/google/zip_reader.cc b/google/zip_reader.cc
index 1532ca3..c6da9a8 100644
--- a/google/zip_reader.cc
+++ b/google/zip_reader.cc
@@ -4,6 +4,7 @@
 
 #include "third_party/zlib/google/zip_reader.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ -11,6 +12,7 @@
 #include "base/files/file.h"
 #include "base/i18n/icu_string_conversions.h"
 #include "base/logging.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
@@ -64,62 +66,21 @@ std::ostream& operator<<(std::ostream& out, Redact r) {
   return LOG_IS_ON(INFO) ? out << "'" << r.path << "'" : out << "(redacted)";
 }
 
-// StringWriterDelegate --------------------------------------------------------
-
-// A writer delegate that writes no more than |max_read_bytes| to a given
-// std::string.
+// A writer delegate that writes to a given string.
 class StringWriterDelegate : public WriterDelegate {
  public:
-  StringWriterDelegate(size_t max_read_bytes, std::string* output);
-
-  StringWriterDelegate(const StringWriterDelegate&) = delete;
-  StringWriterDelegate& operator=(const StringWriterDelegate&) = delete;
-
-  ~StringWriterDelegate() override;
+  explicit StringWriterDelegate(std::string* output) : output_(output) {}
 
   // WriterDelegate methods:
-
-  // Returns true.
-  bool PrepareOutput() override;
-
-  // Appends |num_bytes| bytes from |data| to the output string. Returns false
-  // if |num_bytes| will cause the string to exceed |max_read_bytes|.
-  bool WriteBytes(const char* data, int num_bytes) override;
-
-  void SetTimeModified(const base::Time& time) override;
-
-  void SetPosixFilePermissions(int mode) override;
+  bool WriteBytes(const char* data, int num_bytes) override {
+    output_->append(data, num_bytes);
+    return true;
+  }
 
  private:
-  size_t max_read_bytes_;
-  std::string* output_;
+  std::string* const output_;
 };
 
-StringWriterDelegate::StringWriterDelegate(size_t max_read_bytes,
-                                           std::string* output)
-    : max_read_bytes_(max_read_bytes), output_(output) {}
-
-StringWriterDelegate::~StringWriterDelegate() {}
-
-bool StringWriterDelegate::PrepareOutput() {
-  return true;
-}
-
-bool StringWriterDelegate::WriteBytes(const char* data, int num_bytes) {
-  if (output_->size() + num_bytes > max_read_bytes_)
-    return false;
-  output_->append(data, num_bytes);
-  return true;
-}
-
-void StringWriterDelegate::SetTimeModified(const base::Time& time) {
-  // Do nothing.
-}
-
-void StringWriterDelegate::SetPosixFilePermissions(int mode) {
-  // Do nothing.
-}
-
 #if defined(OS_POSIX)
 void SetPosixFilePermissions(int fd, int mode) {
   base::stat_wrapper_t sb;
@@ -448,41 +409,36 @@ bool ZipReader::ExtractCurrentEntryToString(uint64_t max_read_bytes,
                                             std::string* output) const {
   DCHECK(output);
   DCHECK(zip_file_);
+  DCHECK(ok_);
+  DCHECK(!reached_end_);
+
+  output->clear();
 
-  if (max_read_bytes == 0) {
-    output->clear();
+  if (max_read_bytes == 0)
     return true;
-  }
 
-  if (current_entry_info()->is_directory()) {
-    output->clear();
+  if (entry_.is_directory())
     return true;
-  }
 
-  // The original_size() is the best hint for the real size, so it saves
-  // doing 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.
-  std::string contents;
-  contents.reserve(
-      static_cast<size_t>(std::min(base::checked_cast<int64_t>(max_read_bytes),
-                                   current_entry_info()->original_size())));
+  // The original_size is the best hint for the real size, so it saves doing
+  // 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())));
 
-  StringWriterDelegate writer(max_read_bytes, &contents);
+  StringWriterDelegate writer(output);
   if (!ExtractCurrentEntry(&writer, max_read_bytes)) {
-    if (contents.length() < max_read_bytes) {
+    if (output->size() < max_read_bytes) {
       // There was an error in extracting entry. If ExtractCurrentEntry()
       // returns false, the entire file was not read - in which case
-      // contents.length() should equal |max_read_bytes| unless an error
-      // occurred which caused extraction to be aborted.
+      // output->size() should equal |max_read_bytes| unless an error occurred
+      // which caused extraction to be aborted.
       output->clear();
-    } else {
-      // |num_bytes| is less than the length of current entry.
-      output->swap(contents);
     }
     return false;
   }
-  output->swap(contents);
+
   return true;
 }
 
diff --git a/google/zip_reader.h b/google/zip_reader.h
index 3e46ca9..4eea422 100644
--- a/google/zip_reader.h
+++ b/google/zip_reader.h
@@ -33,19 +33,19 @@ class WriterDelegate {
 
   // Invoked once before any data is streamed out to pave the way (e.g., to open
   // the output file). Return false on failure to cancel extraction.
-  virtual bool PrepareOutput() = 0;
+  virtual bool PrepareOutput() { return true; }
 
   // Invoked to write the next chunk of data. Return false on failure to cancel
   // extraction.
-  virtual bool WriteBytes(const char* data, int num_bytes) = 0;
+  virtual bool WriteBytes(const char* data, int num_bytes) { return true; }
 
   // Sets the last-modified time of the data.
-  virtual void SetTimeModified(const base::Time& time) = 0;
+  virtual void SetTimeModified(const base::Time& time) {}
 
   // Called with the POSIX file permissions of the data; POSIX implementations
   // may apply some of the permissions (for example, the executable bit) to the
   // output file.
-  virtual void SetPosixFilePermissions(int mode) = 0;
+  virtual void SetPosixFilePermissions(int mode) {}
 };
 
 // This class is used for reading ZIP archives. A typical use case of this class