[zip] Improve log messages

Removed option UnzipOptions::log_skipped_files. It wasn't getting set
anywhere. Instead, changed the "skipped entry" log level to VLOG(1).

Improved log messages. Redacted file paths that could be considered PII
in some circumstances.

Changed some debug warning messages to error messages.

BUG=chromium:1303983
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: I2bc5fc7d3dcf5af465faee4bdd4311e244486a9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3520763
Reviewed-by: Alex Danilo <adanilo@chromium.org>
Commit-Queue: François Degros <fdegros@chromium.org>
Cr-Commit-Position: refs/heads/main@{#980441}
NOKEYCHECK=True
GitOrigin-RevId: f2cb378ae18451b14f0176de16fe4745aead0a9f

2 files changed, 15 insertions, 16 deletions

diff --git a/google/zip.cc b/google/zip.cc
index 5687b7f..a52f406 100644
--- a/google/zip.cc
+++ b/google/zip.cc
@@ -14,6 +14,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
+#include "third_party/zlib/google/redact.h"
 #include "third_party/zlib/google/zip_internal.h"
 #include "third_party/zlib/google/zip_reader.h"
 #include "third_party/zlib/google/zip_writer.h"
@@ -55,12 +56,13 @@ class DirectFileAccessor : public FileAccessor {
       const base::FilePath absolute_path = src_dir_.Append(path);
       if (base::DirectoryExists(absolute_path)) {
         files->emplace_back();
-        LOG(ERROR) << "Cannot open '" << path << "': It is a directory";
+        LOG(ERROR) << "Cannot open " << Redact(path) << ": It is a directory";
       } else {
-        files->emplace_back(absolute_path,
-                            base::File::FLAG_OPEN | base::File::FLAG_READ);
-        LOG_IF(ERROR, !files->back().IsValid())
-            << "Cannot open '" << path << "'";
+        const base::File& file = files->emplace_back(
+            absolute_path, base::File::FLAG_OPEN | base::File::FLAG_READ);
+        LOG_IF(ERROR, !file.IsValid())
+            << "Cannot open " << Redact(path) << ": "
+            << base::File::ErrorToString(file.error_details());
       }
     }
 
@@ -93,7 +95,7 @@ class DirectFileAccessor : public FileAccessor {
 
     base::File::Info file_info;
     if (!base::GetFileInfo(src_dir_.Append(path), &file_info)) {
-      LOG(ERROR) << "Cannot get info of '" << path << "'";
+      PLOG(ERROR) << "Cannot get info of " << Redact(path);
       return false;
     }
 
@@ -170,7 +172,8 @@ bool Unzip(const base::FilePath& src_file,
            UnzipOptions options) {
   base::File file(src_file, base::File::FLAG_OPEN | base::File::FLAG_READ);
   if (!file.IsValid()) {
-    DLOG(WARNING) << "Cannot open '" << src_file << "'";
+    LOG(ERROR) << "Cannot open " << Redact(src_file) << ": "
+               << base::File::ErrorToString(file.error_details());
     return false;
   }
 
@@ -189,19 +192,18 @@ bool Unzip(const base::PlatformFile& src_file,
   reader.SetPassword(std::move(options.password));
 
   if (!reader.OpenFromPlatformFile(src_file)) {
-    DLOG(WARNING) << "Cannot open ZIP from file handle " << src_file;
+    LOG(ERROR) << "Cannot open ZIP from file handle " << src_file;
     return false;
   }
 
   while (const ZipReader::Entry* const entry = reader.Next()) {
     if (entry->is_unsafe) {
-      DLOG(WARNING) << "Found unsafe entry in ZIP: " << entry->path;
+      LOG(ERROR) << "Found unsafe entry " << Redact(entry->path) << " in ZIP";
       return false;
     }
 
     if (options.filter && !options.filter.Run(entry->path)) {
-      DLOG_IF(WARNING, options.log_skipped_files)
-          << "Skipped ZIP entry " << entry->path;
+      VLOG(1) << "Skipped ZIP entry " << Redact(entry->path);
       continue;
     }
 
@@ -216,7 +218,8 @@ 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())) {
-      DLOG(WARNING) << "Cannot extract " << entry->path;
+      LOG(ERROR) << "Cannot extract file " << Redact(entry->path)
+                 << " from ZIP";
       return false;
     }
   }
diff --git a/google/zip.h b/google/zip.h
index ebd7c56..0928bbd 100644
--- a/google/zip.h
+++ b/google/zip.h
@@ -182,10 +182,6 @@ struct UnzipOptions {
 
   // Password to decrypt the encrypted files.
   std::string password;
-
-  // If |log_skipped_files| is true, files skipped during extraction are printed
-  // to debug log.
-  bool log_skipped_files = true;
 };
 
 typedef base::RepeatingCallback<std::unique_ptr<WriterDelegate>(