Extract: Gather extracted size for all the ZIP files

Adds a mojo method to retrieve the expected size of an unpacked ZIP file.

Plumbs the size checking of all ZIPs being extracted as a first pass
before doing the actual extraction itself.

Bug: 953256
Tests: components_unittests --gtest_filter='UnzipTest.GetExtracted*"
Change-Id: I72d53203d6c3140adb979ce9d826b71a50d62035
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3627249
Reviewed-by: François Degros <fdegros@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Commit-Queue: Alex Danilo <adanilo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001292}

chrome/browser/ash/file_manager/extract_io_task.cc

@@ -87,6 +87,44 @@ void ExtractIOTask::ExtractArchive(
   }
 }
 
+void ExtractIOTask::ExtractAllSources() {
+  for (size_t index = 0; index < progress_.sources.size(); ++index) {
+    const EntryStatus& source = progress_.sources[index];
+    const base::FilePath source_file = source.url.path().BaseName();
+    util::GenerateUnusedFilename(
+        parent_folder_, source_file.RemoveExtension(), file_system_context_,
+        base::BindOnce(&ExtractIOTask::ExtractArchive,
+                       weak_ptr_factory_.GetWeakPtr(), index));
+  }
+}
+
+void ExtractIOTask::ZipSizeCallback(unzip::mojom::SizePtr size_info) {
+  DCHECK_GT(extractCount_, 0);
+  if (size_info->is_valid) {
+    progress_.total_bytes += size_info->value;
+  }
+  if (--extractCount_ == 0) {
+    // After getting the size of all the ZIPs, extract them.
+    extractCount_ = progress_.sources.size();
+    ExtractAllSources();
+  }
+}
+
+void ExtractIOTask::GetExtractedSize(base::FilePath source_file) {
+  unzip::GetExtractedSize(unzip::LaunchUnzipper(), source_file,
+                          base::BindOnce(&ExtractIOTask::ZipSizeCallback,
+                                         weak_ptr_factory_.GetWeakPtr()));
+}
+
+void ExtractIOTask::CheckSizeThenExtract() {
+  for (const EntryStatus& source : progress_.sources) {
+    base::SequencedTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::BindOnce(&ExtractIOTask::GetExtractedSize,
+                       weak_ptr_factory_.GetWeakPtr(), source.url.path()));
+  }
+}
+
 void ExtractIOTask::Execute(IOTask::ProgressCallback progress_callback,
                             IOTask::CompleteCallback complete_callback) {
   progress_callback_ = std::move(progress_callback);
@@ -99,14 +137,7 @@ void ExtractIOTask::Execute(IOTask::ProgressCallback progress_callback,
     progress_.state = State::kError;
     Complete();
   } else {
-    for (size_t index = 0; index < progress_.sources.size(); ++index) {
-      const EntryStatus& source = progress_.sources[index];
-      const base::FilePath source_file = source.url.path().BaseName();
-      util::GenerateUnusedFilename(
-          parent_folder_, source_file.RemoveExtension(), file_system_context_,
-          base::BindOnce(&ExtractIOTask::ExtractArchive,
-                         weak_ptr_factory_.GetWeakPtr(), index));
-    }
+    CheckSizeThenExtract();
   }
 }
 

chrome/browser/ash/file_manager/extract_io_task.h

@@ -47,6 +47,14 @@ class ExtractIOTask : public IOTask {
       size_t index,
       base::FileErrorOr<storage::FileSystemURL> destination_result);
 
+  void ExtractAllSources();
+
+  void ZipSizeCallback(unzip::mojom::SizePtr size_info);
+
+  void GetExtractedSize(base::FilePath source_file);
+
+  void CheckSizeThenExtract();
+
   // URLs of the files that have archives in them for extraction.
   const std::vector<storage::FileSystemURL> source_urls_;
 

components/services/unzip/public/cpp/unzip.cc

@@ -112,6 +112,33 @@ class DetectEncodingParams : public base::RefCounted<DetectEncodingParams> {
   const base::TimeTicks start_time_ = base::TimeTicks::Now();
 };
 
