Improve Zip File Scanning on Mac

third_party/zlib/google/zip_reader.cc

Index: third_party/zlib/google/zip_reader.cc
diff --git a/third_party/zlib/google/zip_reader.cc b/third_party/zlib/google/zip_reader.cc
index ff5a39514af113d28f5266d2c119d51a4d386328..d8abafbc574b36240dc744a0c9d66e86da8eed60 100644
--- a/third_party/zlib/google/zip_reader.cc
+++ b/third_party/zlib/google/zip_reader.cc
@@ -325,6 +325,65 @@ bool ZipReader::ExtractCurrentEntry(WriterDelegate* delegate) const {
   return success;
 }
 
+bool ZipReader::ExtractPartOfCurrentEntry(WriterDelegate* delegate,

[satorux1, 2017/07/18 07:46:37]

This function looks similar to ExtractCurrentEntry() above. Can you unify them?
I wonder if you can do something like:

  bool ZipReader::ExtractCurrentEntry(WriterDelegate* delegate) const {
    return ExtractPartOfCurrentEntry(delegate, -1);  // -1 means everything
  }

[mortonm, 2017/07/18 19:58:49]

Done.

+                                          size_t num_bytes_to_extract) const {
+  DCHECK(zip_file_);
+
+  const int open_result = unzOpenCurrentFile(zip_file_);
+  if (open_result != UNZ_OK)
+    return false;
+
+  if (!delegate->PrepareOutput())
+    return false;
+
+  bool success = true;  // This becomes false when something bad happens.
+  std::unique_ptr<char[]> buf(new char[num_bytes_to_extract]);
+  base::CheckedNumeric<size_t> total_num_bytes_read = 0;
+  while (true) {
+    // This should never result in a negative value, as |num_bytes_to_extract|
+    // will always be greater than |total_num_bytes_read|.
+    auto bytes_to_read = base::CheckedNumeric<size_t>(num_bytes_to_extract) -
+                         total_num_bytes_read;
+    if (!bytes_to_read.IsValid()) {
+      DLOG(ERROR) << "Size of bytes to read underflows";
+      return false;
+    }
+    const int num_bytes_read = unzReadCurrentFile(
+        zip_file_, buf.get(),
+        base::checked_cast<unsigned int>(bytes_to_read.ValueOrDie()));
+
+    if (num_bytes_read == 0) {
+      // Reached the end of the file, which should not happen unless |num_bytes|
+      // is greater than the file length.
+      return false;
+    } else if (num_bytes_read < 0) {
+      // If num_bytes_read < 0, then it's a specific UNZ_* error code.
+      success = false;
+      break;
+    } else if (num_bytes_read > 0) {
+      // Some data is read.
+      if (!delegate->WriteBytes(buf.get(), num_bytes_read)) {
+        success = false;
+        break;
+      }
+    }
+    // Previous check ensures that num_bytes_read is not negative here.
+    total_num_bytes_read += base::checked_cast<size_t>(num_bytes_read);
+    if (!total_num_bytes_read.IsValid()) {
+      DLOG(ERROR) << "Size of total_num_bytes_read overflows";
+      return false;
+    }
+    if (total_num_bytes_read.ValueOrDie() == num_bytes_to_extract) {
+      // Read all the data.
+      break;
+    }
+  }
+
+  unzCloseCurrentFile(zip_file_);
+
+  return success;
+}
+
 bool ZipReader::ExtractCurrentEntryToFilePath(
     const base::FilePath& output_file_path) const {
   DCHECK(zip_file_);
@@ -446,6 +505,33 @@ bool ZipReader::ExtractCurrentEntryToString(size_t max_read_bytes,
   return true;
 }
 
+bool ZipReader::ExtractPartOfCurrentEntryToString(size_t num_bytes,

[satorux1, 2017/07/18 07:46:37]

ditto. this looks similar to ExtractCurrentEntryToString(). would be good to
unify them.

[mortonm, 2017/07/18 19:58:49]

Done.

+                                                  std::string* output) const {
+  DCHECK(output);
+  DCHECK(zip_file_);
+
+  if (num_bytes == 0U) {
+    output->clear();
+    return true;
+  }
+
+  if (current_entry_info()->is_directory()) {
+    output->clear();
+    return true;
+  }
+
+  std::string contents;
+  contents.reserve(num_bytes);
+
+  StringWriterDelegate writer(num_bytes, &contents);
+  if (!ExtractPartOfCurrentEntry(&writer, num_bytes)) {
+    output->clear();
+    return false;
+  }
+  output->swap(contents);
+  return true;
+}
+
 bool ZipReader::OpenInternal() {
   DCHECK(zip_file_);