From e977b5bca269cd3f696fa5b650746e045a604916 Mon Sep 17 00:00:00 2001 From: Holden Warriner Date: Tue, 27 Jun 2023 17:47:37 -0700 Subject: [PATCH] Add support to unzip from a string representation (#699) This is needed for the in-memory updates feature, where the CRX package will be kept in memory until it's unpacked. Separate commits will have the changes to download the CRX to memory and verify a CRX from memory. b/158043520 --- cobalt/updater/unzipper.cc | 16 ++ components/update_client/unzip/unzip_impl.cc | 7 + .../update_client/unzip/unzip_impl_cobalt.cc | 11 +- components/update_client/unzipper.h | 6 + .../shared/platform_configuration/BUILD.gn | 11 ++ third_party/zlib/google/zip.cc | 67 +++++++- third_party/zlib/google/zip.h | 24 ++- third_party/zlib/google/zip_unittest.cc | 150 ++++++++++++++++++ 8 files changed, 281 insertions(+), 11 deletions(-) diff --git a/cobalt/updater/unzipper.cc b/cobalt/updater/unzipper.cc index 1855b3a090fd..22d6423a65e8 100644 --- a/cobalt/updater/unzipper.cc +++ b/cobalt/updater/unzipper.cc @@ -14,7 +14,9 @@ #include "cobalt/updater/unzipper.h" +#include #include + #include "base/callback.h" #include "base/files/file_path.h" #include "starboard/time.h" @@ -40,6 +42,20 @@ class UnzipperImpl : public update_client::Unzipper { LOG(INFO) << "Unzip took " << time_unzip_took / kSbTimeMillisecond << " milliseconds."; } + +#if defined(IN_MEMORY_UPDATES) + void Unzip(const std::string& zip_str, const base::FilePath& output_path, + UnzipCompleteCallback callback) override { + SbTimeMonotonic time_before_unzip = SbTimeGetMonotonicNow(); + std::move(callback).Run(zip::Unzip(zip_str, output_path)); + SbTimeMonotonic time_unzip_took = + SbTimeGetMonotonicNow() - time_before_unzip; + LOG(INFO) << "Unzip from string"; + LOG(INFO) << "output_path = " << output_path; + LOG(INFO) << "Unzip took " << time_unzip_took / kSbTimeMillisecond + << " milliseconds."; + } +#endif }; } // namespace diff --git a/components/update_client/unzip/unzip_impl.cc b/components/update_client/unzip/unzip_impl.cc index 1fa905818c0e..3dcfab8b5123 100644 --- a/components/update_client/unzip/unzip_impl.cc +++ b/components/update_client/unzip/unzip_impl.cc @@ -20,6 +20,13 @@ class UnzipperImpl : public Unzipper { UnzipCompleteCallback callback) override { unzip::Unzip(callback_.Run(), zip_file, destination, std::move(callback)); } +#if defined(IN_MEMORY_UPDATES) + void Unzip(const std::string& zip_str, + const base::FilePath& destination, + UnzipCompleteCallback callback) override { + unzip::Unzip(callback_.Run(), zip_str, destination, std::move(callback)); + } +#endif private: const UnzipChromiumFactory::Callback callback_; diff --git a/components/update_client/unzip/unzip_impl_cobalt.cc b/components/update_client/unzip/unzip_impl_cobalt.cc index 40882b53ddf4..9b7b82bec7c9 100644 --- a/components/update_client/unzip/unzip_impl_cobalt.cc +++ b/components/update_client/unzip/unzip_impl_cobalt.cc @@ -1,10 +1,12 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. +// Copyright 2019 The Cobalt Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. #include "components/update_client/unzip/unzip_impl_cobalt.h" +#include #include + #include "base/callback.h" #include "base/files/file_path.h" #include "starboard/time.h" @@ -23,6 +25,13 @@ class UnzipperImpl : public update_client::Unzipper { UnzipCompleteCallback callback) override { std::move(callback).Run(zip::Unzip(zip_path, output_path)); } +#if defined(IN_MEMORY_UPDATES) + void Unzip(const std::string& zip_str, + const base::FilePath& output_path, + UnzipCompleteCallback callback) override { + std::move(callback).Run(zip::Unzip(zip_str, output_path)); + } +#endif }; } // namespace diff --git a/components/update_client/unzipper.h b/components/update_client/unzipper.h index 6aed58c5446d..5d1e6029ee12 100644 --- a/components/update_client/unzipper.h +++ b/components/update_client/unzipper.h @@ -25,6 +25,12 @@ class Unzipper { const base::FilePath& destination, UnzipCompleteCallback callback) = 0; +#if defined(IN_MEMORY_UPDATES) + virtual void Unzip(const std::string& zip_str, + const base::FilePath& destination, + UnzipCompleteCallback callback) = 0; +#endif + protected: Unzipper() = default; diff --git a/starboard/evergreen/shared/platform_configuration/BUILD.gn b/starboard/evergreen/shared/platform_configuration/BUILD.gn index ddaf2799667e..6b154a0f2845 100644 --- a/starboard/evergreen/shared/platform_configuration/BUILD.gn +++ b/starboard/evergreen/shared/platform_configuration/BUILD.gn @@ -74,6 +74,17 @@ config("platform_configuration") { # By default, pulls in some X11 headers that have some # nasty macros (|Status|, for example) that conflict with Chromium base. "MESA_EGL_NO_X11_HEADERS", + + # During Evergreen updates the CRX package is kept in-memory, instead of + # on the file system, before getting unpacked. + # TODO(b/158043520): we need to make significant customizations to Chromium + # code to implement this feature and this macro allows us to switch back + # to the legacy behavior during development to verify the customizations + # are made cleanly. Once the feature is complete we may want to remove + # existing customizations that enable the legacy behavior for Cobalt builds, + # change IN_MEMORY_UPDATES references to USE_COBALT_CUSTOMIZATIONS + # references, and remove this macro. + "IN_MEMORY_UPDATES", ] if (is_debug) { diff --git a/third_party/zlib/google/zip.cc b/third_party/zlib/google/zip.cc index 1559ad91285f..54196a630ce2 100644 --- a/third_party/zlib/google/zip.cc +++ b/third_party/zlib/google/zip.cc @@ -176,6 +176,13 @@ bool Unzip(const base::FilePath& src_file, const base::FilePath& dest_dir) { return UnzipWithFilterCallback( src_file, dest_dir, base::BindRepeating(&ExcludeNoFilesFilter), true); } +#if defined(IN_MEMORY_UPDATES) +bool Unzip(const std::string& src_str, const base::FilePath& dest_dir) { + return UnzipWithFilterCallback( + src_str, dest_dir, base::BindRepeating(&ExcludeNoFilesFilter), true); +} +#endif + bool UnzipWithFilterCallback(const base::FilePath& src_file, const base::FilePath& dest_dir, @@ -200,17 +207,24 @@ bool UnzipWithFilterCallback(const base::FilePath& src_file, #endif } +#if defined(IN_MEMORY_UPDATES) +bool UnzipWithFilterCallback(const std::string& src_str, + const base::FilePath& dest_dir, + const FilterCallback& filter_cb, + bool log_skipped_files) { + return UnzipWithFilterAndWriters( + src_str, base::BindRepeating(&CreateFilePathWriterDelegate, dest_dir), + base::BindRepeating(&CreateDirectory, dest_dir), filter_cb, + log_skipped_files); +} +#endif + #if defined(STARBOARD) -bool UnzipWithFilterAndWriters(const base::FilePath& src_file, +bool UnzipWithFilterAndWriters(ZipReader& reader, const WriterFactory& writer_factory, const DirectoryCreator& directory_creator, const FilterCallback& filter_cb, bool log_skipped_files) { - ZipReader reader; - if (!reader.Open(src_file)) { - DLOG(WARNING) << "Failed to open src_file " << src_file; - return false; - } while (reader.HasMore()) { if (!reader.OpenCurrentEntryInZip()) { DLOG(WARNING) << "Failed to open the current file in zip"; @@ -244,7 +258,44 @@ bool UnzipWithFilterAndWriters(const base::FilePath& src_file, } return true; } -#else + +bool UnzipWithFilterAndWriters(const base::FilePath& src_file, + const WriterFactory& writer_factory, + const DirectoryCreator& directory_creator, + const FilterCallback& filter_cb, + bool log_skipped_files) { + ZipReader reader; + if (!reader.Open(src_file)) { + DLOG(WARNING) << "Failed to open src_file " << src_file; + return false; + } + return UnzipWithFilterAndWriters(reader, + writer_factory, + directory_creator, + filter_cb, + log_skipped_files); +} + +#if defined(IN_MEMORY_UPDATES) +bool UnzipWithFilterAndWriters(const std::string& src_str, + const WriterFactory& writer_factory, + const DirectoryCreator& directory_creator, + const FilterCallback& filter_cb, + bool log_skipped_files) { + ZipReader reader; + if (!reader.OpenFromString(src_str)) { + DLOG(WARNING) << "Failed to open src_str"; + return false; + } + return UnzipWithFilterAndWriters(reader, + writer_factory, + directory_creator, + filter_cb, + log_skipped_files); +} +#endif // defined(IN_MEMORY_UPDATES) +#else // defined(STARBOARD) + bool UnzipWithFilterAndWriters(const base::PlatformFile& src_file, const WriterFactory& writer_factory, const DirectoryCreator& directory_creator, @@ -288,7 +339,7 @@ bool UnzipWithFilterAndWriters(const base::PlatformFile& src_file, } return true; } -#endif +#endif // STARBOARD bool ZipWithFilterCallback(const base::FilePath& src_dir, const base::FilePath& dest_file, diff --git a/third_party/zlib/google/zip.h b/third_party/zlib/google/zip.h index dad3f41fd8f2..6c4154e72fad 100644 --- a/third_party/zlib/google/zip.h +++ b/third_party/zlib/google/zip.h @@ -158,6 +158,14 @@ bool UnzipWithFilterCallback(const base::FilePath& zip_file, const FilterCallback& filter_cb, bool log_skipped_files); +#if defined(IN_MEMORY_UPDATES) +// An overload for a zip represented as a string. +bool UnzipWithFilterCallback(const std::string& zip_str, + const base::FilePath& dest_dir, + const FilterCallback& filter_cb, + bool log_skipped_files); +#endif + // Unzip the contents of zip_file, using the writers provided by writer_factory. // For each file in zip_file, include it only if the callback |filter_cb| // returns true. Otherwise omit it. @@ -174,16 +182,28 @@ bool UnzipWithFilterAndWriters(const base::FilePath& zip_file, const DirectoryCreator& directory_creator, const FilterCallback& filter_cb, bool log_skipped_files); -#else +#if defined(IN_MEMORY_UPDATES) +// An overload for a zip represented as a string. +bool UnzipWithFilterAndWriters(const std::string& zip_str, + const WriterFactory& writer_factory, + const DirectoryCreator& directory_creator, + const FilterCallback& filter_cb, + bool log_skipped_files); +#endif // defined(IN_MEMORY_UPDATES) +#else // defined(STARBOARD) bool UnzipWithFilterAndWriters(const base::PlatformFile& zip_file, const WriterFactory& writer_factory, const DirectoryCreator& directory_creator, const FilterCallback& filter_cb, bool log_skipped_files); -#endif +#endif // defined(STARBOARD) // Unzip the contents of zip_file into dest_dir. bool Unzip(const base::FilePath& zip_file, const base::FilePath& dest_dir); +#if defined(IN_MEMORY_UPDATES) +// Unzip the contents of zip_str into dest_dir. +bool Unzip(const std::string& zip_str, const base::FilePath& dest_dir); +#endif } // namespace zip diff --git a/third_party/zlib/google/zip_unittest.cc b/third_party/zlib/google/zip_unittest.cc index b92c65074b22..69763a0cd23d 100644 --- a/third_party/zlib/google/zip_unittest.cc +++ b/third_party/zlib/google/zip_unittest.cc @@ -196,6 +196,12 @@ class ZipTest : public PlatformTest { ASSERT_TRUE(base::PathExists(path)) << "no file " << path.value(); ASSERT_TRUE(zip::Unzip(path, test_dir_)); +#if defined(STARBOARD) + VerifyUnzippedContents(expect_hidden_files); + } + void VerifyUnzippedContents(bool expect_hidden_files) { +#endif + base::FilePath original_dir; ASSERT_TRUE(GetTestDataDirectory(&original_dir)); original_dir = original_dir.AppendASCII("test"); @@ -238,6 +244,21 @@ class ZipTest : public PlatformTest { EXPECT_EQ(expected_count, count); } +#if defined(IN_MEMORY_UPDATES) + void ReadFileToString(const base::FilePath::StringType& filename, + std::string* contents) { + base::FilePath test_dir; + ASSERT_TRUE(GetTestDataDirectory(&test_dir)); + ASSERT_TRUE(base::ReadFileToString(test_dir.Append(filename), contents)); + } + + void TestUnzipString(const std::string& zip_src, bool expect_hidden_files) { + ASSERT_TRUE(zip::Unzip(zip_src, test_dir_)); + + VerifyUnzippedContents(expect_hidden_files); + } +#endif + // This function does the following: // 1) Creates a test.txt file with the given last modification timestamp // 2) Zips test.txt and extracts it back into a different location. @@ -311,10 +332,28 @@ TEST_F(ZipTest, Unzip) { TestUnzipFile(FILE_PATH_LITERAL("test.zip"), true); } +#if defined(IN_MEMORY_UPDATES) +TEST_F(ZipTest, UnzipFromString) { + std::string zip_str; + ReadFileToString(FILE_PATH_LITERAL("test.zip"), &zip_str); + + TestUnzipString(zip_str, true); +} +#endif + TEST_F(ZipTest, UnzipUncompressed) { TestUnzipFile(FILE_PATH_LITERAL("test_nocompress.zip"), true); } +#if defined(IN_MEMORY_UPDATES) +TEST_F(ZipTest, UnzipFromStringUncompressed) { + std::string zip_str; + ReadFileToString(FILE_PATH_LITERAL("test_nocompress.zip"), &zip_str); + + TestUnzipString(zip_str, true); +} +#endif + TEST_F(ZipTest, UnzipEvil) { base::FilePath path; ASSERT_TRUE(GetTestDataDirectory(&path)); @@ -330,6 +369,24 @@ TEST_F(ZipTest, UnzipEvil) { ASSERT_FALSE(base::PathExists(evil_file)); } +#if defined(IN_MEMORY_UPDATES) +TEST_F(ZipTest, UnzipFromStringEvil) { + std::string zip_str; + ReadFileToString(FILE_PATH_LITERAL("evil.zip"), &zip_str); + + // Unzip the zip file into a sub directory of test_dir_ so evil.zip + // won't create a persistent file outside test_dir_ in case of a + // failure. + base::FilePath output_dir = test_dir_.AppendASCII("out"); + ASSERT_FALSE(zip::Unzip(zip_str, output_dir)); + + base::FilePath evil_file = output_dir; + evil_file = evil_file.AppendASCII( + "../levilevilevilevilevilevilevilevilevilevilevilevil"); + ASSERT_FALSE(base::PathExists(evil_file)); +} +#endif + TEST_F(ZipTest, UnzipEvil2) { base::FilePath path; ASSERT_TRUE(GetTestDataDirectory(&path)); @@ -345,6 +402,24 @@ TEST_F(ZipTest, UnzipEvil2) { ASSERT_FALSE(base::PathExists(evil_file)); } +#if defined(IN_MEMORY_UPDATES) +TEST_F(ZipTest, UnzipFromStringEvil2) { + std::string zip_str; + // The zip file contains an evil file with invalid UTF-8 in its file + // name. + ReadFileToString(FILE_PATH_LITERAL("evil_via_invalid_utf8.zip"), &zip_str); + + // See the comment at UnzipEvil() for why we do this. + base::FilePath output_dir = test_dir_.AppendASCII("out"); + // This should fail as it contains an evil file. + ASSERT_FALSE(zip::Unzip(zip_str, output_dir)); + + base::FilePath evil_file = output_dir; + evil_file = evil_file.AppendASCII("../evil.txt"); + ASSERT_FALSE(base::PathExists(evil_file)); +} +#endif + TEST_F(ZipTest, UnzipWithFilter) { auto filter = base::BindRepeating([](const base::FilePath& path) { return path.BaseName().MaybeAsASCII() == "foo.txt"; @@ -381,6 +456,46 @@ TEST_F(ZipTest, UnzipWithFilter) { ASSERT_EQ(0, extracted_count); } +#if defined(IN_MEMORY_UPDATES) +TEST_F(ZipTest, UnzipFromStringWithFilter) { + auto filter = base::BindRepeating([](const base::FilePath& path) { + return path.BaseName().MaybeAsASCII() == "foo.txt"; + }); + std::string zip_str; + ReadFileToString(FILE_PATH_LITERAL("test.zip"), &zip_str); + + ASSERT_TRUE(zip::UnzipWithFilterCallback(zip_str, + test_dir_, filter, false)); + + // Only foo.txt should have been extracted. The following paths should not + // be extracted: + // foo/ + // foo/bar.txt + // foo/bar/ + // foo/bar/.hidden + // foo/bar/baz.txt + // foo/bar/quux.txt + ASSERT_TRUE(base::PathExists(test_dir_.AppendASCII("foo.txt"))); + base::FileEnumerator extractedFiles( + test_dir_, + false, // Do not enumerate recursively - the file must be in the root. + base::FileEnumerator::FileType::FILES); + int extracted_count = 0; + while (!extractedFiles.Next().empty()) + ++extracted_count; + ASSERT_EQ(1, extracted_count); + + base::FileEnumerator extractedDirs( + test_dir_, + false, // Do not enumerate recursively - we require zero directories. + base::FileEnumerator::FileType::DIRECTORIES); + extracted_count = 0; + while (!extractedDirs.Next().empty()) + ++extracted_count; + ASSERT_EQ(0, extracted_count); +} +#endif + TEST_F(ZipTest, UnzipWithDelegates) { auto filter = base::BindRepeating([](const base::FilePath& path) { return true; }); @@ -419,6 +534,41 @@ TEST_F(ZipTest, UnzipWithDelegates) { ASSERT_TRUE(base::PathExists(dir_foo_bar.AppendASCII("quux.txt"))); } +#if defined(IN_MEMORY_UPDATES) +TEST_F(ZipTest, UnzipFromStringWithDelegates) { + auto filter = + base::BindRepeating([](const base::FilePath& path) { return true; }); + auto dir_creator = base::BindRepeating( + [](const base::FilePath& extract_dir, const base::FilePath& entry_path) { + return base::CreateDirectory(extract_dir.Append(entry_path)); + }, + test_dir_); + auto writer = base::BindRepeating( + [](const base::FilePath& extract_dir, const base::FilePath& entry_path) + -> std::unique_ptr { + return std::make_unique( + extract_dir.Append(entry_path)); + }, + test_dir_); + std::string zip_str; + ReadFileToString(FILE_PATH_LITERAL("test.zip"), &zip_str); + + ASSERT_TRUE(zip::UnzipWithFilterAndWriters( + zip_str, writer, dir_creator, filter, false)); + + base::FilePath dir = test_dir_; + base::FilePath dir_foo = dir.AppendASCII("foo"); + base::FilePath dir_foo_bar = dir_foo.AppendASCII("bar"); + ASSERT_TRUE(base::PathExists(dir.AppendASCII("foo.txt"))); + ASSERT_TRUE(base::PathExists(dir_foo)); + ASSERT_TRUE(base::PathExists(dir_foo.AppendASCII("bar.txt"))); + ASSERT_TRUE(base::PathExists(dir_foo_bar)); + ASSERT_TRUE(base::PathExists(dir_foo_bar.AppendASCII(".hidden"))); + ASSERT_TRUE(base::PathExists(dir_foo_bar.AppendASCII("baz.txt"))); + ASSERT_TRUE(base::PathExists(dir_foo_bar.AppendASCII("quux.txt"))); +} +#endif + TEST_F(ZipTest, Zip) { base::FilePath src_dir; ASSERT_TRUE(GetTestDataDirectory(&src_dir));