+class GetExtractedSizeParams : public base::RefCounted<GetExtractedSizeParams> {
+ public:
+  GetExtractedSizeParams(mojo::PendingRemote<mojom::Unzipper> unzipper,
+                         GetExtractedSizeCallback callback)
+      : unzipper_(std::move(unzipper)), callback_(std::move(callback)) {}
+
+  mojo::Remote<mojom::Unzipper>& unzipper() { return unzipper_; }
+
+  void InvokeCallback(mojom::SizePtr size_info) {
+    if (callback_) {
+      // TODO(crbug.com/953256) Add UMA timing.
+      std::move(callback_).Run(std::move(size_info));
+    }
+
+    unzipper_.reset();
+  }
+
+ private:
+  friend class base::RefCounted<GetExtractedSizeParams>;
+
+  ~GetExtractedSizeParams() = default;
+
+  // The Remote is stored so it does not get deleted before the callback runs.
+  mojo::Remote<mojom::Unzipper> unzipper_;
+  GetExtractedSizeCallback callback_;
+};
+
 void DoUnzip(mojo::PendingRemote<mojom::Unzipper> unzipper,
              const base::FilePath& zip_path,
              const base::FilePath& output_dir,
@@ -177,6 +204,32 @@ void DoDetectEncoding(mojo::PendingRemote<mojom::Unzipper> unzipper,
       base::BindOnce(&DetectEncodingParams::InvokeCallback, params));
 }
 
+void DoGetExtractedSize(mojo::PendingRemote<mojom::Unzipper> unzipper,
+                        const base::FilePath& zip_path,
+                        GetExtractedSizeCallback result_callback) {
+  base::File zip_file(zip_path, base::File::FLAG_OPEN | base::File::FLAG_READ);
+  unzip::mojom::SizePtr size_info = unzip::mojom::Size::New(false, 0);
+  if (!zip_file.IsValid()) {
+    LOG(ERROR) << "Cannot open ZIP archive " << Redact(zip_path) << ": "
+               << base::File::ErrorToString(zip_file.error_details());
+    std::move(result_callback).Run(std::move(size_info));
+    return;
+  }
+
+  // |result_callback| is shared between the connection error handler and the
+  // GetExtractedSize call using a refcounted GetExtractedSizeParams object that
+  // owns |result_callback|.
+  auto params = base::MakeRefCounted<GetExtractedSizeParams>(
+      std::move(unzipper), std::move(result_callback));
+
+  params->unzipper().set_disconnect_handler(base::BindOnce(
+      &GetExtractedSizeParams::InvokeCallback, params, std::move(size_info)));
+
+  params->unzipper()->GetExtractedSize(
+      std::move(zip_file),
+      base::BindOnce(&GetExtractedSizeParams::InvokeCallback, params));
+}
+
 }  // namespace
 
 void Unzip(mojo::PendingRemote<mojom::Unzipper> unzipper,
@@ -239,4 +292,18 @@ void DetectEncoding(mojo::PendingRemote<mojom::Unzipper> unzipper,
                                         std::move(result_callback))));
 }
 
+void GetExtractedSize(mojo::PendingRemote<mojom::Unzipper> unzipper,
+                      const base::FilePath& zip_path,
+                      GetExtractedSizeCallback result_callback) {
+  const scoped_refptr<base::SequencedTaskRunner> runner =
+      base::ThreadPool::CreateSequencedTaskRunner(
+          {base::TaskPriority::USER_VISIBLE, base::MayBlock(),
+           base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
+  runner->PostTask(
+      FROM_HERE,
+      base::BindOnce(&DoGetExtractedSize, std::move(unzipper), zip_path,
+                     base::BindPostTask(base::SequencedTaskRunnerHandle::Get(),
+                                        std::move(result_callback))));
+}
+
 }  // namespace unzip

components/services/unzip/public/cpp/unzip.h

@@ -44,6 +44,11 @@ void DetectEncoding(mojo::PendingRemote<mojom::Unzipper> unzipper,
                     const base::FilePath& zip_file,
                     DetectEncodingCallback result_callback);
 
+using GetExtractedSizeCallback = base::OnceCallback<void(mojom::SizePtr)>;
+void GetExtractedSize(mojo::PendingRemote<mojom::Unzipper> unzipper,
+                      const base::FilePath& zip_file,
+                      GetExtractedSizeCallback result_callback);
+
 }  // namespace unzip
 
 #endif  // COMPONENTS_SERVICES_UNZIP_PUBLIC_CPP_UNZIP_H_

components/services/unzip/public/cpp/unzip_unittest.cc

@@ -125,6 +125,24 @@ class UnzipTest : public testing::Test {
     return result;
   }
 
+  mojom::Size DoGetExtractedSize(const base::FilePath& zip_file) {
+    mojo::PendingRemote<mojom::Unzipper> unzipper;
+    receivers_.Add(&unzipper_, unzipper.InitWithNewPipeAndPassReceiver());
+
+    base::RunLoop run_loop;
+    mojom::Size result;
+
+    GetExtractedSizeCallback result_callback =
+        base::BindLambdaForTesting([&](mojom::SizePtr size_info) {
+          result = *size_info;
+          run_loop.QuitClosure().Run();
+        });
+
+    GetExtractedSize(std::move(unzipper), zip_file, std::move(result_callback));
+    run_loop.Run();
+    return result;
+  }
+
  protected:
   void SetUp() override {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
@@ -227,6 +245,17 @@ TEST_F(UnzipTest, DetectEncodingSjis) {
   }
 }
 
+TEST_F(UnzipTest, GetExtractedSize) {
+  mojom::Size result = DoGetExtractedSize(GetArchivePath("good_archive.zip"));
+  EXPECT_TRUE(result.is_valid);
+  EXPECT_EQ(137, static_cast<int64_t>(result.value));
+}
+
+TEST_F(UnzipTest, GetExtractedSizeBrokenArchive) {
+  mojom::Size result = DoGetExtractedSize(GetArchivePath("bad_archive.zip"));
+  EXPECT_FALSE(result.is_valid);
+}
+
 TEST_F(UnzipTest, UnzipWithOptions) {
   EXPECT_TRUE(
       DoUnzipWithOptions(GetArchivePath("good_archive.zip"), unzip_dir_));

components/services/unzip/public/mojom/unzipper.mojom

@@ -15,6 +15,13 @@ struct UnzipOptions {
   string encoding;
 };
 
+struct Size {
+  // Boolean set to false if broken sizes are stored in the ZIP archive.
+  bool is_valid;
+  // Predicted size as read from the ZIP archive (not trustable).
+  uint64 value;
+};
+
 // UnzipFilter is used to call back into the caller to check if
 // the supplied |path| should be unpacked.
 interface UnzipFilter {
@@ -43,4 +50,7 @@ interface Unzipper {
   // third_party/ced/src/util/encodings/encodings.pb.h
   // or UNKNOWN_ENCODING in case of error.
   DetectEncoding(mojo_base.mojom.ReadOnlyFile zip_file) => (int32 encoding);
+
+  // Returns the size of the extracted archive in bytes.
+  GetExtractedSize(mojo_base.mojom.ReadOnlyFile zip_file) => (Size size);
 };

components/services/unzip/unzipper_impl.cc

@@ -222,4 +222,40 @@ void UnzipperImpl::DetectEncoding(base::File zip_file,
   std::move(callback).Run(encoding);
 }
 
+void UnzipperImpl::GetExtractedSize(base::File zip_file,
+                                    GetExtractedSizeCallback callback) {
+  DCHECK(zip_file.IsValid());
+
+  // Open ZIP archive for reading.
+  zip::ZipReader reader;
+  if (!reader.OpenFromPlatformFile(zip_file.GetPlatformFile())) {
+    LOG(ERROR) << "Cannot decode ZIP archive from file handle "
+               << zip_file.GetPlatformFile();
+    unzip::mojom::SizePtr size_info = unzip::mojom::Size::New(false, 0);
+    std::move(callback).Run(std::move(size_info));
+    return;
+  }
+
+  int64_t size = 0;
+  bool valid = true;
+
+  // Iterate over file entries of the ZIP archive.
+  while (const zip::ZipReader::Entry* const entry = reader.Next()) {
+    // Check for (invalid) size stored.
+    if (entry->original_size < 0 ||
+        entry->original_size > std::numeric_limits<int64_t>::max() - size) {
+      LOG(ERROR) << "ZIP bad size info from file handle "
+                 << zip_file.GetPlatformFile();
+      valid = false;
+      break;
+    }
+    // Accumulate size (since original_size is signed, ignore invalid sizes).
+    if (entry->original_size > 0) {
+      size += entry->original_size;
+    }
+  }
+  unzip::mojom::SizePtr size_info = unzip::mojom::Size::New(valid, size);
+  std::move(callback).Run(std::move(size_info));
+}
+
 }  // namespace unzip

components/services/unzip/unzipper_impl.h

@@ -39,6 +39,9 @@ class UnzipperImpl : public mojom::Unzipper {
   void DetectEncoding(base::File zip_file,
                       DetectEncodingCallback callback) override;
 
+  void GetExtractedSize(base::File zip_file,
+                        GetExtractedSizeCallback callback) override;
+
   mojo::Receiver<mojom::Unzipper> receiver_{this};
 };