diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2022-05-11 05:14:13 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2022-05-11 05:14:13 +0000 |
commit | 9da15b046a9397c14531429573a96a19359884dc (patch) | |
tree | 650e1d3a1f5269380163831380aad1e24c6907b6 | |
parent | 3544c2604fc833c89a68e5fb4e563cf0a231cfb0 (diff) | |
parent | 4b17695e63c6e11735f0f7a3d3754b1cac5da348 (diff) | |
download | zlib-9da15b046a9397c14531429573a96a19359884dc.tar.gz |
Snap for 8570526 from 4b17695e63c6e11735f0f7a3d3754b1cac5da348 to mainline-networking-releaseaml_net_331412000aml_net_331313030aml_net_331313010aml_net_331110020aml_net_331011030aml_net_330910010aml_net_330811010
Change-Id: I4f0c455966722acc45d61d41576363e3f44c4d72
39 files changed, 3165 insertions, 1359 deletions
@@ -17,18 +17,14 @@ license { srcs_opt = [ "adler32_simd.c", // See https://chromium-review.googlesource.com/749732. -// TODO: causes `atest org.apache.harmony.tests.java.util.zip.DeflaterTest` failures. -// "contrib/optimizations/inffast_chunk.c", -// "contrib/optimizations/inflate.c", + // TODO: causes `atest org.apache.harmony.tests.java.util.zip.DeflaterTest` failures. + // "contrib/optimizations/inffast_chunk.c", + // "contrib/optimizations/inflate.c", // This file doesn't build for non-neon, so it can't be in the main srcs. "crc32_simd.c", ] cflags_arm = [ - // Since we're building for the platform, we claim to be Linux rather than - // Android so we use getauxval() directly instead of the NDK - // android_getCpuFeatures which isn't available to us anyway. - "-DARMV8_OS_LINUX", // Testing with zlib_bench shows -O3 is a win for ARM but a bit of a wash // for x86, so match the BUILD file in only enabling this for ARM. "-O3", @@ -41,8 +37,8 @@ cflags_arm_neon = [ "-UCPU_NO_SIMD", // We no longer support non-Neon platform builds, but the NDK just has one libz. "-DADLER32_SIMD_NEON", -// TODO: causes `atest org.apache.harmony.tests.java.util.zip.DeflaterTest` failures. -// "-DINFLATE_CHUNK_SIMD_NEON", + // TODO: causes `atest org.apache.harmony.tests.java.util.zip.DeflaterTest` failures. + // "-DINFLATE_CHUNK_SIMD_NEON", // HWCAP_CRC32 is checked at runtime, so it's okay to turn crc32 // acceleration on for both 32- and 64-bit. "-DCRC32_ARMV8_CRC32", @@ -53,8 +49,8 @@ cflags_arm64 = cflags_arm + cflags_arm_neon cflags_x86 = [ // See ARMV8_OS_LINUX above. "-DX86_NOT_WINDOWS", -// TODO: see arm above. -// "-DINFLATE_CHUNK_SIMD_SSE2", + // TODO: see arm above. + // "-DINFLATE_CHUNK_SIMD_SSE2", // Android's host CPU feature requirements are *lower* than the // corresponding device CPU feature requirements, so it's easier to just // say "no SIMD for you" rather than specificially disable SSSE3. @@ -108,6 +104,8 @@ cc_defaults { "-DHAVE_HIDDEN", // We do support const, so turn that on. "-DZLIB_CONST", + // Enable -O3 as per chromium. + "-O3", "-Wall", "-Werror", "-Wno-unused", @@ -128,7 +126,7 @@ cc_defaults { neon: { cflags: cflags_arm_neon, srcs: srcs_opt, - } + }, }, arm64: { cflags: cflags_arm64 + cflags_64, @@ -144,12 +142,33 @@ cc_defaults { }, }, target: { + android_arm: { + cflags: [ + // Since we're building for the platform, we claim to be Linux rather than + // Android so we use getauxval() directly instead of the NDK + // android_getCpuFeatures which isn't available to us anyway. + "-DARMV8_OS_LINUX", + ], + }, android_x86: { cflags: cflags_android_x86, }, android_x86_64: { cflags: cflags_android_x86, }, + darwin_arm64: { + cflags: [ + "-DARMV8_OS_MACOS", + ], + }, + linux_arm64: { + cflags: [ + // Since we're building for the platform, we claim to be Linux rather than + // Android so we use getauxval() directly instead of the NDK + // android_getCpuFeatures which isn't available to us anyway. + "-DARMV8_OS_LINUX", + ], + }, }, } @@ -204,6 +223,8 @@ cc_library { "-DHAVE_HIDDEN", // We do support const, so turn that on. "-DZLIB_CONST", + // Enable -O3 as per chromium + "-O3", "-Wall", "-Werror", "-Wno-unused", @@ -221,7 +242,11 @@ cc_library { cc_binary_host { name: "minigzip", srcs: ["contrib/minigzip/minigzip.c"], - cflags: ["-Wall", "-Werror", "-DUSE_MMAP"], + cflags: [ + "-Wall", + "-Werror", + "-DUSE_MMAP", + ], static_libs: ["libz"], stl: "none", } @@ -229,14 +254,22 @@ cc_binary_host { cc_binary { name: "zlib_bench", srcs: ["contrib/bench/zlib_bench.cc"], - cflags: ["-Wall", "-Werror"], + cflags: [ + "-Wall", + "-Werror", + "-Wno-unused-parameter", + ], host_supported: true, shared_libs: ["libz"], // We build zlib_bench32 and zlib_bench64 so it's easy to test LP32. compile_multilib: "both", multilib: { - lib32: { suffix: "32", }, - lib64: { suffix: "64", }, + lib32: { + suffix: "32", + }, + lib64: { + suffix: "64", + }, }, } @@ -27,6 +27,17 @@ config("zlib_internal_config") { # Enable zlib's asserts in debug and fuzzer builds. defines += [ "ZLIB_DEBUG" ] } + + if (is_win && !is_clang) { + # V8 supports building with msvc, these silence some warnings that + # causes compilation to fail (https://crbug.com/1255096). + cflags = [ + "/wd4244", + "/wd4100", + "/wd4702", + "/wd4127", + ] + } } source_set("zlib_common_headers") { @@ -419,6 +430,48 @@ executable("zlib_bench") { configs += [ "//build/config/compiler:no_chromium_code" ] } +if (!is_win || target_os != "winuwp") { + executable("minizip_bin") { + include_dirs = [ "." ] + + sources = [ "contrib/minizip/minizip.c" ] + + if (is_clang) { + cflags = [ "-Wno-incompatible-pointer-types-discards-qualifiers" ] + } + + if (!is_debug) { + configs -= [ "//build/config/compiler:default_optimization" ] + configs += [ "//build/config/compiler:optimize_speed" ] + } + + deps = [ ":minizip" ] + + configs -= [ "//build/config/compiler:chromium_code" ] + configs += [ "//build/config/compiler:no_chromium_code" ] + } + + executable("miniunz_bin") { + include_dirs = [ "." ] + + sources = [ "contrib/minizip/miniunz.c" ] + + if (is_clang) { + cflags = [ "-Wno-incompatible-pointer-types-discards-qualifiers" ] + } + + if (!is_debug) { + configs -= [ "//build/config/compiler:default_optimization" ] + configs += [ "//build/config/compiler:optimize_speed" ] + } + + deps = [ ":minizip" ] + + configs -= [ "//build/config/compiler:chromium_code" ] + configs += [ "//build/config/compiler:no_chromium_code" ] + } +} + if (build_with_chromium) { test("zlib_unittests") { testonly = true @@ -443,6 +496,9 @@ if (build_with_chromium) { "//testing/gtest", ] + configs -= [ "//build/config/compiler:chromium_code" ] + configs += [ "//build/config/compiler:no_chromium_code" ] + include_dirs = [ "//third_party/googletest/src/googletest/include/gtest", ".", diff --git a/CMakeLists.txt b/CMakeLists.txt new file mode 100644 index 0000000..08c8bd5 --- /dev/null +++ b/CMakeLists.txt @@ -0,0 +1,60 @@ +# Copyright 2021 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# +# Build zlib for Windows x86_64 using MSVC. +# + +project(libz LANGUAGES C) +cmake_minimum_required(VERSION 3.18.1) + +add_library(libz STATIC + # Optimizations for x86-64 + adler32_simd.c + crc32_simd.c + crc_folding.c + fill_window_sse.c + + # Common sources + adler32.c + compress.c + cpu_features.c + crc32.c + deflate.c + gzclose.c + gzlib.c + gzread.c + gzwrite.c + infback.c + inffast.c + inflate.c + inftrees.c + trees.c + uncompr.c + zutil.c +) + +target_compile_definitions(libz PRIVATE + # Enable optimizations: cpu_features.c will enable them at runtime using __cpuid. + ADLER32_SIMD_SSSE3 + CRC32_SIMD_SSE42_PCLMUL + INFLATE_CHUNK_READ_64LE + + X86_WINDOWS + ZLIB_CONST +) + +set(out ${CMAKE_CURRENT_BINARY_DIR}) +configure_file(zconf.h ${out}/dist/include/zconf.h COPYONLY) +configure_file(zlib.h ${out}/dist/include/zlib.h COPYONLY) @@ -5,11 +5,11 @@ third_party { type: GIT value: "https://chromium.googlesource.com/chromium/src/third_party/zlib/" } - version: "eb9ce8c993117f27ea0e5bccc0f2fee2a5323066" + version: "cb89dc607cd4b85d98867f14216c3370109be64d" license_type: NOTICE last_upgrade_date { - year: 2021 - month: 5 - day: 6 + year: 2022 + month: 3 + day: 23 } } @@ -1,5 +1,5 @@ agl@chromium.org cavalcantii@chromium.org cblume@chromium.org -mtklein@google.com +noel@chromium.org scroggo@google.com diff --git a/contrib/bench/zlib_bench.cc b/contrib/bench/zlib_bench.cc index bd06ad3..252560e 100644 --- a/contrib/bench/zlib_bench.cc +++ b/contrib/bench/zlib_bench.cc @@ -193,7 +193,7 @@ void verify_equal(const char* input, size_t size, std::string* output) { exit(3); } -void zlib_file(const char* name, const zlib_wrapper type) { +void zlib_file(const char* name, const zlib_wrapper type, int width) { /* * Read the file data. */ @@ -283,9 +283,9 @@ void zlib_file(const char* name, const zlib_wrapper type) { double inflate_rate_max = length * repeats / mega_byte / utime[0]; // type, block size, compression ratio, etc - printf("%s: [b %dM] bytes %6d -> %6u %4.1f%%", - zlib_wrapper_name(type), block_size / (1 << 20), length, - static_cast<unsigned>(output_length), output_length * 100.0 / length); + printf("%s: [b %dM] bytes %*d -> %*u %4.2f%%", + zlib_wrapper_name(type), block_size / (1 << 20), width, length, width, + unsigned(output_length), output_length * 100.0 / length); // compress / uncompress median (max) rates printf(" comp %5.1f (%5.1f) MB/s uncomp %5.1f (%5.1f) MB/s\n", @@ -300,16 +300,20 @@ char* get_option(int argc, char* argv[], const char* option) { return nullptr; } -bool get_compression(int argc, char* argv[], int* value) { +bool get_compression(int argc, char* argv[], int& value) { if (argn < argc) - *value = isdigit(argv[argn][0]) ? atoi(argv[argn++]) : -1; - return *value >= 0 && *value <= 9; + value = isdigit(argv[argn][0]) ? atoi(argv[argn++]) : -1; + return value >= 0 && value <= 9; } -const char* options = "gzip|zlib|raw [--compression 0:9] [--huffman|--rle]"; +void get_field_width(int argc, char* argv[], int& value) { + value = atoi(argv[argn++]); +} void usage_exit(const char* program) { - printf("usage: %s %s files...", program, options); + static auto* options = + "gzip|zlib|raw [--compression 0:9] [--huffman|--rle] [--field width]"; + printf("usage: %s %s files ...\n", program, options); exit(1); } @@ -324,10 +328,14 @@ int main(int argc, char* argv[]) { else usage_exit(argv[0]); + int file_size_field_width = 0; + while (argn < argc && argv[argn][0] == '-') { if (get_option(argc, argv, "--compression")) { - if (!get_compression(argc, argv, &zlib_compression_level)) + if (!get_compression(argc, argv, zlib_compression_level)) usage_exit(argv[0]); + } else if (get_option(argc, argv, "--field")) { + get_field_width(argc, argv, file_size_field_width); } else if (get_option(argc, argv, "--huffman")) { zlib_strategy = Z_HUFFMAN_ONLY; } else if (get_option(argc, argv, "--rle")) { @@ -339,8 +347,11 @@ int main(int argc, char* argv[]) { if (argn >= argc) usage_exit(argv[0]); + + if (file_size_field_width < 6) + file_size_field_width = 6; while (argn < argc) - zlib_file(argv[argn++], type); + zlib_file(argv[argn++], type, file_size_field_width); return 0; } diff --git a/contrib/minizip/miniunz.c b/contrib/minizip/miniunz.c index 3d65401..08737f6 100644 --- a/contrib/minizip/miniunz.c +++ b/contrib/minizip/miniunz.c @@ -12,7 +12,7 @@ Copyright (C) 2009-2010 Mathias Svensson ( http://result42.com ) */ -#if (!defined(_WIN32)) && (!defined(WIN32)) && (!defined(__APPLE__)) +#if (!defined(_WIN32)) && (!defined(WIN32)) && (!defined(__APPLE__)) && (!defined(__ANDROID_API__)) #ifndef __USE_FILE_OFFSET64 #define __USE_FILE_OFFSET64 #endif @@ -27,7 +27,7 @@ #endif #endif -#ifdef __APPLE__ +#if defined(__APPLE__) || defined(__Fuchsia__) || defined(__ANDROID_API__) // In darwin and perhaps other BSD variants off_t is a 64 bit value, hence no need for specific 64 bit functions #define FOPEN_FUNC(filename, mode) fopen(filename, mode) #define FTELLO_FUNC(stream) ftello(stream) @@ -45,6 +45,7 @@ #include <time.h> #include <errno.h> #include <fcntl.h> +#include <sys/stat.h> #ifdef _WIN32 # include <direct.h> @@ -97,7 +98,7 @@ void change_file_date(filename,dosdate,tmu_date) SetFileTime(hFile,&ftm,&ftLastAcc,&ftm); CloseHandle(hFile); #else -#ifdef unix || __APPLE__ +#if defined(unix) || defined(__APPLE__) || defined(__Fuchsia__) || defined(__ANDROID_API__) struct utimbuf ut; struct tm newdate; newdate.tm_sec = tmu_date.tm_sec; @@ -125,11 +126,9 @@ int mymkdir(dirname) const char* dirname; { int ret=0; -#ifdef _WIN32 +#if defined(_WIN32) ret = _mkdir(dirname); -#elif unix - ret = mkdir (dirname,0775); -#elif __APPLE__ +#elif defined(unix) || defined(__APPLE__) || defined(__Fuchsia__) || defined(__ANDROID_API__) ret = mkdir (dirname,0775); #endif return ret; diff --git a/contrib/minizip/minizip.c b/contrib/minizip/minizip.c index 4288962..b794953 100644 --- a/contrib/minizip/minizip.c +++ b/contrib/minizip/minizip.c @@ -12,8 +12,7 @@ Copyright (C) 2009-2010 Mathias Svensson ( http://result42.com ) */ - -#if (!defined(_WIN32)) && (!defined(WIN32)) && (!defined(__APPLE__)) +#if (!defined(_WIN32)) && (!defined(WIN32)) && (!defined(__APPLE__)) && (!defined(__ANDROID_API__)) #ifndef __USE_FILE_OFFSET64 #define __USE_FILE_OFFSET64 #endif @@ -28,7 +27,7 @@ #endif #endif -#ifdef __APPLE__ +#if defined(__APPLE__) || defined(__Fuchsia__) || defined(__ANDROID_API__) // In darwin and perhaps other BSD variants off_t is a 64 bit value, hence no need for specific 64 bit functions #define FOPEN_FUNC(filename, mode) fopen(filename, mode) #define FTELLO_FUNC(stream) ftello(stream) @@ -94,7 +93,7 @@ uLong filetime(f, tmzip, dt) return ret; } #else -#ifdef unix || __APPLE__ +#if defined(unix) || defined(__APPLE__) || defined(__Fuchsia__) || defined(__ANDROID_API__) uLong filetime(f, tmzip, dt) char *f; /* name of file to get info on */ tm_zip *tmzip; /* return value: access, modific. and creation times */ diff --git a/contrib/minizip/minizip.md b/contrib/minizip/minizip.md new file mode 100644 index 0000000..9f15dd2 --- /dev/null +++ b/contrib/minizip/minizip.md @@ -0,0 +1,9 @@ +Minizip is a library provided by //third_party/zlib [1]. Its zip and unzip +tools can be built in a developer checkout for testing purposes with: + +```shell + autoninja -C out/Release minizip_bin + autoninja -C out/Release miniunz_bin +``` + +[1] Upstream is https://github.com/madler/zlib/tree/master/contrib/minizip diff --git a/contrib/tests/infcover.cc b/contrib/tests/infcover.cc index c5300a5..16dd744 100644 --- a/contrib/tests/infcover.cc +++ b/contrib/tests/infcover.cc @@ -395,7 +395,9 @@ void cover_support(void) mem_setup(&strm); strm.avail_in = 0; strm.next_in = Z_NULL; - ret = inflateInit_(&strm, ZLIB_VERSION - 1, (int)sizeof(z_stream)); + char versioncpy[] = ZLIB_VERSION; + versioncpy[0] -= 1; + ret = inflateInit_(&strm, versioncpy, (int)sizeof(z_stream)); assert(ret == Z_VERSION_ERROR); mem_done(&strm, "wrong version"); diff --git a/google/BUILD.gn b/google/BUILD.gn index c29e892..e996b16 100644 --- a/google/BUILD.gn +++ b/google/BUILD.gn @@ -7,6 +7,7 @@ import("//build_overrides/build.gni") if (build_with_chromium) { static_library("zip") { sources = [ + "redact.h", "zip.cc", "zip.h", "zip_internal.cc", @@ -18,6 +19,7 @@ if (build_with_chromium) { ] deps = [ "//base", + "//base:i18n", "//third_party/zlib:minizip", ] } diff --git a/google/OWNERS b/google/OWNERS index 868af3c..411670c 100644 --- a/google/OWNERS +++ b/google/OWNERS @@ -1,3 +1,5 @@ +fdegros@chromium.org +noel@chromium.org satorux@chromium.org # compression_utils* diff --git a/google/compression_utils_unittest.cc b/google/compression_utils_unittest.cc index 31c3226..76572e5 100644 --- a/google/compression_utils_unittest.cc +++ b/google/compression_utils_unittest.cc @@ -7,9 +7,9 @@ #include <stddef.h> #include <stdint.h> +#include <iterator> #include <string> -#include "base/stl_util.h" #include "testing/gtest/include/gtest/gtest.h" namespace compression { @@ -33,24 +33,24 @@ const uint8_t kCompressedData[] = { } // namespace TEST(CompressionUtilsTest, GzipCompression) { - std::string data(reinterpret_cast<const char*>(kData), base::size(kData)); + std::string data(reinterpret_cast<const char*>(kData), std::size(kData)); std::string compressed_data; EXPECT_TRUE(GzipCompress(data, &compressed_data)); std::string golden_compressed_data( reinterpret_cast<const char*>(kCompressedData), - base::size(kCompressedData)); + std::size(kCompressedData)); EXPECT_EQ(golden_compressed_data, compressed_data); } TEST(CompressionUtilsTest, GzipUncompression) { std::string compressed_data(reinterpret_cast<const char*>(kCompressedData), - base::size(kCompressedData)); + std::size(kCompressedData)); std::string uncompressed_data; EXPECT_TRUE(GzipUncompress(compressed_data, &uncompressed_data)); std::string golden_data(reinterpret_cast<const char*>(kData), - base::size(kData)); + std::size(kData)); EXPECT_EQ(golden_data, uncompressed_data); } @@ -59,7 +59,7 @@ TEST(CompressionUtilsTest, GzipUncompressionFromSpanToString) { EXPECT_TRUE(GzipUncompress(kCompressedData, &uncompressed_data)); std::string golden_data(reinterpret_cast<const char*>(kData), - base::size(kData)); + std::size(kData)); EXPECT_EQ(golden_data, uncompressed_data); } @@ -84,10 +84,10 @@ TEST(CompressionUtilsTest, LargeInput) { TEST(CompressionUtilsTest, InPlace) { const std::string original_data(reinterpret_cast<const char*>(kData), - base::size(kData)); + std::size(kData)); const std::string golden_compressed_data( reinterpret_cast<const char*>(kCompressedData), - base::size(kCompressedData)); + std::size(kCompressedData)); std::string data(original_data); EXPECT_TRUE(GzipCompress(data, &data)); diff --git a/google/redact.h b/google/redact.h new file mode 100644 index 0000000..ea7da16 --- /dev/null +++ b/google/redact.h @@ -0,0 +1,31 @@ +// Copyright (c) 2022 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#ifndef THIRD_PARTY_ZLIB_GOOGLE_REDACT_H_ +#define THIRD_PARTY_ZLIB_GOOGLE_REDACT_H_ + +#include <ostream> + +#include "base/files/file_path.h" +#include "base/logging.h" + +namespace zip { + +// Redacts file paths in log messages. +// Example: +// LOG(ERROR) << "Cannot open " << Redact(path); +class Redact { + public: + explicit Redact(const base::FilePath& path) : path_(path) {} + + friend std::ostream& operator<<(std::ostream& out, const Redact&& r) { + return LOG_IS_ON(INFO) ? out << "'" << r.path_ << "'" : out << "(redacted)"; + } + + private: + const base::FilePath& path_; +}; + +} // namespace zip + +#endif // THIRD_PARTY_ZLIB_GOOGLE_REDACT_H_ diff --git a/google/test/data/Different Encryptions.zip b/google/test/data/Different Encryptions.zip Binary files differnew file mode 100644 index 0000000..858e7c7 --- /dev/null +++ b/google/test/data/Different Encryptions.zip diff --git a/google/test/data/Empty Dir Same Name As File.zip b/google/test/data/Empty Dir Same Name As File.zip Binary files differnew file mode 100644 index 0000000..e26c42d --- /dev/null +++ b/google/test/data/Empty Dir Same Name As File.zip diff --git a/google/test/data/Mixed Paths.zip b/google/test/data/Mixed Paths.zip Binary files differnew file mode 100644 index 0000000..2af418b --- /dev/null +++ b/google/test/data/Mixed Paths.zip diff --git a/google/test/data/Parent Dir Same Name As File.zip b/google/test/data/Parent Dir Same Name As File.zip Binary files differnew file mode 100644 index 0000000..76cfd90 --- /dev/null +++ b/google/test/data/Parent Dir Same Name As File.zip diff --git a/google/test/data/README.md b/google/test/data/README.md new file mode 100644 index 0000000..c76648f --- /dev/null +++ b/google/test/data/README.md @@ -0,0 +1,15 @@ +## test\_posix\_permissions.zip +Rebuild this zip by running: +``` +rm test_posix_permissions.zip && +mkdir z && +cd z && +touch 0.txt 1.txt 2.txt 3.txt && +chmod a+x 0.txt && +chmod o+x 1.txt && +chmod u+x 2.txt && +zip test_posix_permissions.zip * && +mv test_posix_permissions.zip .. && +cd .. && +rm -r z +``` diff --git a/google/test/data/Repeated Dir Name.zip b/google/test/data/Repeated Dir Name.zip Binary files differnew file mode 100644 index 0000000..2375bc7 --- /dev/null +++ b/google/test/data/Repeated Dir Name.zip diff --git a/google/test/data/Repeated File Name With Different Cases.zip b/google/test/data/Repeated File Name With Different Cases.zip Binary files differnew file mode 100644 index 0000000..9eb44e1 --- /dev/null +++ b/google/test/data/Repeated File Name With Different Cases.zip diff --git a/google/test/data/Repeated File Name.zip b/google/test/data/Repeated File Name.zip Binary files differnew file mode 100644 index 0000000..4a3e7b1 --- /dev/null +++ b/google/test/data/Repeated File Name.zip diff --git a/google/test/data/SJIS Bug 846195.zip b/google/test/data/SJIS Bug 846195.zip Binary files differnew file mode 100644 index 0000000..0783002 --- /dev/null +++ b/google/test/data/SJIS Bug 846195.zip diff --git a/google/test/data/Windows Special Names.zip b/google/test/data/Windows Special Names.zip Binary files differnew file mode 100644 index 0000000..3b8a3ab --- /dev/null +++ b/google/test/data/Windows Special Names.zip diff --git a/google/test/data/Wrong CRC.zip b/google/test/data/Wrong CRC.zip Binary files differnew file mode 100644 index 0000000..ee9a1ef --- /dev/null +++ b/google/test/data/Wrong CRC.zip diff --git a/google/test/data/empty.zip b/google/test/data/empty.zip Binary files differnew file mode 100644 index 0000000..15cb0ec --- /dev/null +++ b/google/test/data/empty.zip diff --git a/google/test/data/test_posix_permissions.zip b/google/test/data/test_posix_permissions.zip Binary files differnew file mode 100644 index 0000000..a058ba1 --- /dev/null +++ b/google/test/data/test_posix_permissions.zip diff --git a/google/zip.cc b/google/zip.cc index 907e5da..1a43196 100644 --- a/google/zip.cc +++ b/google/zip.cc @@ -4,17 +4,18 @@ #include "third_party/zlib/google/zip.h" -#include <list> #include <string> #include <vector> #include "base/bind.h" #include "base/files/file.h" #include "base/files/file_enumerator.h" +#include "base/files/file_util.h" #include "base/logging.h" #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" @@ -26,18 +27,13 @@ bool IsHiddenFile(const base::FilePath& file_path) { return file_path.BaseName().value()[0] == '.'; } -bool ExcludeNoFilesFilter(const base::FilePath& file_path) { - return true; -} - -bool ExcludeHiddenFilesFilter(const base::FilePath& file_path) { - return !IsHiddenFile(file_path); -} - // Creates a directory at |extract_dir|/|entry_path|, including any parents. bool CreateDirectory(const base::FilePath& extract_dir, const base::FilePath& entry_path) { - return base::CreateDirectory(extract_dir.Append(entry_path)); + const base::FilePath dir = extract_dir.Append(entry_path); + const bool ok = base::CreateDirectory(dir); + PLOG_IF(ERROR, !ok) << "Cannot create directory " << Redact(dir); + return ok; } // Creates a WriterDelegate that can write a file at |extract_dir|/|entry_path|. @@ -50,222 +46,226 @@ std::unique_ptr<WriterDelegate> CreateFilePathWriterDelegate( class DirectFileAccessor : public FileAccessor { public: - explicit DirectFileAccessor(base::FilePath src_dir) : src_dir_(src_dir) {} + explicit DirectFileAccessor(base::FilePath src_dir) + : src_dir_(std::move(src_dir)) {} + ~DirectFileAccessor() override = default; - std::vector<base::File> OpenFilesForReading( - const std::vector<base::FilePath>& paths) override { - std::vector<base::File> files; - for (const auto& path : paths) { - base::File file; - if (base::PathExists(path) && !base::DirectoryExists(path)) { - file = base::File(path, base::File::FLAG_OPEN | base::File::FLAG_READ); + bool Open(const Paths paths, std::vector<base::File>* const files) override { + DCHECK(files); + files->reserve(files->size() + paths.size()); + + for (const base::FilePath& path : paths) { + DCHECK(!path.IsAbsolute()); + const base::FilePath absolute_path = src_dir_.Append(path); + if (base::DirectoryExists(absolute_path)) { + files->emplace_back(); + LOG(ERROR) << "Cannot open " << Redact(path) << ": It is a directory"; + } else { + 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()); } - files.push_back(std::move(file)); } - return files; - } - bool DirectoryExists(const base::FilePath& file) override { - return base::DirectoryExists(file); + return true; } - std::vector<DirectoryContentEntry> ListDirectoryContent( - const base::FilePath& dir) override { - std::vector<DirectoryContentEntry> files; + bool List(const base::FilePath& path, + std::vector<base::FilePath>* const files, + std::vector<base::FilePath>* const subdirs) override { + DCHECK(!path.IsAbsolute()); + DCHECK(files); + DCHECK(subdirs); + base::FileEnumerator file_enumerator( - dir, false /* recursive */, + src_dir_.Append(path), false /* recursive */, base::FileEnumerator::FILES | base::FileEnumerator::DIRECTORIES); - for (base::FilePath path = file_enumerator.Next(); !path.value().empty(); - path = file_enumerator.Next()) { - files.push_back(DirectoryContentEntry(path, base::DirectoryExists(path))); + + while (!file_enumerator.Next().empty()) { + const base::FileEnumerator::FileInfo info = file_enumerator.GetInfo(); + (info.IsDirectory() ? subdirs : files) + ->push_back(path.Append(info.GetName())); } - return files; + + return true; } - base::Time GetLastModifiedTime(const base::FilePath& path) override { + bool GetInfo(const base::FilePath& path, Info* const info) override { + DCHECK(!path.IsAbsolute()); + DCHECK(info); + base::File::Info file_info; - if (!base::GetFileInfo(path, &file_info)) { - LOG(ERROR) << "Failed to retrieve file modification time for " - << path.value(); + if (!base::GetFileInfo(src_dir_.Append(path), &file_info)) { + PLOG(ERROR) << "Cannot get info of " << Redact(path); + return false; } - return file_info.last_modified; + + info->is_directory = file_info.is_directory; + info->last_modified = file_info.last_modified; + + return true; } private: - base::FilePath src_dir_; - - DISALLOW_COPY_AND_ASSIGN(DirectFileAccessor); + const base::FilePath src_dir_; }; } // namespace -ZipParams::ZipParams(const base::FilePath& src_dir, - const base::FilePath& dest_file) - : src_dir_(src_dir), - dest_file_(dest_file), - file_accessor_(new DirectFileAccessor(src_dir)) {} - -#if defined(OS_POSIX) -// Does not take ownership of |fd|. -ZipParams::ZipParams(const base::FilePath& src_dir, int dest_fd) - : src_dir_(src_dir), - dest_fd_(dest_fd), - file_accessor_(new DirectFileAccessor(src_dir)) {} -#endif +std::ostream& operator<<(std::ostream& out, const Progress& progress) { + return out << progress.bytes << " bytes, " << progress.files << " files, " + << progress.directories << " dirs, " << progress.errors + << " errors"; +} bool Zip(const ZipParams& params) { - // Using a pointer to avoid copies of a potentially large array. - const std::vector<base::FilePath>* files_to_add = ¶ms.files_to_zip(); - std::vector<base::FilePath> all_files; - if (files_to_add->empty()) { - // Include all files from the src_dir (modulo the src_dir itself and - // filtered and hidden files). - - files_to_add = &all_files; - // Using a list so we can call push_back while iterating. - std::list<FileAccessor::DirectoryContentEntry> entries; - entries.push_back(FileAccessor::DirectoryContentEntry( - params.src_dir(), true /* is directory*/)); - const FilterCallback& filter_callback = params.filter_callback(); - for (auto iter = entries.begin(); iter != entries.end(); ++iter) { - const base::FilePath& entry_path = iter->path; - if (iter != entries.begin() && // Don't filter the root dir. - ((!params.include_hidden_files() && IsHiddenFile(entry_path)) || - (filter_callback && !filter_callback.Run(entry_path)))) { - continue; - } - - if (iter != entries.begin()) { // Exclude the root dir from the ZIP file. - // Make the path relative for AddEntryToZip. - base::FilePath relative_path; - bool success = - params.src_dir().AppendRelativePath(entry_path, &relative_path); - DCHECK(success); - all_files.push_back(relative_path); - } - - if (iter->is_directory) { - std::vector<FileAccessor::DirectoryContentEntry> subentries = - params.file_accessor()->ListDirectoryContent(entry_path); - entries.insert(entries.end(), subentries.begin(), subentries.end()); - } - } - } + DirectFileAccessor default_accessor(params.src_dir); + FileAccessor* const file_accessor = params.file_accessor ?: &default_accessor; std::unique_ptr<internal::ZipWriter> zip_writer; -#if defined(OS_POSIX) - if (params.dest_fd() != base::kInvalidPlatformFile) { - DCHECK(params.dest_file().empty()); - zip_writer = internal::ZipWriter::CreateWithFd( - params.dest_fd(), params.src_dir(), params.file_accessor()); + +#if defined(OS_POSIX) || defined(OS_FUCHSIA) + if (params.dest_fd != base::kInvalidPlatformFile) { + DCHECK(params.dest_file.empty()); + zip_writer = + internal::ZipWriter::CreateWithFd(params.dest_fd, file_accessor); if (!zip_writer) return false; } #endif + if (!zip_writer) { - zip_writer = internal::ZipWriter::Create( - params.dest_file(), params.src_dir(), params.file_accessor()); + zip_writer = internal::ZipWriter::Create(params.dest_file, file_accessor); if (!zip_writer) return false; } - return zip_writer->WriteEntries(*files_to_add); -} -bool Unzip(const base::FilePath& src_file, const base::FilePath& dest_dir) { - return UnzipWithFilterCallback( - src_file, dest_dir, base::BindRepeating(&ExcludeNoFilesFilter), true); + zip_writer->SetProgressCallback(params.progress_callback, + params.progress_period); + zip_writer->SetRecursive(params.recursive); + zip_writer->ContinueOnError(params.continue_on_error); + + if (!params.include_hidden_files || params.filter_callback) + zip_writer->SetFilterCallback(base::BindRepeating( + [](const ZipParams* const params, const base::FilePath& path) -> bool { + return (params->include_hidden_files || !IsHiddenFile(path)) && + (!params->filter_callback || + params->filter_callback.Run(params->src_dir.Append(path))); + }, + ¶ms)); + + if (params.src_files.empty()) { + // No source items are specified. Zip the entire source directory. + zip_writer->SetRecursive(true); + if (!zip_writer->AddDirectoryContents(base::FilePath())) + return false; + } else { + // Only zip the specified source items. + if (!zip_writer->AddMixedEntries(params.src_files)) + return false; + } + + return zip_writer->Close(); } -bool UnzipWithFilterCallback(const base::FilePath& src_file, - const base::FilePath& dest_dir, - const FilterCallback& filter_cb, - bool log_skipped_files) { +bool Unzip(const base::FilePath& src_file, + const base::FilePath& dest_dir, + UnzipOptions options) { base::File file(src_file, base::File::FLAG_OPEN | base::File::FLAG_READ); if (!file.IsValid()) { - DLOG(WARNING) << "Failed to open " << src_file.value(); + PLOG(ERROR) << "Cannot open " << Redact(src_file) << ": " + << base::File::ErrorToString(file.error_details()); return false; } - return UnzipWithFilterAndWriters( - file.GetPlatformFile(), - base::BindRepeating(&CreateFilePathWriterDelegate, dest_dir), - base::BindRepeating(&CreateDirectory, dest_dir), filter_cb, - log_skipped_files); + + DLOG_IF(WARNING, !base::IsDirectoryEmpty(dest_dir)) + << "ZIP extraction directory is not empty: " << dest_dir; + + return Unzip(file.GetPlatformFile(), + base::BindRepeating(&CreateFilePathWriterDelegate, dest_dir), + base::BindRepeating(&CreateDirectory, dest_dir), + std::move(options)); } -bool UnzipWithFilterAndWriters(const base::PlatformFile& src_file, - const WriterFactory& writer_factory, - const DirectoryCreator& directory_creator, - const FilterCallback& filter_cb, - bool log_skipped_files) { +bool Unzip(const base::PlatformFile& src_file, + WriterFactory writer_factory, + DirectoryCreator directory_creator, + UnzipOptions options) { ZipReader reader; + reader.SetEncoding(std::move(options.encoding)); + reader.SetPassword(std::move(options.password)); + if (!reader.OpenFromPlatformFile(src_file)) { - DLOG(WARNING) << "Failed to open src_file " << src_file; + LOG(ERROR) << "Cannot open ZIP from file handle " << src_file; return false; } - while (reader.HasMore()) { - if (!reader.OpenCurrentEntryInZip()) { - DLOG(WARNING) << "Failed to open the current file in zip"; - return false; + + while (const ZipReader::Entry* const entry = reader.Next()) { + if (entry->is_unsafe) { + LOG(ERROR) << "Found unsafe entry " << Redact(entry->path) << " in ZIP"; + if (!options.continue_on_error) + return false; + continue; } - const base::FilePath& entry_path = reader.current_entry_info()->file_path(); - if (reader.current_entry_info()->is_unsafe()) { - DLOG(WARNING) << "Found an unsafe file in zip " << entry_path; - return false; + + if (options.filter && !options.filter.Run(entry->path)) { + VLOG(1) << "Skipped ZIP entry " << Redact(entry->path); + continue; } - if (filter_cb.Run(entry_path)) { - if (reader.current_entry_info()->is_directory()) { - if (!directory_creator.Run(entry_path)) - return false; - } else { - std::unique_ptr<WriterDelegate> writer = writer_factory.Run(entry_path); - if (!reader.ExtractCurrentEntry(writer.get(), - std::numeric_limits<uint64_t>::max())) { - DLOG(WARNING) << "Failed to extract " << entry_path; + + if (entry->is_directory) { + // It's a directory. + if (!directory_creator.Run(entry->path)) { + LOG(ERROR) << "Cannot create directory " << Redact(entry->path); + if (!options.continue_on_error) return false; - } } - } else if (log_skipped_files) { - DLOG(WARNING) << "Skipped file " << entry_path; + + continue; } - if (!reader.AdvanceToNextEntry()) { - DLOG(WARNING) << "Failed to advance to the next file"; - return false; + // It's a file. + std::unique_ptr<WriterDelegate> writer = writer_factory.Run(entry->path); + if (!writer || !reader.ExtractCurrentEntry(writer.get())) { + LOG(ERROR) << "Cannot extract file " << Redact(entry->path) + << " from ZIP"; + if (!options.continue_on_error) + return false; } } - return true; + + return reader.ok(); } bool ZipWithFilterCallback(const base::FilePath& src_dir, const base::FilePath& dest_file, - const FilterCallback& filter_cb) { + FilterCallback filter) { DCHECK(base::DirectoryExists(src_dir)); - ZipParams params(src_dir, dest_file); - params.set_filter_callback(filter_cb); - return Zip(params); + return Zip({.src_dir = src_dir, + .dest_file = dest_file, + .filter_callback = std::move(filter)}); } -bool Zip(const base::FilePath& src_dir, const base::FilePath& dest_file, +bool Zip(const base::FilePath& src_dir, + const base::FilePath& dest_file, bool include_hidden_files) { - if (include_hidden_files) { - return ZipWithFilterCallback(src_dir, dest_file, - base::BindRepeating(&ExcludeNoFilesFilter)); - } else { - return ZipWithFilterCallback( - src_dir, dest_file, base::BindRepeating(&ExcludeHiddenFilesFilter)); - } + return Zip({.src_dir = src_dir, + .dest_file = dest_file, + .include_hidden_files = include_hidden_files}); } -#if defined(OS_POSIX) +#if defined(OS_POSIX) || defined(OS_FUCHSIA) bool ZipFiles(const base::FilePath& src_dir, - const std::vector<base::FilePath>& src_relative_paths, + Paths src_relative_paths, int dest_fd) { DCHECK(base::DirectoryExists(src_dir)); - ZipParams params(src_dir, dest_fd); - params.set_files_to_zip(src_relative_paths); - return Zip(params); + return Zip({.src_dir = src_dir, + .dest_fd = dest_fd, + .src_files = src_relative_paths}); } -#endif // defined(OS_POSIX) +#endif // defined(OS_POSIX) || defined(OS_FUCHSIA) } // namespace zip diff --git a/google/zip.h b/google/zip.h index 4f64a8a..25ec655 100644 --- a/google/zip.h +++ b/google/zip.h @@ -5,9 +5,13 @@ #ifndef THIRD_PARTY_ZLIB_GOOGLE_ZIP_H_ #define THIRD_PARTY_ZLIB_GOOGLE_ZIP_H_ +#include <cstdint> +#include <ostream> +#include <utility> #include <vector> #include "base/callback.h" +#include "base/containers/span.h" #include "base/files/file_path.h" #include "base/files/platform_file.h" #include "base/time/time.h" @@ -21,99 +25,118 @@ namespace zip { class WriterDelegate; +// Paths passed as span to avoid copying them. +using Paths = base::span<const base::FilePath>; + // Abstraction for file access operation required by Zip(). +// // Can be passed to the ZipParams for providing custom access to the files, // for example over IPC. -// If none is provided, the files are accessed directly. -// All parameters paths are expected to be absolute. +// +// All parameters paths are expected to be relative to the source directory. class FileAccessor { public: virtual ~FileAccessor() = default; - struct DirectoryContentEntry { - DirectoryContentEntry(const base::FilePath& path, bool is_directory) - : path(path), is_directory(is_directory) {} - base::FilePath path; + struct Info { bool is_directory = false; + base::Time last_modified; }; // Opens files specified in |paths|. // Directories should be mapped to invalid files. - virtual std::vector<base::File> OpenFilesForReading( - const std::vector<base::FilePath>& paths) = 0; + virtual bool Open(Paths paths, std::vector<base::File>* files) = 0; + + // Lists contents of a directory at |path|. + virtual bool List(const base::FilePath& path, + std::vector<base::FilePath>* files, + std::vector<base::FilePath>* subdirs) = 0; - virtual bool DirectoryExists(const base::FilePath& path) = 0; - virtual std::vector<DirectoryContentEntry> ListDirectoryContent( - const base::FilePath& dir_path) = 0; - virtual base::Time GetLastModifiedTime(const base::FilePath& path) = 0; + // Gets info about a file or directory. + virtual bool GetInfo(const base::FilePath& path, Info* info) = 0; }; -class ZipParams { - public: - ZipParams(const base::FilePath& src_dir, const base::FilePath& dest_file); -#if defined(OS_POSIX) - // Does not take ownership of |dest_fd|. - ZipParams(const base::FilePath& src_dir, int dest_fd); +// Progress of a ZIP creation operation. +struct Progress { + // Total number of bytes read from files getting zipped so far. + std::int64_t bytes = 0; - int dest_fd() const { return dest_fd_; } -#endif + // Number of file entries added to the ZIP so far. + // A file entry is added after its bytes have been processed. + int files = 0; - const base::FilePath& src_dir() const { return src_dir_; } - - const base::FilePath& dest_file() const { return dest_file_; } - - // Restricts the files actually zipped to the paths listed in - // |src_relative_paths|. They must be relative to the |src_dir| passed in the - // constructor and will be used as the file names in the created zip file. All - // source paths must be under |src_dir| in the file system hierarchy. - void set_files_to_zip(const std::vector<base::FilePath>& src_relative_paths) { - src_files_ = src_relative_paths; - } - const std::vector<base::FilePath>& files_to_zip() const { return src_files_; } - - using FilterCallback = base::RepeatingCallback<bool(const base::FilePath&)>; - void set_filter_callback(FilterCallback filter_callback) { - filter_callback_ = filter_callback; - } - const FilterCallback& filter_callback() const { return filter_callback_; } - - void set_include_hidden_files(bool include_hidden_files) { - include_hidden_files_ = include_hidden_files; - } - bool include_hidden_files() const { return include_hidden_files_; } - - // Sets a custom file accessor for file operations. Default is to directly - // access the files (with fopen and the rest). - // Useful in cases where running in a sandbox process and file access has to - // go through IPC, for example. - void set_file_accessor(std::unique_ptr<FileAccessor> file_accessor) { - file_accessor_ = std::move(file_accessor); - } - FileAccessor* file_accessor() const { return file_accessor_.get(); } - - private: - base::FilePath src_dir_; - - base::FilePath dest_file_; -#if defined(OS_POSIX) - int dest_fd_ = base::kInvalidPlatformFile; -#endif + // Number of directory entries added to the ZIP so far. + // A directory entry is added before items in it. + int directories = 0; + + // Number of errors encountered so far (files that cannot be opened, + // directories that cannot be listed). + int errors = 0; +}; + +// Prints Progress to output stream. +std::ostream& operator<<(std::ostream& out, const Progress& progress); + +// Callback reporting the progress of a ZIP creation operation. +// +// This callback returns a boolean indicating whether the ZIP creation operation +// should continue. If it returns false once, then the ZIP creation operation is +// immediately cancelled and the callback won't be called again. +using ProgressCallback = base::RepeatingCallback<bool(const Progress&)>; + +using FilterCallback = base::RepeatingCallback<bool(const base::FilePath&)>; - // The relative paths to the files that should be included in the zip file. If - // this is empty, all files in |src_dir_| are included. - std::vector<base::FilePath> src_files_; +// ZIP creation parameters and options. +struct ZipParams { + // Source directory. Ignored if |file_accessor| is set. + base::FilePath src_dir; - // Filter used to exclude files from the ZIP file. Only effective when - // |src_files_| is empty. - FilterCallback filter_callback_; + // Abstraction around file system access used to read files. + // If left null, an implementation that accesses files directly is used. + FileAccessor* file_accessor = nullptr; // Not owned - // Whether hidden files should be included in the ZIP file. Only effective - // when |src_files_| is empty. - bool include_hidden_files_ = true; + // Destination file path. + // Either dest_file or dest_fd should be set, but not both. + base::FilePath dest_file; - // Abstraction around file system access used to read files. An implementation - // that accesses files directly is provided by default. - std::unique_ptr<FileAccessor> file_accessor_; +#if defined(OS_POSIX) || defined(OS_FUCHSIA) + // Destination file passed a file descriptor. + // Either dest_file or dest_fd should be set, but not both. + int dest_fd = base::kInvalidPlatformFile; +#endif + + // The relative paths to the files and directories that should be included in + // the ZIP file. If this is empty, the whole contents of |src_dir| are + // included. + // + // These paths must be relative to |src_dir| and will be used as the file + // names in the created ZIP file. All files must be under |src_dir| in the + // file system hierarchy. + // + // All the paths in |src_files| are included in the created ZIP file, + // irrespective of |include_hidden_files| and |filter_callback|. + Paths src_files; + + // Filter used to exclude files from the ZIP file. This is only taken in + // account when recursively adding subdirectory contents. + FilterCallback filter_callback; + + // Optional progress reporting callback. + ProgressCallback progress_callback; + + // Progress reporting period. The final callback is always called when the ZIP + // creation operation completes. + base::TimeDelta progress_period; + + // Should add hidden files? This is only taken in account when recursively + // adding subdirectory contents. + bool include_hidden_files = true; + + // Should recursively add subdirectory contents? + bool recursive = false; + + // Should ignore errors when discovering files and zipping them? + bool continue_on_error = false; }; // Zip files specified into a ZIP archives. The source files and ZIP destination @@ -125,56 +148,65 @@ bool Zip(const ZipParams& params); // of src_dir will be at the root level of the created zip. For each file in // src_dir, include it only if the callback |filter_cb| returns true. Otherwise // omit it. -using FilterCallback = base::RepeatingCallback<bool(const base::FilePath&)>; bool ZipWithFilterCallback(const base::FilePath& src_dir, const base::FilePath& dest_file, - const FilterCallback& filter_cb); + FilterCallback filter_cb); // Convenience method for callers who don't need to set up the filter callback. // If |include_hidden_files| is true, files starting with "." are included. // Otherwise they are omitted. -bool Zip(const base::FilePath& src_dir, const base::FilePath& dest_file, +bool Zip(const base::FilePath& src_dir, + const base::FilePath& dest_file, bool include_hidden_files); -#if defined(OS_POSIX) +#if defined(OS_POSIX) || defined(OS_FUCHSIA) // Zips files listed in |src_relative_paths| to destination specified by file // descriptor |dest_fd|, without taking ownership of |dest_fd|. The paths listed // in |src_relative_paths| are relative to the |src_dir| and will be used as the // file names in the created zip file. All source paths must be under |src_dir| // in the file system hierarchy. bool ZipFiles(const base::FilePath& src_dir, - const std::vector<base::FilePath>& src_relative_paths, + Paths src_relative_paths, int dest_fd); -#endif // defined(OS_POSIX) +#endif // defined(OS_POSIX) || defined(OS_FUCHSIA) + +// Options of the Unzip function, with valid default values. +struct UnzipOptions { + // Encoding of entry paths in the ZIP archive. By default, paths are assumed + // to be in UTF-8. + std::string encoding; + + // Only extract the entries for which |filter_cb| returns true. By default, + // everything gets extracted. + FilterCallback filter; + + // Password to decrypt the encrypted files. + std::string password; + + // Should ignore errors when extracting files? + bool continue_on_error = false; +}; -// Unzip the contents of zip_file into dest_dir. -// For each file in zip_file, include it only if the callback |filter_cb| -// returns true. Otherwise omit it. -// If |log_skipped_files| is true, files skipped during extraction are printed -// to debug log. -using FilterCallback = base::RepeatingCallback<bool(const base::FilePath&)>; -bool UnzipWithFilterCallback(const base::FilePath& zip_file, - const base::FilePath& dest_dir, - const FilterCallback& filter_cb, - bool log_skipped_files); - -// 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. -// If |log_skipped_files| is true, files skipped during extraction are printed -// to debug log. typedef base::RepeatingCallback<std::unique_ptr<WriterDelegate>( const base::FilePath&)> WriterFactory; + typedef base::RepeatingCallback<bool(const base::FilePath&)> DirectoryCreator; -bool UnzipWithFilterAndWriters(const base::PlatformFile& zip_file, - const WriterFactory& writer_factory, - const DirectoryCreator& directory_creator, - const FilterCallback& filter_cb, - bool log_skipped_files); - -// Unzip the contents of zip_file into dest_dir. -bool Unzip(const base::FilePath& zip_file, const base::FilePath& dest_dir); + +// Unzips the contents of |zip_file|, using the writers provided by +// |writer_factory|. +bool Unzip(const base::PlatformFile& zip_file, + WriterFactory writer_factory, + DirectoryCreator directory_creator, + UnzipOptions options = {}); + +// Unzips the contents of |zip_file| into |dest_dir|. +// This function does not overwrite any existing file. +// A filename collision will result in an error. +// Therefore, |dest_dir| should initially be an empty directory. +bool Unzip(const base::FilePath& zip_file, + const base::FilePath& dest_dir, + UnzipOptions options = {}); } // namespace zip diff --git a/google/zip_internal.cc b/google/zip_internal.cc index 354fbf8..1adf2e6 100644 --- a/google/zip_internal.cc +++ b/google/zip_internal.cc @@ -8,9 +8,13 @@ #include <string.h> #include <algorithm> +#include <unordered_set> +#include "base/files/file_path.h" #include "base/logging.h" +#include "base/no_destructor.h" #include "base/notreached.h" +#include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #if defined(USE_SYSTEM_MINIZIP) @@ -36,9 +40,9 @@ typedef struct { } WIN32FILE_IOWIN; // This function is derived from third_party/minizip/iowin32.c. -// Its only difference is that it treats the char* as UTF8 and +// Its only difference is that it treats the filename as UTF-8 and // uses the Unicode version of CreateFile. -void* ZipOpenFunc(void *opaque, const char* filename, int mode) { +void* ZipOpenFunc(void* opaque, const void* filename, int mode) { DWORD desired_access = 0, creation_disposition = 0; DWORD share_mode = 0, flags_and_attributes = 0; HANDLE file = 0; @@ -56,10 +60,11 @@ void* ZipOpenFunc(void *opaque, const char* filename, int mode) { creation_disposition = CREATE_ALWAYS; } - std::wstring filenamew = base::UTF8ToWide(filename); - if ((filename != NULL) && (desired_access != 0)) { - file = CreateFile(filenamew.c_str(), desired_access, share_mode, NULL, - creation_disposition, flags_and_attributes, NULL); + if (filename != nullptr && desired_access != 0) { + file = CreateFileW( + base::UTF8ToWide(static_cast<const char*>(filename)).c_str(), + desired_access, share_mode, nullptr, creation_disposition, + flags_and_attributes, nullptr); } if (file == INVALID_HANDLE_VALUE) @@ -79,11 +84,11 @@ void* ZipOpenFunc(void *opaque, const char* filename, int mode) { } #endif -#if defined(OS_POSIX) +#if defined(OS_POSIX) || defined(OS_FUCHSIA) // Callback function for zlib that opens a file stream from a file descriptor. // Since we do not own the file descriptor, dup it so that we can fdopen/fclose // a file stream. -void* FdOpenFileFunc(void* opaque, const char* filename, int mode) { +void* FdOpenFileFunc(void* opaque, const void* filename, int mode) { FILE* file = NULL; const char* mode_fopen = NULL; @@ -105,15 +110,15 @@ void* FdOpenFileFunc(void* opaque, const char* filename, int mode) { int FdCloseFileFunc(void* opaque, void* stream) { fclose(static_cast<FILE*>(stream)); - free(opaque); // malloc'ed in FillFdOpenFileFunc() + free(opaque); // malloc'ed in FillFdOpenFileFunc() return 0; } // Fills |pzlib_filecunc_def| appropriately to handle the zip file // referred to by |fd|. -void FillFdOpenFileFunc(zlib_filefunc_def* pzlib_filefunc_def, int fd) { - fill_fopen_filefunc(pzlib_filefunc_def); - pzlib_filefunc_def->zopen_file = FdOpenFileFunc; +void FillFdOpenFileFunc(zlib_filefunc64_def* pzlib_filefunc_def, int fd) { + fill_fopen64_filefunc(pzlib_filefunc_def); + pzlib_filefunc_def->zopen64_file = FdOpenFileFunc; pzlib_filefunc_def->zclose_file = FdCloseFileFunc; int* ptr_fd = static_cast<int*>(malloc(sizeof(fd))); *ptr_fd = fd; @@ -124,7 +129,7 @@ void FillFdOpenFileFunc(zlib_filefunc_def* pzlib_filefunc_def, int fd) { #if defined(OS_WIN) // Callback function for zlib that opens a file stream from a Windows handle. // Does not take ownership of the handle. -void* HandleOpenFileFunc(void* opaque, const char* filename, int mode) { +void* HandleOpenFileFunc(void* opaque, const void* /*filename*/, int mode) { WIN32FILE_IOWIN file_ret; file_ret.hf = static_cast<HANDLE>(opaque); file_ret.error = 0; @@ -138,7 +143,7 @@ void* HandleOpenFileFunc(void* opaque, const char* filename, int mode) { } int HandleCloseFileFunc(void* opaque, void* stream) { - free(stream); // malloc'ed in HandleOpenFileFunc() + free(stream); // malloc'ed in HandleOpenFileFunc() return 0; } #endif @@ -148,8 +153,8 @@ int HandleCloseFileFunc(void* opaque, void* stream) { // expect their opaque parameters refer to this struct. struct ZipBuffer { const char* data; // weak - size_t length; - size_t offset; + ZPOS64_T length; + ZPOS64_T offset; }; // Opens the specified file. When this function returns a non-NULL pointer, zlib @@ -158,7 +163,7 @@ struct ZipBuffer { // given opaque parameter and returns it because this parameter stores all // information needed for uncompressing data. (This function does not support // writing compressed data and it returns NULL for this case.) -void* OpenZipBuffer(void* opaque, const char* /*filename*/, int mode) { +void* OpenZipBuffer(void* opaque, const void* /*filename*/, int mode) { if ((mode & ZLIB_FILEFUNC_MODE_READWRITEFILTER) != ZLIB_FILEFUNC_MODE_READ) { NOTREACHED(); return NULL; @@ -175,10 +180,11 @@ void* OpenZipBuffer(void* opaque, const char* /*filename*/, int mode) { uLong ReadZipBuffer(void* opaque, void* /*stream*/, void* buf, uLong size) { ZipBuffer* buffer = static_cast<ZipBuffer*>(opaque); DCHECK_LE(buffer->offset, buffer->length); - size_t remaining_bytes = buffer->length - buffer->offset; + ZPOS64_T remaining_bytes = buffer->length - buffer->offset; if (!buffer || !buffer->data || !remaining_bytes) return 0; - size = std::min(size, static_cast<uLong>(remaining_bytes)); + if (size > remaining_bytes) + size = remaining_bytes; memcpy(buf, &buffer->data[buffer->offset], size); buffer->offset += size; return size; @@ -195,21 +201,23 @@ uLong WriteZipBuffer(void* /*opaque*/, } // Returns the offset from the beginning of the data. -long GetOffsetOfZipBuffer(void* opaque, void* /*stream*/) { +ZPOS64_T GetOffsetOfZipBuffer(void* opaque, void* /*stream*/) { ZipBuffer* buffer = static_cast<ZipBuffer*>(opaque); if (!buffer) return -1; - return static_cast<long>(buffer->offset); + return buffer->offset; } // Moves the current offset to the specified position. -long SeekZipBuffer(void* opaque, void* /*stream*/, uLong offset, int origin) { +long SeekZipBuffer(void* opaque, + void* /*stream*/, + ZPOS64_T offset, + int origin) { ZipBuffer* buffer = static_cast<ZipBuffer*>(opaque); if (!buffer) return -1; if (origin == ZLIB_FILEFUNC_SEEK_CUR) { - buffer->offset = std::min(buffer->offset + static_cast<size_t>(offset), - buffer->length); + buffer->offset = std::min(buffer->offset + offset, buffer->length); return 0; } if (origin == ZLIB_FILEFUNC_SEEK_END) { @@ -217,7 +225,7 @@ long SeekZipBuffer(void* opaque, void* /*stream*/, uLong offset, int origin) { return 0; } if (origin == ZLIB_FILEFUNC_SEEK_SET) { - buffer->offset = std::min(buffer->length, static_cast<size_t>(offset)); + buffer->offset = std::min(buffer->length, offset); return 0; } NOTREACHED(); @@ -243,7 +251,7 @@ int GetErrorOfZipBuffer(void* /*opaque*/, void* /*stream*/) { // Returns a zip_fileinfo struct with the time represented by |file_time|. zip_fileinfo TimeToZipFileInfo(const base::Time& file_time) { base::Time::Exploded file_time_parts; - file_time.LocalExplode(&file_time_parts); + file_time.UTCExplode(&file_time_parts); zip_fileinfo zip_info = {}; if (file_time_parts.year >= 1980) { @@ -268,33 +276,33 @@ namespace zip { namespace internal { unzFile OpenForUnzipping(const std::string& file_name_utf8) { - zlib_filefunc_def* zip_func_ptrs = NULL; + zlib_filefunc64_def* zip_func_ptrs = nullptr; #if defined(OS_WIN) - zlib_filefunc_def zip_funcs; - fill_win32_filefunc(&zip_funcs); - zip_funcs.zopen_file = ZipOpenFunc; + zlib_filefunc64_def zip_funcs; + fill_win32_filefunc64(&zip_funcs); + zip_funcs.zopen64_file = ZipOpenFunc; zip_func_ptrs = &zip_funcs; #endif - return unzOpen2(file_name_utf8.c_str(), zip_func_ptrs); + return unzOpen2_64(file_name_utf8.c_str(), zip_func_ptrs); } -#if defined(OS_POSIX) +#if defined(OS_POSIX) || defined(OS_FUCHSIA) unzFile OpenFdForUnzipping(int zip_fd) { - zlib_filefunc_def zip_funcs; + zlib_filefunc64_def zip_funcs; FillFdOpenFileFunc(&zip_funcs, zip_fd); // Passing dummy "fd" filename to zlib. - return unzOpen2("fd", &zip_funcs); + return unzOpen2_64("fd", &zip_funcs); } #endif #if defined(OS_WIN) unzFile OpenHandleForUnzipping(HANDLE zip_handle) { - zlib_filefunc_def zip_funcs; - fill_win32_filefunc(&zip_funcs); - zip_funcs.zopen_file = HandleOpenFileFunc; + zlib_filefunc64_def zip_funcs; + fill_win32_filefunc64(&zip_funcs); + zip_funcs.zopen64_file = HandleOpenFileFunc; zip_funcs.zclose_file = HandleCloseFileFunc; zip_funcs.opaque = zip_handle; - return unzOpen2("fd", &zip_funcs); + return unzOpen2_64("fd", &zip_funcs); } #endif @@ -310,72 +318,152 @@ unzFile PrepareMemoryForUnzipping(const std::string& data) { buffer->length = data.length(); buffer->offset = 0; - zlib_filefunc_def zip_functions; - zip_functions.zopen_file = OpenZipBuffer; + zlib_filefunc64_def zip_functions; + zip_functions.zopen64_file = OpenZipBuffer; zip_functions.zread_file = ReadZipBuffer; zip_functions.zwrite_file = WriteZipBuffer; - zip_functions.ztell_file = GetOffsetOfZipBuffer; - zip_functions.zseek_file = SeekZipBuffer; + zip_functions.ztell64_file = GetOffsetOfZipBuffer; + zip_functions.zseek64_file = SeekZipBuffer; zip_functions.zclose_file = CloseZipBuffer; zip_functions.zerror_file = GetErrorOfZipBuffer; - zip_functions.opaque = static_cast<void*>(buffer); - return unzOpen2(NULL, &zip_functions); + zip_functions.opaque = buffer; + return unzOpen2_64(nullptr, &zip_functions); } zipFile OpenForZipping(const std::string& file_name_utf8, int append_flag) { - zlib_filefunc_def* zip_func_ptrs = NULL; + zlib_filefunc64_def* zip_func_ptrs = nullptr; #if defined(OS_WIN) - zlib_filefunc_def zip_funcs; - fill_win32_filefunc(&zip_funcs); - zip_funcs.zopen_file = ZipOpenFunc; + zlib_filefunc64_def zip_funcs; + fill_win32_filefunc64(&zip_funcs); + zip_funcs.zopen64_file = ZipOpenFunc; zip_func_ptrs = &zip_funcs; #endif - return zipOpen2(file_name_utf8.c_str(), - append_flag, - NULL, // global comment - zip_func_ptrs); + return zipOpen2_64(file_name_utf8.c_str(), append_flag, nullptr, + zip_func_ptrs); } -#if defined(OS_POSIX) +#if defined(OS_POSIX) || defined(OS_FUCHSIA) zipFile OpenFdForZipping(int zip_fd, int append_flag) { - zlib_filefunc_def zip_funcs; + zlib_filefunc64_def zip_funcs; FillFdOpenFileFunc(&zip_funcs, zip_fd); // Passing dummy "fd" filename to zlib. - return zipOpen2("fd", append_flag, NULL, &zip_funcs); + return zipOpen2_64("fd", append_flag, nullptr, &zip_funcs); } #endif bool ZipOpenNewFileInZip(zipFile zip_file, const std::string& str_path, - base::Time last_modified_time) { + base::Time last_modified_time, + Compression compression) { // Section 4.4.4 http://www.pkware.com/documents/casestudies/APPNOTE.TXT // Setting the Language encoding flag so the file is told to be in utf-8. const uLong LANGUAGE_ENCODING_FLAG = 0x1 << 11; - zip_fileinfo file_info = TimeToZipFileInfo(last_modified_time); - if (ZIP_OK != zipOpenNewFileInZip4(zip_file, // file - str_path.c_str(), // filename - &file_info, // zip_fileinfo - NULL, // extrafield_local, - 0u, // size_extrafield_local - NULL, // extrafield_global - 0u, // size_extrafield_global - NULL, // comment - Z_DEFLATED, // method - Z_DEFAULT_COMPRESSION, // level - 0, // raw - -MAX_WBITS, // windowBits - DEF_MEM_LEVEL, // memLevel - Z_DEFAULT_STRATEGY, // strategy - NULL, // password - 0, // crcForCrypting - 0, // versionMadeBy - LANGUAGE_ENCODING_FLAG)) { // flagBase - DLOG(ERROR) << "Could not open zip file entry " << str_path; + const zip_fileinfo file_info = TimeToZipFileInfo(last_modified_time); + const int err = zipOpenNewFileInZip4_64( + /*file=*/zip_file, + /*filename=*/str_path.c_str(), + /*zip_fileinfo=*/&file_info, + /*extrafield_local=*/nullptr, + /*size_extrafield_local=*/0u, + /*extrafield_global=*/nullptr, + /*size_extrafield_global=*/0u, + /*comment=*/nullptr, + /*method=*/compression, + /*level=*/Z_DEFAULT_COMPRESSION, + /*raw=*/0, + /*windowBits=*/-MAX_WBITS, + /*memLevel=*/DEF_MEM_LEVEL, + /*strategy=*/Z_DEFAULT_STRATEGY, + /*password=*/nullptr, + /*crcForCrypting=*/0, + /*versionMadeBy=*/0, + /*flagBase=*/LANGUAGE_ENCODING_FLAG, + /*zip64=*/1); + + if (err != ZIP_OK) { + DLOG(ERROR) << "Cannot open ZIP file entry '" << str_path + << "': zipOpenNewFileInZip4_64 returned " << err; return false; } + return true; } +Compression GetCompressionMethod(const base::FilePath& path) { + // Get the filename extension in lower case. + const base::FilePath::StringType ext = + base::ToLowerASCII(path.FinalExtension()); + + if (ext.empty()) + return kDeflated; + + using StringPiece = base::FilePath::StringPieceType; + + // Skip the leading dot. + StringPiece ext_without_dot = ext; + DCHECK_EQ(ext_without_dot.front(), FILE_PATH_LITERAL('.')); + ext_without_dot.remove_prefix(1); + + // Well known filename extensions of files that a likely to be already + // compressed. The extensions are in lower case without the leading dot. + static const base::NoDestructor< + std::unordered_set<StringPiece, base::StringPieceHashImpl<StringPiece>>> + exts(std::initializer_list<StringPiece>{ + FILE_PATH_LITERAL("3g2"), // + FILE_PATH_LITERAL("3gp"), // + FILE_PATH_LITERAL("7z"), // + FILE_PATH_LITERAL("7zip"), // + FILE_PATH_LITERAL("aac"), // + FILE_PATH_LITERAL("avi"), // + FILE_PATH_LITERAL("bz"), // + FILE_PATH_LITERAL("bz2"), // + FILE_PATH_LITERAL("crx"), // + FILE_PATH_LITERAL("gif"), // + FILE_PATH_LITERAL("gz"), // + FILE_PATH_LITERAL("jar"), // + FILE_PATH_LITERAL("jpeg"), // + FILE_PATH_LITERAL("jpg"), // + FILE_PATH_LITERAL("lz"), // + FILE_PATH_LITERAL("m2v"), // + FILE_PATH_LITERAL("m4p"), // + FILE_PATH_LITERAL("m4v"), // + FILE_PATH_LITERAL("mng"), // + FILE_PATH_LITERAL("mov"), // + FILE_PATH_LITERAL("mp2"), // + FILE_PATH_LITERAL("mp3"), // + FILE_PATH_LITERAL("mp4"), // + FILE_PATH_LITERAL("mpe"), // + FILE_PATH_LITERAL("mpeg"), // + FILE_PATH_LITERAL("mpg"), // + FILE_PATH_LITERAL("mpv"), // + FILE_PATH_LITERAL("ogg"), // + FILE_PATH_LITERAL("ogv"), // + FILE_PATH_LITERAL("png"), // + FILE_PATH_LITERAL("qt"), // + FILE_PATH_LITERAL("rar"), // + FILE_PATH_LITERAL("taz"), // + FILE_PATH_LITERAL("tb2"), // + FILE_PATH_LITERAL("tbz"), // + FILE_PATH_LITERAL("tbz2"), // + FILE_PATH_LITERAL("tgz"), // + FILE_PATH_LITERAL("tlz"), // + FILE_PATH_LITERAL("tz"), // + FILE_PATH_LITERAL("tz2"), // + FILE_PATH_LITERAL("vob"), // + FILE_PATH_LITERAL("webm"), // + FILE_PATH_LITERAL("wma"), // + FILE_PATH_LITERAL("wmv"), // + FILE_PATH_LITERAL("xz"), // + FILE_PATH_LITERAL("z"), // + FILE_PATH_LITERAL("zip"), // + }); + + if (exts->count(ext_without_dot)) + return kStored; + + return kDeflated; +} + } // namespace internal } // namespace zip diff --git a/google/zip_internal.h b/google/zip_internal.h index 49fb902..92833fa 100644 --- a/google/zip_internal.h +++ b/google/zip_internal.h @@ -35,7 +35,7 @@ namespace internal { // Windows. unzFile OpenForUnzipping(const std::string& file_name_utf8); -#if defined(OS_POSIX) +#if defined(OS_POSIX) || defined(OS_FUCHSIA) // Opens the file referred to by |zip_fd| for unzipping. unzFile OpenFdForUnzipping(int zip_fd); #endif @@ -54,16 +54,30 @@ unzFile PrepareMemoryForUnzipping(const std::string& data); // Windows. |append_flag| will be passed to zipOpen2(). zipFile OpenForZipping(const std::string& file_name_utf8, int append_flag); -#if defined(OS_POSIX) +#if defined(OS_POSIX) || defined(OS_FUCHSIA) // Opens the file referred to by |zip_fd| for zipping. |append_flag| will be // passed to zipOpen2(). zipFile OpenFdForZipping(int zip_fd, int append_flag); #endif -// Wrapper around zipOpenNewFileInZip4 which passes most common options. +// Compression methods. +enum Compression { + kStored = 0, // Stored (no compression) + kDeflated = Z_DEFLATED, // Deflated +}; + +// Adds a file (or directory) entry to the ZIP archive. bool ZipOpenNewFileInZip(zipFile zip_file, const std::string& str_path, - base::Time last_modified_time); + base::Time last_modified_time, + Compression compression); + +// Selects the best compression method for the given file. The heuristic is +// based on the filename extension. By default, the compression method is +// kDeflated. But if the given path has an extension indicating a well known +// file format which is likely to be already compressed (eg ZIP, RAR, JPG, +// PNG...) then the compression method is simply kStored. +Compression GetCompressionMethod(const base::FilePath& path); const int kZipMaxPath = 256; const int kZipBufSize = 8192; diff --git a/google/zip_reader.cc b/google/zip_reader.cc index 1910cf2..b8143cd 100644 --- a/google/zip_reader.cc +++ b/google/zip_reader.cc @@ -4,16 +4,23 @@ #include "third_party/zlib/google/zip_reader.h" +#include <algorithm> #include <utility> #include "base/bind.h" +#include "base/check.h" #include "base/files/file.h" +#include "base/files/file_util.h" +#include "base/i18n/icu_string_conversions.h" #include "base/logging.h" -#include "base/macros.h" +#include "base/numerics/safe_conversions.h" +#include "base/strings/strcat.h" +#include "base/strings/string_piece.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "base/threading/sequenced_task_runner_handle.h" #include "build/build_config.h" +#include "third_party/zlib/google/redact.h" #include "third_party/zlib/google/zip_internal.h" #if defined(USE_SYSTEM_MINIZIP) @@ -25,114 +32,73 @@ #endif // defined(OS_WIN) #endif // defined(USE_SYSTEM_MINIZIP) -namespace zip { +#if defined(OS_POSIX) +#include <sys/stat.h> +#endif +namespace zip { namespace { -// StringWriterDelegate -------------------------------------------------------- +enum UnzipError : int; + +std::ostream& operator<<(std::ostream& out, UnzipError error) { +#define SWITCH_ERR(X) \ + case X: \ + return out << #X; + switch (error) { + SWITCH_ERR(UNZ_OK); + SWITCH_ERR(UNZ_END_OF_LIST_OF_FILE); + SWITCH_ERR(UNZ_ERRNO); + SWITCH_ERR(UNZ_PARAMERROR); + SWITCH_ERR(UNZ_BADZIPFILE); + SWITCH_ERR(UNZ_INTERNALERROR); + SWITCH_ERR(UNZ_CRCERROR); + default: + return out << "UNZ" << static_cast<int>(error); + } +#undef SWITCH_ERR +} -// 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() 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; + 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_; - - DISALLOW_COPY_AND_ASSIGN(StringWriterDelegate); + 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. +#if defined(OS_POSIX) +void SetPosixFilePermissions(int fd, int mode) { + base::stat_wrapper_t sb; + if (base::File::Fstat(fd, &sb)) { + return; + } + mode_t new_mode = sb.st_mode; + // Transfer the executable bit only if the file is readable. + if ((sb.st_mode & S_IRUSR) == S_IRUSR && (mode & S_IXUSR) == S_IXUSR) { + new_mode |= S_IXUSR; + } + if ((sb.st_mode & S_IRGRP) == S_IRGRP && (mode & S_IXGRP) == S_IXGRP) { + new_mode |= S_IXGRP; + } + if ((sb.st_mode & S_IROTH) == S_IROTH && (mode & S_IXOTH) == S_IXOTH) { + new_mode |= S_IXOTH; + } + if (new_mode != sb.st_mode) { + fchmod(fd, new_mode); + } } +#endif } // namespace -// TODO(satorux): The implementation assumes that file names in zip files -// are encoded in UTF-8. This is true for zip files created by Zip() -// function in zip.h, but not true for user-supplied random zip files. -ZipReader::EntryInfo::EntryInfo(const std::string& file_name_in_zip, - const unz_file_info& raw_file_info) - : file_path_(base::FilePath::FromUTF8Unsafe(file_name_in_zip)), - is_directory_(false), - is_unsafe_(false), - is_encrypted_(false) { - original_size_ = raw_file_info.uncompressed_size; - - // Directory entries in zip files end with "/". - is_directory_ = base::EndsWith(file_name_in_zip, "/", - base::CompareCase::INSENSITIVE_ASCII); - - // Check the file name here for directory traversal issues. - is_unsafe_ = file_path_.ReferencesParent(); - - // We also consider that the file name is unsafe, if it's invalid UTF-8. - std::u16string file_name_utf16; - if (!base::UTF8ToUTF16(file_name_in_zip.data(), file_name_in_zip.size(), - &file_name_utf16)) { - is_unsafe_ = true; - } - - // We also consider that the file name is unsafe, if it's absolute. - // On Windows, IsAbsolute() returns false for paths starting with "/". - if (file_path_.IsAbsolute() || - base::StartsWith(file_name_in_zip, "/", - base::CompareCase::INSENSITIVE_ASCII)) - is_unsafe_ = true; - - // Whether the file is encrypted is bit 0 of the flag. - is_encrypted_ = raw_file_info.flag & 1; - - // Construct the last modified time. The timezone info is not present in - // zip files, so we construct the time as local time. - base::Time::Exploded exploded_time = {}; // Zero-clear. - exploded_time.year = raw_file_info.tmu_date.tm_year; - // The month in zip file is 0-based, whereas ours is 1-based. - exploded_time.month = raw_file_info.tmu_date.tm_mon + 1; - exploded_time.day_of_month = raw_file_info.tmu_date.tm_mday; - exploded_time.hour = raw_file_info.tmu_date.tm_hour; - exploded_time.minute = raw_file_info.tmu_date.tm_min; - exploded_time.second = raw_file_info.tmu_date.tm_sec; - exploded_time.millisecond = 0; - - if (!base::Time::FromLocalExploded(exploded_time, &last_modified_)) - last_modified_ = base::Time::UnixEpoch(); -} - ZipReader::ZipReader() { Reset(); } @@ -141,13 +107,14 @@ ZipReader::~ZipReader() { Close(); } -bool ZipReader::Open(const base::FilePath& zip_file_path) { +bool ZipReader::Open(const base::FilePath& zip_path) { DCHECK(!zip_file_); // Use of "Unsafe" function does not look good, but there is no way to do // this safely on Linux. See file_util.h for details. - zip_file_ = internal::OpenForUnzipping(zip_file_path.AsUTF8Unsafe()); + zip_file_ = internal::OpenForUnzipping(zip_path.AsUTF8Unsafe()); if (!zip_file_) { + LOG(ERROR) << "Cannot open ZIP archive " << Redact(zip_path); return false; } @@ -157,12 +124,13 @@ bool ZipReader::Open(const base::FilePath& zip_file_path) { bool ZipReader::OpenFromPlatformFile(base::PlatformFile zip_fd) { DCHECK(!zip_file_); -#if defined(OS_POSIX) +#if defined(OS_POSIX) || defined(OS_FUCHSIA) zip_file_ = internal::OpenFdForUnzipping(zip_fd); #elif defined(OS_WIN) zip_file_ = internal::OpenHandleForUnzipping(zip_fd); #endif if (!zip_file_) { + LOG(ERROR) << "Cannot open ZIP from file handle " << zip_fd; return false; } @@ -178,107 +146,183 @@ bool ZipReader::OpenFromString(const std::string& data) { void ZipReader::Close() { if (zip_file_) { - unzClose(zip_file_); + if (const UnzipError err{unzClose(zip_file_)}; err != UNZ_OK) { + LOG(ERROR) << "Error while closing ZIP archive: " << err; + } } Reset(); } -bool ZipReader::HasMore() { - return !reached_end_; -} - -bool ZipReader::AdvanceToNextEntry() { +const ZipReader::Entry* ZipReader::Next() { DCHECK(zip_file_); - // Should not go further if we already reached the end. if (reached_end_) - return false; + return nullptr; - unz_file_pos position = {}; - if (unzGetFilePos(zip_file_, &position) != UNZ_OK) - return false; - const int current_entry_index = position.num_of_file; - // If we are currently at the last entry, then the next position is the - // end of the zip file, so mark that we reached the end. - if (current_entry_index + 1 == num_entries_) { - reached_end_ = true; - } else { - DCHECK_LT(current_entry_index + 1, num_entries_); - if (unzGoToNextFile(zip_file_) != UNZ_OK) { - return false; + DCHECK(ok_); + + // Move to the next entry if we're not trying to open the first entry. + if (next_index_ > 0) { + if (const UnzipError err{unzGoToNextFile(zip_file_)}; err != UNZ_OK) { + reached_end_ = true; + if (err != UNZ_END_OF_LIST_OF_FILE) { + LOG(ERROR) << "Cannot go to next entry in ZIP: " << err; + ok_ = false; + } + return nullptr; } } - current_entry_info_.reset(); - return true; + + next_index_++; + + if (!OpenEntry()) { + reached_end_ = true; + ok_ = false; + return nullptr; + } + + return &entry_; } -bool ZipReader::OpenCurrentEntryInZip() { +bool ZipReader::OpenEntry() { DCHECK(zip_file_); - unz_file_info raw_file_info = {}; - char raw_file_name_in_zip[internal::kZipMaxPath] = {}; - const int result = unzGetCurrentFileInfo(zip_file_, - &raw_file_info, - raw_file_name_in_zip, - sizeof(raw_file_name_in_zip) - 1, - NULL, // extraField. - 0, // extraFieldBufferSize. - NULL, // szComment. - 0); // commentBufferSize. - if (result != UNZ_OK) + // Get entry info. + unz_file_info64 info = {}; + char path_in_zip[internal::kZipMaxPath] = {}; + if (const UnzipError err{unzGetCurrentFileInfo64( + zip_file_, &info, path_in_zip, sizeof(path_in_zip) - 1, nullptr, 0, + nullptr, 0)}; + err != UNZ_OK) { + LOG(ERROR) << "Cannot get entry from ZIP: " << err; return false; - if (raw_file_name_in_zip[0] == '\0') + } + + entry_.path_in_original_encoding = path_in_zip; + + // Convert path from original encoding to Unicode. + std::u16string path_in_utf16; + const char* const encoding = encoding_.empty() ? "UTF-8" : encoding_.c_str(); + if (!base::CodepageToUTF16(entry_.path_in_original_encoding, encoding, + base::OnStringConversionError::SUBSTITUTE, + &path_in_utf16)) { + LOG(ERROR) << "Cannot convert path from encoding " << encoding; return false; - current_entry_info_.reset( - new EntryInfo(raw_file_name_in_zip, raw_file_info)); + } + + entry_.path = base::FilePath::FromUTF16Unsafe(path_in_utf16); + entry_.original_size = info.uncompressed_size; + + // Directory entries in ZIP have a path ending with "/". + entry_.is_directory = base::EndsWith(path_in_utf16, u"/"); + + // Check the entry path for directory traversal issues. We consider entry + // paths unsafe if they are absolute or if they contain "..". On Windows, + // IsAbsolute() returns false for paths starting with "/". + entry_.is_unsafe = entry_.path.ReferencesParent() || + entry_.path.IsAbsolute() || + base::StartsWith(path_in_utf16, u"/"); + + // The file content of this entry is encrypted if flag bit 0 is set. + entry_.is_encrypted = info.flag & 1; + + // Construct the last modified time. The timezone info is not present in ZIP + // archives, so we construct the time as UTC. + base::Time::Exploded exploded_time = {}; + exploded_time.year = info.tmu_date.tm_year; + exploded_time.month = info.tmu_date.tm_mon + 1; // 0-based vs 1-based + exploded_time.day_of_month = info.tmu_date.tm_mday; + exploded_time.hour = info.tmu_date.tm_hour; + exploded_time.minute = info.tmu_date.tm_min; + exploded_time.second = info.tmu_date.tm_sec; + exploded_time.millisecond = 0; + + if (!base::Time::FromUTCExploded(exploded_time, &entry_.last_modified)) + entry_.last_modified = base::Time::UnixEpoch(); + +#if defined(OS_POSIX) + entry_.posix_mode = (info.external_fa >> 16L) & (S_IRWXU | S_IRWXG | S_IRWXO); +#else + entry_.posix_mode = 0; +#endif + return true; } bool ZipReader::ExtractCurrentEntry(WriterDelegate* delegate, uint64_t num_bytes_to_extract) const { DCHECK(zip_file_); - - const int open_result = unzOpenCurrentFile(zip_file_); - if (open_result != UNZ_OK) + DCHECK_LT(0, next_index_); + DCHECK(ok_); + DCHECK(!reached_end_); + + // Use password only for encrypted files. For non-encrypted files, no password + // is needed, and must be nullptr. + const char* const password = + entry_.is_encrypted ? password_.c_str() : nullptr; + if (const UnzipError err{unzOpenCurrentFilePassword(zip_file_, password)}; + err != UNZ_OK) { + LOG(ERROR) << "Cannot open file " << Redact(entry_.path) + << " from ZIP: " << err; return false; + } + DCHECK(delegate); if (!delegate->PrepareOutput()) return false; - std::unique_ptr<char[]> buf(new char[internal::kZipBufSize]); uint64_t remaining_capacity = num_bytes_to_extract; bool entire_file_extracted = false; while (remaining_capacity > 0) { + char buf[internal::kZipBufSize]; const int num_bytes_read = - unzReadCurrentFile(zip_file_, buf.get(), internal::kZipBufSize); + unzReadCurrentFile(zip_file_, buf, internal::kZipBufSize); if (num_bytes_read == 0) { entire_file_extracted = true; break; - } else if (num_bytes_read < 0) { - // If num_bytes_read < 0, then it's a specific UNZ_* error code. + } + + if (num_bytes_read < 0) { + LOG(ERROR) << "Cannot read file " << Redact(entry_.path) + << " from ZIP: " << UnzipError(num_bytes_read); break; - } else if (num_bytes_read > 0) { - uint64_t num_bytes_to_write = std::min<uint64_t>( - remaining_capacity, base::checked_cast<uint64_t>(num_bytes_read)); - if (!delegate->WriteBytes(buf.get(), num_bytes_to_write)) - break; - if (remaining_capacity == base::checked_cast<uint64_t>(num_bytes_read)) { - // Ensures function returns true if the entire file has been read. - entire_file_extracted = - (unzReadCurrentFile(zip_file_, buf.get(), 1) == 0); - } - CHECK_GE(remaining_capacity, num_bytes_to_write); - remaining_capacity -= num_bytes_to_write; } + + DCHECK_LT(0, num_bytes_read); + CHECK_LE(num_bytes_read, internal::kZipBufSize); + + uint64_t num_bytes_to_write = std::min<uint64_t>( + remaining_capacity, base::checked_cast<uint64_t>(num_bytes_read)); + if (!delegate->WriteBytes(buf, num_bytes_to_write)) + break; + + if (remaining_capacity == base::checked_cast<uint64_t>(num_bytes_read)) { + // Ensures function returns true if the entire file has been read. + const int n = unzReadCurrentFile(zip_file_, buf, 1); + entire_file_extracted = (n == 0); + LOG_IF(ERROR, n < 0) << "Cannot read file " << Redact(entry_.path) + << " from ZIP: " << UnzipError(n); + } + + CHECK_GE(remaining_capacity, num_bytes_to_write); + remaining_capacity -= num_bytes_to_write; } - unzCloseCurrentFile(zip_file_); + if (const UnzipError err{unzCloseCurrentFile(zip_file_)}; err != UNZ_OK) { + LOG(ERROR) << "Cannot extract file " << Redact(entry_.path) + << " from ZIP: " << err; + entire_file_extracted = false; + } - if (entire_file_extracted && - current_entry_info()->last_modified() != base::Time::UnixEpoch()) { - delegate->SetTimeModified(current_entry_info()->last_modified()); + if (entire_file_extracted) { + delegate->SetPosixFilePermissions(entry_.posix_mode); + if (entry_.last_modified != base::Time::UnixEpoch()) { + delegate->SetTimeModified(entry_.last_modified); + } + } else { + delegate->OnError(); } return entire_file_extracted; @@ -288,25 +332,33 @@ void ZipReader::ExtractCurrentEntryToFilePathAsync( const base::FilePath& output_file_path, SuccessCallback success_callback, FailureCallback failure_callback, - const ProgressCallback& progress_callback) { + ProgressCallback progress_callback) { DCHECK(zip_file_); - DCHECK(current_entry_info_.get()); + DCHECK_LT(0, next_index_); + DCHECK(ok_); + DCHECK(!reached_end_); // If this is a directory, just create it and return. - if (current_entry_info()->is_directory()) { + if (entry_.is_directory) { if (base::CreateDirectory(output_file_path)) { base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, std::move(success_callback)); } else { - DVLOG(1) << "Unzip failed: unable to create directory."; + LOG(ERROR) << "Cannot create directory " << Redact(output_file_path); base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, std::move(failure_callback)); } return; } - if (unzOpenCurrentFile(zip_file_) != UNZ_OK) { - DVLOG(1) << "Unzip failed: unable to open current zip entry."; + // Use password only for encrypted files. For non-encrypted files, no password + // is needed, and must be nullptr. + const char* const password = + entry_.is_encrypted ? password_.c_str() : nullptr; + if (const UnzipError err{unzOpenCurrentFilePassword(zip_file_, password)}; + err != UNZ_OK) { + LOG(ERROR) << "Cannot open file " << Redact(entry_.path) + << " from ZIP: " << err; base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, std::move(failure_callback)); return; @@ -314,7 +366,7 @@ void ZipReader::ExtractCurrentEntryToFilePathAsync( base::FilePath output_dir_path = output_file_path.DirName(); if (!base::CreateDirectory(output_dir_path)) { - DVLOG(1) << "Unzip failed: unable to create containing directory."; + LOG(ERROR) << "Cannot create directory " << Redact(output_dir_path); base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, std::move(failure_callback)); return; @@ -324,8 +376,7 @@ void ZipReader::ExtractCurrentEntryToFilePathAsync( base::File output_file(output_file_path, flags); if (!output_file.IsValid()) { - DVLOG(1) << "Unzip failed: unable to create platform file at " - << output_file_path.value(); + LOG(ERROR) << "Cannot create file " << Redact(output_file_path); base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, std::move(failure_callback)); return; @@ -335,7 +386,7 @@ void ZipReader::ExtractCurrentEntryToFilePathAsync( FROM_HERE, base::BindOnce(&ZipReader::ExtractChunk, weak_ptr_factory_.GetWeakPtr(), std::move(output_file), std::move(success_callback), - std::move(failure_callback), progress_callback, + std::move(failure_callback), std::move(progress_callback), 0 /* initial offset */)); } @@ -343,120 +394,122 @@ bool ZipReader::ExtractCurrentEntryToString(uint64_t max_read_bytes, std::string* output) const { DCHECK(output); DCHECK(zip_file_); + DCHECK_LT(0, next_index_); + 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()))); - - StringWriterDelegate writer(max_read_bytes, &contents); - if (!ExtractCurrentEntry(&writer, max_read_bytes)) { - if (contents.length() < 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->clear(); - } else { - // |num_bytes| is less than the length of current entry. - output->swap(contents); - } - return false; - } - output->swap(contents); - 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. + output->reserve(base::checked_cast<size_t>(std::min<uint64_t>( + max_read_bytes, base::checked_cast<uint64_t>(entry_.original_size)))); + + StringWriterDelegate writer(output); + return ExtractCurrentEntry(&writer, max_read_bytes); } bool ZipReader::OpenInternal() { DCHECK(zip_file_); unz_global_info zip_info = {}; // Zero-clear. - if (unzGetGlobalInfo(zip_file_, &zip_info) != UNZ_OK) { + if (const UnzipError err{unzGetGlobalInfo(zip_file_, &zip_info)}; + err != UNZ_OK) { + LOG(ERROR) << "Cannot get ZIP info: " << err; return false; } - num_entries_ = zip_info.number_entry; - if (num_entries_ < 0) - return false; - // We are already at the end if the zip file is empty. - reached_end_ = (num_entries_ == 0); + num_entries_ = zip_info.number_entry; + reached_end_ = (num_entries_ <= 0); + ok_ = true; return true; } void ZipReader::Reset() { - zip_file_ = NULL; + zip_file_ = nullptr; num_entries_ = 0; - reached_end_ = false; - current_entry_info_.reset(); + next_index_ = 0; + reached_end_ = true; + ok_ = false; + entry_ = {}; } void ZipReader::ExtractChunk(base::File output_file, SuccessCallback success_callback, FailureCallback failure_callback, - const ProgressCallback& progress_callback, - const int64_t offset) { + ProgressCallback progress_callback, + int64_t offset) { char buffer[internal::kZipBufSize]; - const int num_bytes_read = unzReadCurrentFile(zip_file_, - buffer, - internal::kZipBufSize); + const int num_bytes_read = + unzReadCurrentFile(zip_file_, buffer, internal::kZipBufSize); if (num_bytes_read == 0) { - unzCloseCurrentFile(zip_file_); - std::move(success_callback).Run(); - } else if (num_bytes_read < 0) { - DVLOG(1) << "Unzip failed: error while reading zipfile " - << "(" << num_bytes_read << ")"; - std::move(failure_callback).Run(); - } else { - if (num_bytes_read != output_file.Write(offset, buffer, num_bytes_read)) { - DVLOG(1) << "Unzip failed: unable to write all bytes to target."; + if (const UnzipError err{unzCloseCurrentFile(zip_file_)}; err != UNZ_OK) { + LOG(ERROR) << "Cannot extract file " << Redact(entry_.path) + << " from ZIP: " << err; std::move(failure_callback).Run(); return; } - int64_t current_progress = offset + num_bytes_read; + std::move(success_callback).Run(); + return; + } - progress_callback.Run(current_progress); + if (num_bytes_read < 0) { + LOG(ERROR) << "Cannot read file " << Redact(entry_.path) + << " from ZIP: " << UnzipError(num_bytes_read); + std::move(failure_callback).Run(); + return; + } - base::SequencedTaskRunnerHandle::Get()->PostTask( - FROM_HERE, - base::BindOnce(&ZipReader::ExtractChunk, weak_ptr_factory_.GetWeakPtr(), - std::move(output_file), std::move(success_callback), - std::move(failure_callback), progress_callback, - current_progress)); + if (num_bytes_read != output_file.Write(offset, buffer, num_bytes_read)) { + LOG(ERROR) << "Cannot write " << num_bytes_read + << " bytes to file at offset " << offset; + std::move(failure_callback).Run(); + return; } + + offset += num_bytes_read; + progress_callback.Run(offset); + + base::SequencedTaskRunnerHandle::Get()->PostTask( + FROM_HERE, + base::BindOnce(&ZipReader::ExtractChunk, weak_ptr_factory_.GetWeakPtr(), + std::move(output_file), std::move(success_callback), + std::move(failure_callback), std::move(progress_callback), + offset)); } // FileWriterDelegate ---------------------------------------------------------- -FileWriterDelegate::FileWriterDelegate(base::File* file) : file_(file) {} - -FileWriterDelegate::FileWriterDelegate(std::unique_ptr<base::File> file) - : file_(file.get()), owned_file_(std::move(file)) {} +FileWriterDelegate::FileWriterDelegate(base::File* file) : file_(file) { + DCHECK(file_); +} -FileWriterDelegate::~FileWriterDelegate() { - if (!file_->SetLength(file_length_)) { - DVPLOG(1) << "Failed updating length of written file"; - } +FileWriterDelegate::FileWriterDelegate(base::File owned_file) + : owned_file_(std::move(owned_file)) { + DCHECK_EQ(file_, &owned_file_); } +FileWriterDelegate::~FileWriterDelegate() {} + bool FileWriterDelegate::PrepareOutput() { - return file_->Seek(base::File::FROM_BEGIN, 0) >= 0; + DCHECK(file_); + const bool ok = file_->IsValid(); + if (ok) { + DCHECK_EQ(file_->GetLength(), 0) + << " The output file should be initially empty"; + } + return ok; } bool FileWriterDelegate::WriteBytes(const char* data, int num_bytes) { @@ -470,32 +523,50 @@ void FileWriterDelegate::SetTimeModified(const base::Time& time) { file_->SetTimes(base::Time::Now(), time); } +void FileWriterDelegate::SetPosixFilePermissions(int mode) { +#if defined(OS_POSIX) + zip::SetPosixFilePermissions(file_->GetPlatformFile(), mode); +#endif +} + +void FileWriterDelegate::OnError() { + file_length_ = 0; + file_->SetLength(0); +} + // FilePathWriterDelegate ------------------------------------------------------ -FilePathWriterDelegate::FilePathWriterDelegate( - const base::FilePath& output_file_path) - : output_file_path_(output_file_path) {} +FilePathWriterDelegate::FilePathWriterDelegate(base::FilePath output_file_path) + : FileWriterDelegate(base::File()), + output_file_path_(std::move(output_file_path)) {} FilePathWriterDelegate::~FilePathWriterDelegate() {} bool FilePathWriterDelegate::PrepareOutput() { // We can't rely on parent directory entries being specified in the // zip, so we make sure they are created. - if (!base::CreateDirectory(output_file_path_.DirName())) + if (const base::FilePath dir = output_file_path_.DirName(); + !base::CreateDirectory(dir)) { + PLOG(ERROR) << "Cannot create directory " << Redact(dir); return false; + } - file_.Initialize(output_file_path_, - base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); - return file_.IsValid(); + owned_file_.Initialize(output_file_path_, + base::File::FLAG_CREATE | base::File::FLAG_WRITE); + PLOG_IF(ERROR, !owned_file_.IsValid()) + << "Cannot create file " << Redact(output_file_path_) << ": " + << base::File::ErrorToString(owned_file_.error_details()); + return FileWriterDelegate::PrepareOutput(); } -bool FilePathWriterDelegate::WriteBytes(const char* data, int num_bytes) { - return num_bytes == file_.WriteAtCurrentPos(data, num_bytes); -} +void FilePathWriterDelegate::OnError() { + FileWriterDelegate::OnError(); + owned_file_.Close(); -void FilePathWriterDelegate::SetTimeModified(const base::Time& time) { - file_.Close(); - base::TouchFile(output_file_path_, base::Time::Now(), time); + if (!base::DeleteFile(output_file_path_)) { + LOG(ERROR) << "Cannot delete partially extracted file " + << Redact(output_file_path_); + } } } // namespace zip diff --git a/google/zip_reader.h b/google/zip_reader.h index d442d42..df7452a 100644 --- a/google/zip_reader.h +++ b/google/zip_reader.h @@ -7,15 +7,15 @@ #include <stddef.h> #include <stdint.h> +#include <limits> #include <memory> #include <string> #include "base/callback.h" #include "base/files/file.h" #include "base/files/file_path.h" -#include "base/files/file_util.h" -#include "base/macros.h" #include "base/memory/weak_ptr.h" +#include "base/numerics/safe_conversions.h" #include "base/time/time.h" #if defined(USE_SYSTEM_MINIZIP) @@ -34,33 +34,47 @@ 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) {} + + // Called if an error occurred while extracting the file. The WriterDelegate + // can then remove and clean up the partially extracted data. + virtual void OnError() {} }; -// This class is used for reading zip files. A typical use case of this -// class is to scan entries in a zip file and extract them. The code will -// look like: +// This class is used for reading ZIP archives. A typical use case of this class +// is to scan entries in a ZIP archive and extract them. The code will look +// like: // // ZipReader reader; -// reader.Open(zip_file_path); -// while (reader.HasMore()) { -// reader.OpenCurrentEntryInZip(); -// const base::FilePath& entry_path = -// reader.current_entry_info()->file_path(); -// auto writer = CreateFilePathWriterDelegate(extract_dir, entry_path); -// reader.ExtractCurrentEntry(writer, std::numeric_limits<uint64_t>::max()); -// reader.AdvanceToNextEntry(); +// if (!reader.Open(zip_path)) { +// // Cannot open +// return; // } // -// For simplicity, error checking is omitted in the example code above. The -// production code should check return values from all of these functions. +// while (const ZipReader::entry* entry = reader.Next()) { +// auto writer = CreateFilePathWriterDelegate(extract_dir, entry->path); +// if (!reader.ExtractCurrentEntry(writer)) { +// // Cannot extract +// return; +// } +// } +// +// if (!reader.ok()) { +// // Error while enumerating entries +// return; +// } // class ZipReader { public: @@ -72,62 +86,65 @@ class ZipReader { // of bytes that have been processed so far. using ProgressCallback = base::RepeatingCallback<void(int64_t)>; - // This class represents information of an entry (file or directory) in - // a zip file. - class EntryInfo { - public: - EntryInfo(const std::string& filename_in_zip, - const unz_file_info& raw_file_info); - - // Returns the file path. The path is usually relative like - // "foo/bar.txt", but if it's absolute, is_unsafe() returns true. - const base::FilePath& file_path() const { return file_path_; } - - // Returns the size of the original file (i.e. after uncompressed). - // Returns 0 if the entry is a directory. - // Note: this value should not be trusted, because it is stored as metadata - // in the zip archive and can be different from the real uncompressed size. - int64_t original_size() const { return original_size_; } - - // Returns the last modified time. If the time stored in the zip file was - // not valid, the unix epoch will be returned. + // Information of an entry (file or directory) in a ZIP archive. + struct Entry { + // Path of this entry, in its original encoding as it is stored in the ZIP + // archive. The encoding is not specified here. It might or might not be + // UTF-8, and the caller needs to use other means to determine the encoding + // if it wants to interpret this path correctly. + std::string path_in_original_encoding; + + // Path of the entry, converted to Unicode. This path is usually relative + // (eg "foo/bar.txt"), but it can also be absolute (eg "/foo/bar.txt") or + // parent-relative (eg "../foo/bar.txt"). See also |is_unsafe|. + base::FilePath path; + + // Size of the original uncompressed file, or 0 if the entry is a directory. + // This value should not be trusted, because it is stored as metadata in the + // ZIP archive and can be different from the real uncompressed size. + int64_t original_size; + + // Last modified time. If the timestamp stored in the ZIP archive is not + // valid, the Unix epoch will be returned. + // + // The timestamp stored in the ZIP archive uses the MS-DOS date and time + // format. // - // The time stored in the zip archive uses the MS-DOS date and time format. // http://msdn.microsoft.com/en-us/library/ms724247(v=vs.85).aspx + // // As such the following limitations apply: - // * only years from 1980 to 2107 can be represented. - // * the time stamp has a 2 second resolution. - // * there's no timezone information, so the time is interpreted as local. - base::Time last_modified() const { return last_modified_; } - - // Returns true if the entry is a directory. - bool is_directory() const { return is_directory_; } - - // Returns true if the entry is unsafe, like having ".." or invalid - // UTF-8 characters in its file name, or the file path is absolute. - bool is_unsafe() const { return is_unsafe_; } - - // Returns true if the entry is encrypted. - bool is_encrypted() const { return is_encrypted_; } - - private: - const base::FilePath file_path_; - int64_t original_size_; - base::Time last_modified_; - bool is_directory_; - bool is_unsafe_; - bool is_encrypted_; - DISALLOW_COPY_AND_ASSIGN(EntryInfo); + // * Only years from 1980 to 2107 can be represented. + // * The timestamp has a 2-second resolution. + // * There is no timezone information, so the time is interpreted as UTC. + base::Time last_modified; + + // True if the entry is a directory. + // False if the entry is a file. + bool is_directory; + + // True if the entry path is considered unsafe, ie if it is absolute or if + // it contains "..". + bool is_unsafe; + + // True if the file content is encrypted. + bool is_encrypted; + + // Entry POSIX permissions (POSIX systems only). + int posix_mode; }; ZipReader(); + + ZipReader(const ZipReader&) = delete; + ZipReader& operator=(const ZipReader&) = delete; + ~ZipReader(); - // Opens the zip file specified by |zip_file_path|. Returns true on + // Opens the ZIP archive specified by |zip_path|. Returns true on // success. - bool Open(const base::FilePath& zip_file_path); + bool Open(const base::FilePath& zip_path); - // Opens the zip file referred to by the platform file |zip_fd|, without + // Opens the ZIP archive referred to by the platform file |zip_fd|, without // taking ownership of |zip_fd|. Returns true on success. bool OpenFromPlatformFile(base::PlatformFile zip_fd); @@ -136,72 +153,94 @@ class ZipReader { // string until it finishes extracting files. bool OpenFromString(const std::string& data); - // Closes the currently opened zip file. This function is called in the + // Closes the currently opened ZIP archive. This function is called in the // destructor of the class, so you usually don't need to call this. void Close(); - // Returns true if there is at least one entry to read. This function is - // used to scan entries with AdvanceToNextEntry(), like: - // - // while (reader.HasMore()) { - // // Do something with the current file here. - // reader.AdvanceToNextEntry(); - // } - bool HasMore(); + // Sets the encoding of entry paths in the ZIP archive. + // By default, paths are assumed to be in UTF-8. + void SetEncoding(std::string encoding) { encoding_ = std::move(encoding); } - // Advances the next entry. Returns true on success. - bool AdvanceToNextEntry(); + // Sets the decryption password that will be used to decrypt encrypted file in + // the ZIP archive. + void SetPassword(std::string password) { password_ = std::move(password); } - // Opens the current entry in the zip file. On success, returns true and - // updates the the current entry state (i.e. current_entry_info() is - // updated). This function should be called before operations over the - // current entry like ExtractCurrentEntryToFile(). + // Gets the next entry. Returns null if there is no more entry, or if an error + // occurred while scanning entries. The returned Entry is owned by this + // ZipReader, and is valid until Next() is called again or until this + // ZipReader is closed. + // + // This function should be called before operations over the current entry + // like ExtractCurrentEntryToFile(). // - // Note that there is no CloseCurrentEntryInZip(). The the current entry - // state is reset automatically as needed. - bool OpenCurrentEntryInZip(); + // while (const ZipReader::Entry* entry = reader.Next()) { + // // Do something with the current entry here. + // ... + // } + // + // // Finished scanning entries. + // // Check if the scanning stopped because of an error. + // if (!reader.ok()) { + // // There was an error. + // ... + // } + const Entry* Next(); + + // Returns true if the enumeration of entries was successful, or false if it + // stopped because of an error. + bool ok() const { return ok_; } // Extracts |num_bytes_to_extract| bytes of the current entry to |delegate|, - // starting from the beginning of the entry. Return value specifies whether - // the entire file was extracted. + // starting from the beginning of the entry. + // + // Returns true if the entire file was extracted without error. + // + // Precondition: Next() returned a non-null Entry. bool ExtractCurrentEntry(WriterDelegate* delegate, - uint64_t num_bytes_to_extract) const; + uint64_t num_bytes_to_extract = + std::numeric_limits<uint64_t>::max()) const; - // Asynchronously extracts the current entry to the given output file path. - // If the current entry is a directory it just creates the directory - // synchronously instead. OpenCurrentEntryInZip() must be called beforehand. - // success_callback will be called on success and failure_callback will be - // called on failure. progress_callback will be called at least once. + // Asynchronously extracts the current entry to the given output file path. If + // the current entry is a directory it just creates the directory + // synchronously instead. + // + // |success_callback| will be called on success and |failure_callback| will be + // called on failure. |progress_callback| will be called at least once. // Callbacks will be posted to the current MessageLoop in-order. + // + // Precondition: Next() returned a non-null Entry. void ExtractCurrentEntryToFilePathAsync( const base::FilePath& output_file_path, SuccessCallback success_callback, FailureCallback failure_callback, - const ProgressCallback& progress_callback); + ProgressCallback progress_callback); // Extracts the current entry into memory. If the current entry is a - // directory, the |output| parameter is set to the empty string. If the - // current entry is a file, the |output| parameter is filled with its - // contents. OpenCurrentEntryInZip() must be called beforehand. Note: the - // |output| parameter can be filled with a big amount of data, avoid passing - // it around by value, but by reference or pointer. Note: the value returned - // by EntryInfo::original_size() cannot be trusted, so the real size of the - // uncompressed contents can be different. |max_read_bytes| limits the ammount - // of memory used to carry the entry. Returns true if the entire content is - // read. If the entry is bigger than |max_read_bytes|, returns false and - // |output| is filled with |max_read_bytes| of data. If an error occurs, - // returns false, and |output| is set to the empty string. + // directory, |*output| is set to the empty string. If the current entry is a + // file, |*output| is filled with its contents. + // + // The value in |Entry::original_size| cannot be trusted, so the real size of + // the uncompressed contents can be different. |max_read_bytes| limits the + // amount of memory used to carry the entry. + // + // Returns true if the entire content is read without error. If the content is + // bigger than |max_read_bytes|, this function returns false and |*output| is + // filled with |max_read_bytes| of data. If an error occurs, this function + // returns false and |*output| contains the content extracted so far, which + // might be garbage data. + // + // Precondition: Next() returned a non-null Entry. bool ExtractCurrentEntryToString(uint64_t max_read_bytes, std::string* output) const; - // Returns the current entry info. Returns NULL if the current entry is - // not yet opened. OpenCurrentEntryInZip() must be called beforehand. - EntryInfo* current_entry_info() const { - return current_entry_info_.get(); + bool ExtractCurrentEntryToString(std::string* output) const { + return ExtractCurrentEntryToString( + base::checked_cast<uint64_t>(output->max_size()), output); } - // Returns the number of entries in the zip file. - // Open() must be called beforehand. + // Returns the number of entries in the ZIP archive. + // + // Precondition: one of the Open() methods returned true. int num_entries() const { return num_entries_; } private: @@ -211,25 +250,35 @@ class ZipReader { // Resets the internal state. void Reset(); + // Opens the current entry in the ZIP archive. On success, returns true and + // updates the current entry state |entry_|. + // + // Note that there is no matching CloseEntry(). The current entry state is + // reset automatically as needed. + bool OpenEntry(); + // Extracts a chunk of the file to the target. Will post a task for the next // chunk and success/failure/progress callbacks as necessary. void ExtractChunk(base::File target_file, SuccessCallback success_callback, FailureCallback failure_callback, - const ProgressCallback& progress_callback, + ProgressCallback progress_callback, const int64_t offset); + std::string encoding_; + std::string password_; unzFile zip_file_; int num_entries_; + int next_index_; bool reached_end_; - std::unique_ptr<EntryInfo> current_entry_info_; + bool ok_; + Entry entry_; base::WeakPtrFactory<ZipReader> weak_ptr_factory_{this}; - - DISALLOW_COPY_AND_ASSIGN(ZipReader); }; -// A writer delegate that writes to a given File. +// A writer delegate that writes to a given File. This file is expected to be +// initially empty. class FileWriterDelegate : public WriterDelegate { public: // Constructs a FileWriterDelegate that manipulates |file|. The delegate will @@ -238,14 +287,14 @@ class FileWriterDelegate : public WriterDelegate { explicit FileWriterDelegate(base::File* file); // Constructs a FileWriterDelegate that takes ownership of |file|. - explicit FileWriterDelegate(std::unique_ptr<base::File> file); + explicit FileWriterDelegate(base::File owned_file); - // Truncates the file to the number of bytes written. - ~FileWriterDelegate() override; + FileWriterDelegate(const FileWriterDelegate&) = delete; + FileWriterDelegate& operator=(const FileWriterDelegate&) = delete; - // WriterDelegate methods: + ~FileWriterDelegate() override; - // Seeks to the beginning of the file, returning false if the seek fails. + // Returns true if the file handle passed to the constructor is valid. bool PrepareOutput() override; // Writes |num_bytes| bytes of |data| to the file, returning false on error or @@ -255,45 +304,48 @@ class FileWriterDelegate : public WriterDelegate { // Sets the last-modified time of the data. void SetTimeModified(const base::Time& time) override; - // Return the actual size of the file. - int64_t file_length() { return file_length_; } + // On POSIX systems, sets the file to be executable if the source file was + // executable. + void SetPosixFilePermissions(int mode) override; - private: - // The file the delegate modifies. - base::File* file_; + // Empties the file to avoid leaving garbage data in it. + void OnError() override; + + // Gets the number of bytes written into the file. + int64_t file_length() { return file_length_; } + protected: // The delegate can optionally own the file it modifies, in which case // owned_file_ is set and file_ is an alias for owned_file_. - std::unique_ptr<base::File> owned_file_; + base::File owned_file_; - int64_t file_length_ = 0; + // The file the delegate modifies. + base::File* const file_ = &owned_file_; - DISALLOW_COPY_AND_ASSIGN(FileWriterDelegate); + int64_t file_length_ = 0; }; -// A writer delegate that writes a file at a given path. -class FilePathWriterDelegate : public WriterDelegate { +// A writer delegate that creates and writes a file at a given path. This does +// not overwrite any existing file. +class FilePathWriterDelegate : public FileWriterDelegate { public: - explicit FilePathWriterDelegate(const base::FilePath& output_file_path); - ~FilePathWriterDelegate() override; + explicit FilePathWriterDelegate(base::FilePath output_file_path); - // WriterDelegate methods: + FilePathWriterDelegate(const FilePathWriterDelegate&) = delete; + FilePathWriterDelegate& operator=(const FilePathWriterDelegate&) = delete; - // Creates the output file and any necessary intermediate directories. - bool PrepareOutput() override; + ~FilePathWriterDelegate() override; - // Writes |num_bytes| bytes of |data| to the file, returning false if not all - // bytes could be written. - bool WriteBytes(const char* data, int num_bytes) override; + // Creates the output file and any necessary intermediate directories. Does + // not overwrite any existing file, and returns false if the output file + // cannot be created because another file conflicts with it. + bool PrepareOutput() override; - // Sets the last-modified time of the data. - void SetTimeModified(const base::Time& time) override; + // Deletes the output file. + void OnError() override; private: - base::FilePath output_file_path_; - base::File file_; - - DISALLOW_COPY_AND_ASSIGN(FilePathWriterDelegate); + const base::FilePath output_file_path_; }; } // namespace zip diff --git a/google/zip_reader_unittest.cc b/google/zip_reader_unittest.cc index 44134f8..fc80637 100644 --- a/google/zip_reader_unittest.cc +++ b/google/zip_reader_unittest.cc @@ -8,30 +8,36 @@ #include <stdint.h> #include <string.h> -#include <set> +#include <iterator> #include <string> +#include <vector> #include "base/bind.h" #include "base/check.h" #include "base/files/file.h" +#include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" #include "base/hash/md5.h" #include "base/path_service.h" #include "base/run_loop.h" -#include "base/stl_util.h" #include "base/strings/string_piece.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" +#include "base/test/bind.h" #include "base/test/task_environment.h" #include "base/time/time.h" +#include "build/build_config.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" #include "third_party/zlib/google/zip_internal.h" -using ::testing::Return; using ::testing::_; +using ::testing::ElementsAre; +using ::testing::ElementsAreArray; +using ::testing::Return; +using ::testing::SizeIs; namespace { @@ -39,10 +45,7 @@ const static std::string kQuuxExpectedMD5 = "d1ae4ac8a17a0e09317113ab284b57a6"; class FileWrapper { public: - typedef enum { - READ_ONLY, - READ_WRITE - } AccessMode; + typedef enum { READ_ONLY, READ_WRITE } AccessMode; FileWrapper(const base::FilePath& path, AccessMode mode) { int flags = base::File::FLAG_READ; @@ -73,18 +76,13 @@ class MockUnzipListener : public base::SupportsWeakPtr<MockUnzipListener> { : success_calls_(0), failure_calls_(0), progress_calls_(0), - current_progress_(0) { - } + current_progress_(0) {} // Success callback for async functions. - void OnUnzipSuccess() { - success_calls_++; - } + void OnUnzipSuccess() { success_calls_++; } // Failure callback for async functions. - void OnUnzipFailure() { - failure_calls_++; - } + void OnUnzipFailure() { failure_calls_++; } // Progress callback for async functions. void OnUnzipProgress(int64_t progress) { @@ -111,184 +109,189 @@ class MockWriterDelegate : public zip::WriterDelegate { MOCK_METHOD0(PrepareOutput, bool()); MOCK_METHOD2(WriteBytes, bool(const char*, int)); MOCK_METHOD1(SetTimeModified, void(const base::Time&)); + MOCK_METHOD1(SetPosixFilePermissions, void(int)); + MOCK_METHOD0(OnError, void()); }; bool ExtractCurrentEntryToFilePath(zip::ZipReader* reader, base::FilePath path) { zip::FilePathWriterDelegate writer(path); - return reader->ExtractCurrentEntry(&writer, - std::numeric_limits<uint64_t>::max()); + return reader->ExtractCurrentEntry(&writer); } -bool LocateAndOpenEntry(zip::ZipReader* reader, - const base::FilePath& path_in_zip) { +const zip::ZipReader::Entry* LocateAndOpenEntry( + zip::ZipReader* const reader, + const base::FilePath& path_in_zip) { + DCHECK(reader); + EXPECT_TRUE(reader->ok()); + // The underlying library can do O(1) access, but ZipReader does not expose // that. O(N) access is acceptable for these tests. - while (reader->HasMore()) { - if (!reader->OpenCurrentEntryInZip()) - return false; - if (reader->current_entry_info()->file_path() == path_in_zip) - return true; - reader->AdvanceToNextEntry(); + while (const zip::ZipReader::Entry* const entry = reader->Next()) { + EXPECT_TRUE(reader->ok()); + if (entry->path == path_in_zip) + return entry; } - return false; + + EXPECT_TRUE(reader->ok()); + return nullptr; } -} // namespace +using Paths = std::vector<base::FilePath>; + +} // namespace namespace zip { // Make the test a PlatformTest to setup autorelease pools properly on Mac. class ZipReaderTest : public PlatformTest { protected: - virtual void SetUp() { + void SetUp() override { PlatformTest::SetUp(); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); test_dir_ = temp_dir_.GetPath(); - - ASSERT_TRUE(GetTestDataDirectory(&test_data_dir_)); - - test_zip_file_ = test_data_dir_.AppendASCII("test.zip"); - encrypted_zip_file_ = test_data_dir_.AppendASCII("test_encrypted.zip"); - evil_zip_file_ = test_data_dir_.AppendASCII("evil.zip"); - evil_via_invalid_utf8_zip_file_ = test_data_dir_.AppendASCII( - "evil_via_invalid_utf8.zip"); - evil_via_absolute_file_name_zip_file_ = test_data_dir_.AppendASCII( - "evil_via_absolute_file_name.zip"); - - test_zip_contents_.insert(base::FilePath(FILE_PATH_LITERAL("foo/"))); - test_zip_contents_.insert(base::FilePath(FILE_PATH_LITERAL("foo/bar/"))); - test_zip_contents_.insert( - base::FilePath(FILE_PATH_LITERAL("foo/bar/baz.txt"))); - test_zip_contents_.insert( - base::FilePath(FILE_PATH_LITERAL("foo/bar/quux.txt"))); - test_zip_contents_.insert( - base::FilePath(FILE_PATH_LITERAL("foo/bar.txt"))); - test_zip_contents_.insert(base::FilePath(FILE_PATH_LITERAL("foo.txt"))); - test_zip_contents_.insert( - base::FilePath(FILE_PATH_LITERAL("foo/bar/.hidden"))); } - virtual void TearDown() { - PlatformTest::TearDown(); + static base::FilePath GetTestDataDirectory() { + base::FilePath path; + CHECK(base::PathService::Get(base::DIR_SOURCE_ROOT, &path)); + return path.AppendASCII("third_party") + .AppendASCII("zlib") + .AppendASCII("google") + .AppendASCII("test") + .AppendASCII("data"); } - bool GetTestDataDirectory(base::FilePath* path) { - bool success = base::PathService::Get(base::DIR_SOURCE_ROOT, path); - EXPECT_TRUE(success); - if (!success) - return false; - *path = path->AppendASCII("third_party"); - *path = path->AppendASCII("zlib"); - *path = path->AppendASCII("google"); - *path = path->AppendASCII("test"); - *path = path->AppendASCII("data"); - return true; - } + static Paths GetPaths(const base::FilePath& zip_path, + base::StringPiece encoding = {}) { + Paths paths; + + if (ZipReader reader; reader.Open(zip_path)) { + if (!encoding.empty()) + reader.SetEncoding(std::string(encoding)); + + while (const ZipReader::Entry* const entry = reader.Next()) { + EXPECT_TRUE(reader.ok()); + paths.push_back(entry->path); + } + + EXPECT_TRUE(reader.ok()); + } - bool CompareFileAndMD5(const base::FilePath& path, - const std::string expected_md5) { - // Read the output file and compute the MD5. - std::string output; - if (!base::ReadFileToString(path, &output)) - return false; - const std::string md5 = base::MD5String(output); - return expected_md5 == md5; + return paths; } // The path to temporary directory used to contain the test operations. base::FilePath test_dir_; // The path to the test data directory where test.zip etc. are located. - base::FilePath test_data_dir_; + const base::FilePath data_dir_ = GetTestDataDirectory(); // The path to test.zip in the test data directory. - base::FilePath test_zip_file_; - // The path to test_encrypted.zip in the test data directory. - base::FilePath encrypted_zip_file_; - // The path to evil.zip in the test data directory. - base::FilePath evil_zip_file_; - // The path to evil_via_invalid_utf8.zip in the test data directory. - base::FilePath evil_via_invalid_utf8_zip_file_; - // The path to evil_via_absolute_file_name.zip in the test data directory. - base::FilePath evil_via_absolute_file_name_zip_file_; - std::set<base::FilePath> test_zip_contents_; - + const base::FilePath test_zip_file_ = data_dir_.AppendASCII("test.zip"); + const Paths test_zip_contents_ = { + base::FilePath(FILE_PATH_LITERAL("foo/")), + base::FilePath(FILE_PATH_LITERAL("foo/bar/")), + base::FilePath(FILE_PATH_LITERAL("foo/bar/baz.txt")), + base::FilePath(FILE_PATH_LITERAL("foo/bar/quux.txt")), + base::FilePath(FILE_PATH_LITERAL("foo/bar.txt")), + base::FilePath(FILE_PATH_LITERAL("foo.txt")), + base::FilePath(FILE_PATH_LITERAL("foo/bar/.hidden")), + }; base::ScopedTempDir temp_dir_; - base::test::TaskEnvironment task_environment_; }; TEST_F(ZipReaderTest, Open_ValidZipFile) { ZipReader reader; - ASSERT_TRUE(reader.Open(test_zip_file_)); + EXPECT_TRUE(reader.Open(test_zip_file_)); + EXPECT_TRUE(reader.ok()); } TEST_F(ZipReaderTest, Open_ValidZipPlatformFile) { ZipReader reader; + EXPECT_FALSE(reader.ok()); FileWrapper zip_fd_wrapper(test_zip_file_, FileWrapper::READ_ONLY); - ASSERT_TRUE(reader.OpenFromPlatformFile(zip_fd_wrapper.platform_file())); + EXPECT_TRUE(reader.OpenFromPlatformFile(zip_fd_wrapper.platform_file())); + EXPECT_TRUE(reader.ok()); } TEST_F(ZipReaderTest, Open_NonExistentFile) { ZipReader reader; - ASSERT_FALSE(reader.Open(test_data_dir_.AppendASCII("nonexistent.zip"))); + EXPECT_FALSE(reader.ok()); + EXPECT_FALSE(reader.Open(data_dir_.AppendASCII("nonexistent.zip"))); + EXPECT_FALSE(reader.ok()); } TEST_F(ZipReaderTest, Open_ExistentButNonZipFile) { ZipReader reader; - ASSERT_FALSE(reader.Open(test_data_dir_.AppendASCII("create_test_zip.sh"))); + EXPECT_FALSE(reader.ok()); + EXPECT_FALSE(reader.Open(data_dir_.AppendASCII("create_test_zip.sh"))); + EXPECT_FALSE(reader.ok()); } -// Iterate through the contents in the test zip file, and compare that the -// contents collected from the zip reader matches the expected contents. +TEST_F(ZipReaderTest, Open_EmptyFile) { + ZipReader reader; + EXPECT_FALSE(reader.ok()); + EXPECT_FALSE(reader.Open(data_dir_.AppendASCII("empty.zip"))); + EXPECT_FALSE(reader.ok()); +} + +// Iterate through the contents in the test ZIP archive, and compare that the +// contents collected from the ZipReader matches the expected contents. TEST_F(ZipReaderTest, Iteration) { - std::set<base::FilePath> actual_contents; + Paths actual_contents; ZipReader reader; - ASSERT_TRUE(reader.Open(test_zip_file_)); - while (reader.HasMore()) { - ASSERT_TRUE(reader.OpenCurrentEntryInZip()); - actual_contents.insert(reader.current_entry_info()->file_path()); - ASSERT_TRUE(reader.AdvanceToNextEntry()); + EXPECT_FALSE(reader.ok()); + EXPECT_TRUE(reader.Open(test_zip_file_)); + EXPECT_TRUE(reader.ok()); + while (const ZipReader::Entry* const entry = reader.Next()) { + EXPECT_TRUE(reader.ok()); + actual_contents.push_back(entry->path); } - EXPECT_FALSE(reader.AdvanceToNextEntry()); // Shouldn't go further. - EXPECT_EQ(test_zip_contents_.size(), - static_cast<size_t>(reader.num_entries())); - EXPECT_EQ(test_zip_contents_.size(), actual_contents.size()); - EXPECT_EQ(test_zip_contents_, actual_contents); + + EXPECT_TRUE(reader.ok()); + EXPECT_FALSE(reader.Next()); // Shouldn't go further. + EXPECT_TRUE(reader.ok()); + + EXPECT_THAT(actual_contents, SizeIs(reader.num_entries())); + EXPECT_THAT(actual_contents, ElementsAreArray(test_zip_contents_)); } -// Open the test zip file from a file descriptor, iterate through its contents, -// and compare that they match the expected contents. +// Open the test ZIP archive from a file descriptor, iterate through its +// contents, and compare that they match the expected contents. TEST_F(ZipReaderTest, PlatformFileIteration) { - std::set<base::FilePath> actual_contents; + Paths actual_contents; ZipReader reader; FileWrapper zip_fd_wrapper(test_zip_file_, FileWrapper::READ_ONLY); - ASSERT_TRUE(reader.OpenFromPlatformFile(zip_fd_wrapper.platform_file())); - while (reader.HasMore()) { - ASSERT_TRUE(reader.OpenCurrentEntryInZip()); - actual_contents.insert(reader.current_entry_info()->file_path()); - ASSERT_TRUE(reader.AdvanceToNextEntry()); + EXPECT_TRUE(reader.OpenFromPlatformFile(zip_fd_wrapper.platform_file())); + EXPECT_TRUE(reader.ok()); + while (const ZipReader::Entry* const entry = reader.Next()) { + EXPECT_TRUE(reader.ok()); + actual_contents.push_back(entry->path); } - EXPECT_FALSE(reader.AdvanceToNextEntry()); // Shouldn't go further. - EXPECT_EQ(test_zip_contents_.size(), - static_cast<size_t>(reader.num_entries())); - EXPECT_EQ(test_zip_contents_.size(), actual_contents.size()); - EXPECT_EQ(test_zip_contents_, actual_contents); + + EXPECT_TRUE(reader.ok()); + EXPECT_FALSE(reader.Next()); // Shouldn't go further. + EXPECT_TRUE(reader.ok()); + + EXPECT_THAT(actual_contents, SizeIs(reader.num_entries())); + EXPECT_THAT(actual_contents, ElementsAreArray(test_zip_contents_)); } -TEST_F(ZipReaderTest, current_entry_info_RegularFile) { +TEST_F(ZipReaderTest, RegularFile) { ZipReader reader; ASSERT_TRUE(reader.Open(test_zip_file_)); base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt")); - ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path)); - ZipReader::EntryInfo* current_entry_info = reader.current_entry_info(); - EXPECT_EQ(target_path, current_entry_info->file_path()); - EXPECT_EQ(13527, current_entry_info->original_size()); + const ZipReader::Entry* entry = LocateAndOpenEntry(&reader, target_path); + ASSERT_TRUE(entry); + + EXPECT_EQ(target_path, entry->path); + EXPECT_EQ(13527, entry->original_size); // The expected time stamp: 2009-05-29 06:22:20 base::Time::Exploded exploded = {}; // Zero-clear. - current_entry_info->last_modified().LocalExplode(&exploded); + entry->last_modified.UTCExplode(&exploded); EXPECT_EQ(2009, exploded.year); EXPECT_EQ(5, exploded.month); EXPECT_EQ(29, exploded.day_of_month); @@ -297,67 +300,108 @@ TEST_F(ZipReaderTest, current_entry_info_RegularFile) { EXPECT_EQ(20, exploded.second); EXPECT_EQ(0, exploded.millisecond); - EXPECT_FALSE(current_entry_info->is_unsafe()); - EXPECT_FALSE(current_entry_info->is_directory()); + EXPECT_FALSE(entry->is_unsafe); + EXPECT_FALSE(entry->is_directory); } -TEST_F(ZipReaderTest, current_entry_info_DotDotFile) { +TEST_F(ZipReaderTest, DotDotFile) { ZipReader reader; - ASSERT_TRUE(reader.Open(evil_zip_file_)); + ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("evil.zip"))); base::FilePath target_path(FILE_PATH_LITERAL( "../levilevilevilevilevilevilevilevilevilevilevilevil")); - ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path)); - ZipReader::EntryInfo* current_entry_info = reader.current_entry_info(); - EXPECT_EQ(target_path, current_entry_info->file_path()); - + const ZipReader::Entry* entry = LocateAndOpenEntry(&reader, target_path); + ASSERT_TRUE(entry); + EXPECT_EQ(target_path, entry->path); // This file is unsafe because of ".." in the file name. - EXPECT_TRUE(current_entry_info->is_unsafe()); - EXPECT_FALSE(current_entry_info->is_directory()); + EXPECT_TRUE(entry->is_unsafe); + EXPECT_FALSE(entry->is_directory); } -TEST_F(ZipReaderTest, current_entry_info_InvalidUTF8File) { +TEST_F(ZipReaderTest, InvalidUTF8File) { ZipReader reader; - ASSERT_TRUE(reader.Open(evil_via_invalid_utf8_zip_file_)); - // The evil file is the 2nd file in the zip file. - // We cannot locate by the file name ".\x80.\\evil.txt", - // as FilePath may internally convert the string. - ASSERT_TRUE(reader.AdvanceToNextEntry()); - ASSERT_TRUE(reader.OpenCurrentEntryInZip()); - ZipReader::EntryInfo* current_entry_info = reader.current_entry_info(); + ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("evil_via_invalid_utf8.zip"))); + base::FilePath target_path = base::FilePath::FromUTF8Unsafe(".�.\\evil.txt"); + const ZipReader::Entry* entry = LocateAndOpenEntry(&reader, target_path); + ASSERT_TRUE(entry); + EXPECT_EQ(target_path, entry->path); + EXPECT_FALSE(entry->is_unsafe); + EXPECT_FALSE(entry->is_directory); +} + +// By default, file paths in ZIPs are interpreted as UTF-8. But in this test, +// the ZIP archive contains file paths that are actually encoded in Shift JIS. +// The SJIS-encoded paths are thus wrongly interpreted as UTF-8, resulting in +// garbled paths. Invalid UTF-8 sequences are safely converted to the +// replacement character �. +TEST_F(ZipReaderTest, EncodingSjisAsUtf8) { + EXPECT_THAT( + GetPaths(data_dir_.AppendASCII("SJIS Bug 846195.zip")), + ElementsAre( + base::FilePath::FromUTF8Unsafe("�V�����t�H���_/SJIS_835C_�\\.txt"), + base::FilePath::FromUTF8Unsafe( + "�V�����t�H���_/�V�����e�L�X�g �h�L�������g.txt"))); +} + +// In this test, SJIS-encoded paths are interpreted as Code Page 1252. This +// results in garbled paths. Note the presence of C1 control codes U+0090 and +// U+0081 in the garbled paths. +TEST_F(ZipReaderTest, EncodingSjisAs1252) { + EXPECT_THAT( + GetPaths(data_dir_.AppendASCII("SJIS Bug 846195.zip"), "windows-1252"), + ElementsAre(base::FilePath::FromUTF8Unsafe( + "\u0090V‚µ‚¢ƒtƒHƒ‹ƒ_/SJIS_835C_ƒ\\.txt"), + base::FilePath::FromUTF8Unsafe( + "\u0090V‚µ‚¢ƒtƒHƒ‹ƒ_/\u0090V‚µ‚¢ƒeƒLƒXƒg " + "ƒhƒLƒ…ƒ\u0081ƒ“ƒg.txt"))); +} + +// In this test, SJIS-encoded paths are interpreted as Code Page 866. This +// results in garbled paths. +TEST_F(ZipReaderTest, EncodingSjisAsIbm866) { + EXPECT_THAT( + GetPaths(data_dir_.AppendASCII("SJIS Bug 846195.zip"), "IBM866"), + ElementsAre( + base::FilePath::FromUTF8Unsafe("РVВ╡ВвГtГHГЛГ_/SJIS_835C_Г\\.txt"), + base::FilePath::FromUTF8Unsafe( + "РVВ╡ВвГtГHГЛГ_/РVВ╡ВвГeГLГXГg ГhГLГЕГБГУГg.txt"))); +} - // This file is unsafe because of invalid UTF-8 in the file name. - EXPECT_TRUE(current_entry_info->is_unsafe()); - EXPECT_FALSE(current_entry_info->is_directory()); +// Tests that SJIS-encoded paths are correctly converted to Unicode. +TEST_F(ZipReaderTest, EncodingSjis) { + EXPECT_THAT( + GetPaths(data_dir_.AppendASCII("SJIS Bug 846195.zip"), "Shift_JIS"), + ElementsAre( + base::FilePath::FromUTF8Unsafe("新しいフォルダ/SJIS_835C_ソ.txt"), + base::FilePath::FromUTF8Unsafe( + "新しいフォルダ/新しいテキスト ドキュメント.txt"))); } -TEST_F(ZipReaderTest, current_entry_info_AbsoluteFile) { +TEST_F(ZipReaderTest, AbsoluteFile) { ZipReader reader; - ASSERT_TRUE(reader.Open(evil_via_absolute_file_name_zip_file_)); + ASSERT_TRUE( + reader.Open(data_dir_.AppendASCII("evil_via_absolute_file_name.zip"))); base::FilePath target_path(FILE_PATH_LITERAL("/evil.txt")); - ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path)); - ZipReader::EntryInfo* current_entry_info = reader.current_entry_info(); - EXPECT_EQ(target_path, current_entry_info->file_path()); - + const ZipReader::Entry* entry = LocateAndOpenEntry(&reader, target_path); + ASSERT_TRUE(entry); + EXPECT_EQ(target_path, entry->path); // This file is unsafe because of the absolute file name. - EXPECT_TRUE(current_entry_info->is_unsafe()); - EXPECT_FALSE(current_entry_info->is_directory()); + EXPECT_TRUE(entry->is_unsafe); + EXPECT_FALSE(entry->is_directory); } -TEST_F(ZipReaderTest, current_entry_info_Directory) { +TEST_F(ZipReaderTest, Directory) { ZipReader reader; ASSERT_TRUE(reader.Open(test_zip_file_)); base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/")); - ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path)); - ZipReader::EntryInfo* current_entry_info = reader.current_entry_info(); - - EXPECT_EQ(base::FilePath(FILE_PATH_LITERAL("foo/bar/")), - current_entry_info->file_path()); + const ZipReader::Entry* entry = LocateAndOpenEntry(&reader, target_path); + ASSERT_TRUE(entry); + EXPECT_EQ(target_path, entry->path); // The directory size should be zero. - EXPECT_EQ(0, current_entry_info->original_size()); + EXPECT_EQ(0, entry->original_size); // The expected time stamp: 2009-05-31 15:49:52 base::Time::Exploded exploded = {}; // Zero-clear. - current_entry_info->last_modified().LocalExplode(&exploded); + entry->last_modified.UTCExplode(&exploded); EXPECT_EQ(2009, exploded.year); EXPECT_EQ(5, exploded.month); EXPECT_EQ(31, exploded.day_of_month); @@ -366,22 +410,91 @@ TEST_F(ZipReaderTest, current_entry_info_Directory) { EXPECT_EQ(52, exploded.second); EXPECT_EQ(0, exploded.millisecond); - EXPECT_FALSE(current_entry_info->is_unsafe()); - EXPECT_TRUE(current_entry_info->is_directory()); + EXPECT_FALSE(entry->is_unsafe); + EXPECT_TRUE(entry->is_directory); } -TEST_F(ZipReaderTest, current_entry_info_EncryptedFile) { +TEST_F(ZipReaderTest, EncryptedFile_WrongPassword) { ZipReader reader; - base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt")); + ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("Different Encryptions.zip"))); + reader.SetPassword("wrong password"); - ASSERT_TRUE(reader.Open(encrypted_zip_file_)); - ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path)); - EXPECT_TRUE(reader.current_entry_info()->is_encrypted()); - reader.Close(); + { + const ZipReader::Entry* entry = reader.Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(base::FilePath::FromASCII("ClearText.txt"), entry->path); + EXPECT_FALSE(entry->is_directory); + EXPECT_FALSE(entry->is_encrypted); + std::string contents = "dummy"; + EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents)); + EXPECT_EQ("This is not encrypted.\n", contents); + } - ASSERT_TRUE(reader.Open(test_zip_file_)); - ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path)); - EXPECT_FALSE(reader.current_entry_info()->is_encrypted()); + for (const base::StringPiece path : { + "Encrypted AES-128.txt", + "Encrypted AES-192.txt", + "Encrypted AES-256.txt", + "Encrypted ZipCrypto.txt", + }) { + const ZipReader::Entry* entry = reader.Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(base::FilePath::FromASCII(path), entry->path); + EXPECT_FALSE(entry->is_directory); + EXPECT_TRUE(entry->is_encrypted); + std::string contents = "dummy"; + EXPECT_FALSE(reader.ExtractCurrentEntryToString(&contents)); + } + + EXPECT_FALSE(reader.Next()); + EXPECT_TRUE(reader.ok()); +} + +TEST_F(ZipReaderTest, EncryptedFile_RightPassword) { + ZipReader reader; + ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("Different Encryptions.zip"))); + reader.SetPassword("password"); + + { + const ZipReader::Entry* entry = reader.Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(base::FilePath::FromASCII("ClearText.txt"), entry->path); + EXPECT_FALSE(entry->is_directory); + EXPECT_FALSE(entry->is_encrypted); + std::string contents = "dummy"; + EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents)); + EXPECT_EQ("This is not encrypted.\n", contents); + } + + // TODO(crbug.com/1296838) Support AES encryption. + for (const base::StringPiece path : { + "Encrypted AES-128.txt", + "Encrypted AES-192.txt", + "Encrypted AES-256.txt", + }) { + const ZipReader::Entry* entry = reader.Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(base::FilePath::FromASCII(path), entry->path); + EXPECT_FALSE(entry->is_directory); + EXPECT_TRUE(entry->is_encrypted); + std::string contents = "dummy"; + EXPECT_FALSE(reader.ExtractCurrentEntryToString(&contents)); + EXPECT_EQ("", contents); + } + + { + const ZipReader::Entry* entry = reader.Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(base::FilePath::FromASCII("Encrypted ZipCrypto.txt"), + entry->path); + EXPECT_FALSE(entry->is_directory); + EXPECT_TRUE(entry->is_encrypted); + std::string contents = "dummy"; + EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents)); + EXPECT_EQ("This is encrypted with ZipCrypto.\n", contents); + } + + EXPECT_FALSE(reader.Next()); + EXPECT_TRUE(reader.ok()); } // Verifies that the ZipReader class can extract a file from a zip archive @@ -404,7 +517,7 @@ TEST_F(ZipReaderTest, OpenFromString) { "\x50\x75\x78\x0b\x00\x01\x04\x8e\xf0\x00\x00\x04\x88\x13\x00\x00" "\x50\x4b\x05\x06\x00\x00\x00\x00\x01\x00\x01\x00\x4e\x00\x00\x00" "\x52\x00\x00\x00\x00\x00"; - std::string data(kTestData, base::size(kTestData)); + std::string data(kTestData, std::size(kTestData)); ZipReader reader; ASSERT_TRUE(reader.OpenFromString(data)); base::FilePath target_path(FILE_PATH_LITERAL("test.txt")); @@ -413,8 +526,8 @@ TEST_F(ZipReaderTest, OpenFromString) { test_dir_.AppendASCII("test.txt"))); std::string actual; - ASSERT_TRUE(base::ReadFileToString( - test_dir_.AppendASCII("test.txt"), &actual)); + ASSERT_TRUE( + base::ReadFileToString(test_dir_.AppendASCII("test.txt"), &actual)); EXPECT_EQ(std::string("This is a test.\n"), actual); } @@ -445,8 +558,8 @@ TEST_F(ZipReaderTest, ExtractToFileAsync_RegularFile) { EXPECT_LE(1, listener.progress_calls()); std::string output; - ASSERT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("quux.txt"), - &output)); + ASSERT_TRUE( + base::ReadFileToString(test_dir_.AppendASCII("quux.txt"), &output)); const std::string md5 = base::MD5String(output); EXPECT_EQ(kQuuxExpectedMD5, md5); @@ -456,6 +569,103 @@ TEST_F(ZipReaderTest, ExtractToFileAsync_RegularFile) { EXPECT_EQ(file_size, listener.current_progress()); } +TEST_F(ZipReaderTest, ExtractToFileAsync_Encrypted_NoPassword) { + MockUnzipListener listener; + + ZipReader reader; + ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("Different Encryptions.zip"))); + ASSERT_TRUE(LocateAndOpenEntry( + &reader, base::FilePath::FromASCII("Encrypted ZipCrypto.txt"))); + const base::FilePath target_path = test_dir_.AppendASCII("extracted"); + reader.ExtractCurrentEntryToFilePathAsync( + target_path, + base::BindOnce(&MockUnzipListener::OnUnzipSuccess, listener.AsWeakPtr()), + base::BindOnce(&MockUnzipListener::OnUnzipFailure, listener.AsWeakPtr()), + base::BindRepeating(&MockUnzipListener::OnUnzipProgress, + listener.AsWeakPtr())); + + EXPECT_EQ(0, listener.success_calls()); + EXPECT_EQ(0, listener.failure_calls()); + EXPECT_EQ(0, listener.progress_calls()); + + base::RunLoop().RunUntilIdle(); + + EXPECT_EQ(0, listener.success_calls()); + EXPECT_EQ(1, listener.failure_calls()); + EXPECT_LE(1, listener.progress_calls()); + + // The extracted file contains rubbish data. + // We probably shouldn't even look at it. + std::string contents; + ASSERT_TRUE(base::ReadFileToString(target_path, &contents)); + EXPECT_NE("", contents); + EXPECT_EQ(contents.size(), listener.current_progress()); +} + +TEST_F(ZipReaderTest, ExtractToFileAsync_Encrypted_RightPassword) { + MockUnzipListener listener; + + ZipReader reader; + reader.SetPassword("password"); + ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("Different Encryptions.zip"))); + ASSERT_TRUE(LocateAndOpenEntry( + &reader, base::FilePath::FromASCII("Encrypted ZipCrypto.txt"))); + const base::FilePath target_path = test_dir_.AppendASCII("extracted"); + reader.ExtractCurrentEntryToFilePathAsync( + target_path, + base::BindOnce(&MockUnzipListener::OnUnzipSuccess, listener.AsWeakPtr()), + base::BindOnce(&MockUnzipListener::OnUnzipFailure, listener.AsWeakPtr()), + base::BindRepeating(&MockUnzipListener::OnUnzipProgress, + listener.AsWeakPtr())); + + EXPECT_EQ(0, listener.success_calls()); + EXPECT_EQ(0, listener.failure_calls()); + EXPECT_EQ(0, listener.progress_calls()); + + base::RunLoop().RunUntilIdle(); + + EXPECT_EQ(1, listener.success_calls()); + EXPECT_EQ(0, listener.failure_calls()); + EXPECT_LE(1, listener.progress_calls()); + + std::string contents; + ASSERT_TRUE(base::ReadFileToString(target_path, &contents)); + EXPECT_EQ("This is encrypted with ZipCrypto.\n", contents); + EXPECT_EQ(contents.size(), listener.current_progress()); +} + +TEST_F(ZipReaderTest, ExtractToFileAsync_WrongCrc) { + MockUnzipListener listener; + + ZipReader reader; + ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("Wrong CRC.zip"))); + ASSERT_TRUE( + LocateAndOpenEntry(&reader, base::FilePath::FromASCII("Corrupted.txt"))); + const base::FilePath target_path = test_dir_.AppendASCII("extracted"); + reader.ExtractCurrentEntryToFilePathAsync( + target_path, + base::BindOnce(&MockUnzipListener::OnUnzipSuccess, listener.AsWeakPtr()), + base::BindOnce(&MockUnzipListener::OnUnzipFailure, listener.AsWeakPtr()), + base::BindRepeating(&MockUnzipListener::OnUnzipProgress, + listener.AsWeakPtr())); + + EXPECT_EQ(0, listener.success_calls()); + EXPECT_EQ(0, listener.failure_calls()); + EXPECT_EQ(0, listener.progress_calls()); + + base::RunLoop().RunUntilIdle(); + + EXPECT_EQ(0, listener.success_calls()); + EXPECT_EQ(1, listener.failure_calls()); + EXPECT_LE(1, listener.progress_calls()); + + std::string contents; + ASSERT_TRUE(base::ReadFileToString(target_path, &contents)); + EXPECT_EQ("This file has been changed after its CRC was computed.\n", + contents); + EXPECT_EQ(contents.size(), listener.current_progress()); +} + // Verifies that the asynchronous extraction to a file works. TEST_F(ZipReaderTest, ExtractToFileAsync_Directory) { MockUnzipListener listener; @@ -490,7 +700,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntryToString) { // sizes from 0 to 7 bytes respectively, being the contents of each file a // substring of "0123456" starting at '0'. base::FilePath test_zip_file = - test_data_dir_.AppendASCII("test_mismatch_size.zip"); + data_dir_.AppendASCII("test_mismatch_size.zip"); ZipReader reader; std::string contents; @@ -515,7 +725,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntryToString) { } // More than necessary byte read limit: must pass. - EXPECT_TRUE(reader.ExtractCurrentEntryToString(16, &contents)); + EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents)); EXPECT_EQ(std::string(base::StringPiece("0123456", i)), contents); } reader.Close(); @@ -526,7 +736,7 @@ TEST_F(ZipReaderTest, ExtractPartOfCurrentEntry) { // sizes from 0 to 7 bytes respectively, being the contents of each file a // substring of "0123456" starting at '0'. base::FilePath test_zip_file = - test_data_dir_.AppendASCII("test_mismatch_size.zip"); + data_dir_.AppendASCII("test_mismatch_size.zip"); ZipReader reader; std::string contents; @@ -564,6 +774,37 @@ TEST_F(ZipReaderTest, ExtractPartOfCurrentEntry) { reader.Close(); } +TEST_F(ZipReaderTest, ExtractPosixPermissions) { + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + + ZipReader reader; + ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("test_posix_permissions.zip"))); + for (auto entry : {"0.txt", "1.txt", "2.txt", "3.txt"}) { + ASSERT_TRUE(LocateAndOpenEntry(&reader, base::FilePath::FromASCII(entry))); + FilePathWriterDelegate delegate(temp_dir.GetPath().AppendASCII(entry)); + ASSERT_TRUE(reader.ExtractCurrentEntry(&delegate)); + } + reader.Close(); + +#if defined(OS_POSIX) + // This assumes a umask of at least 0400. + int mode = 0; + EXPECT_TRUE(base::GetPosixFilePermissions( + temp_dir.GetPath().AppendASCII("0.txt"), &mode)); + EXPECT_EQ(mode & 0700, 0700); + EXPECT_TRUE(base::GetPosixFilePermissions( + temp_dir.GetPath().AppendASCII("1.txt"), &mode)); + EXPECT_EQ(mode & 0700, 0600); + EXPECT_TRUE(base::GetPosixFilePermissions( + temp_dir.GetPath().AppendASCII("2.txt"), &mode)); + EXPECT_EQ(mode & 0700, 0700); + EXPECT_TRUE(base::GetPosixFilePermissions( + temp_dir.GetPath().AppendASCII("3.txt"), &mode)); + EXPECT_EQ(mode & 0700, 0600); +#endif +} + // This test exposes http://crbug.com/430959, at least on OS X TEST_F(ZipReaderTest, DISABLED_LeakDetectionTest) { for (int i = 0; i < 100000; ++i) { @@ -578,45 +819,40 @@ TEST_F(ZipReaderTest, DISABLED_LeakDetectionTest) { TEST_F(ZipReaderTest, ExtractCurrentEntryPrepareFailure) { testing::StrictMock<MockWriterDelegate> mock_writer; - EXPECT_CALL(mock_writer, PrepareOutput()) - .WillOnce(Return(false)); + EXPECT_CALL(mock_writer, PrepareOutput()).WillOnce(Return(false)); base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt")); ZipReader reader; ASSERT_TRUE(reader.Open(test_zip_file_)); ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path)); - ASSERT_FALSE(reader.ExtractCurrentEntry( - &mock_writer, std::numeric_limits<uint64_t>::max())); + ASSERT_FALSE(reader.ExtractCurrentEntry(&mock_writer)); } -// Test that when WriterDelegate::WriteBytes returns false, no other methods on -// the delegate are called and the extraction fails. +// Test that when WriterDelegate::WriteBytes returns false, only the OnError +// method on the delegate is called and the extraction fails. TEST_F(ZipReaderTest, ExtractCurrentEntryWriteBytesFailure) { testing::StrictMock<MockWriterDelegate> mock_writer; - EXPECT_CALL(mock_writer, PrepareOutput()) - .WillOnce(Return(true)); - EXPECT_CALL(mock_writer, WriteBytes(_, _)) - .WillOnce(Return(false)); + EXPECT_CALL(mock_writer, PrepareOutput()).WillOnce(Return(true)); + EXPECT_CALL(mock_writer, WriteBytes(_, _)).WillOnce(Return(false)); + EXPECT_CALL(mock_writer, OnError()); base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt")); ZipReader reader; ASSERT_TRUE(reader.Open(test_zip_file_)); ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path)); - ASSERT_FALSE(reader.ExtractCurrentEntry( - &mock_writer, std::numeric_limits<uint64_t>::max())); + ASSERT_FALSE(reader.ExtractCurrentEntry(&mock_writer)); } // Test that extraction succeeds when the writer delegate reports all is well. TEST_F(ZipReaderTest, ExtractCurrentEntrySuccess) { testing::StrictMock<MockWriterDelegate> mock_writer; - EXPECT_CALL(mock_writer, PrepareOutput()) - .WillOnce(Return(true)); - EXPECT_CALL(mock_writer, WriteBytes(_, _)) - .WillRepeatedly(Return(true)); + EXPECT_CALL(mock_writer, PrepareOutput()).WillOnce(Return(true)); + EXPECT_CALL(mock_writer, WriteBytes(_, _)).WillRepeatedly(Return(true)); + EXPECT_CALL(mock_writer, SetPosixFilePermissions(_)); EXPECT_CALL(mock_writer, SetTimeModified(_)); base::FilePath target_path(FILE_PATH_LITERAL("foo/bar/quux.txt")); @@ -624,50 +860,84 @@ TEST_F(ZipReaderTest, ExtractCurrentEntrySuccess) { ASSERT_TRUE(reader.Open(test_zip_file_)); ASSERT_TRUE(LocateAndOpenEntry(&reader, target_path)); - ASSERT_TRUE(reader.ExtractCurrentEntry(&mock_writer, - std::numeric_limits<uint64_t>::max())); + ASSERT_TRUE(reader.ExtractCurrentEntry(&mock_writer)); +} + +TEST_F(ZipReaderTest, WrongCrc) { + ZipReader reader; + ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("Wrong CRC.zip"))); + + const ZipReader::Entry* const entry = + LocateAndOpenEntry(&reader, base::FilePath::FromASCII("Corrupted.txt")); + ASSERT_TRUE(entry); + + std::string contents = "dummy"; + EXPECT_FALSE(reader.ExtractCurrentEntryToString(&contents)); + EXPECT_EQ("This file has been changed after its CRC was computed.\n", + contents); + + contents = "dummy"; + EXPECT_FALSE( + reader.ExtractCurrentEntryToString(entry->original_size + 1, &contents)); + EXPECT_EQ("This file has been changed after its CRC was computed.\n", + contents); + + contents = "dummy"; + EXPECT_FALSE( + reader.ExtractCurrentEntryToString(entry->original_size, &contents)); + EXPECT_EQ("This file has been changed after its CRC was computed.\n", + contents); + + contents = "dummy"; + EXPECT_FALSE( + reader.ExtractCurrentEntryToString(entry->original_size - 1, &contents)); + EXPECT_EQ("This file has been changed after its CRC was computed.", contents); } class FileWriterDelegateTest : public ::testing::Test { protected: void SetUp() override { ASSERT_TRUE(base::CreateTemporaryFile(&temp_file_path_)); - file_.Initialize(temp_file_path_, (base::File::FLAG_CREATE_ALWAYS | - base::File::FLAG_READ | - base::File::FLAG_WRITE | - base::File::FLAG_TEMPORARY | - base::File::FLAG_DELETE_ON_CLOSE)); + file_.Initialize(temp_file_path_, + (base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_READ | + base::File::FLAG_WRITE | base::File::FLAG_WIN_TEMPORARY | + base::File::FLAG_DELETE_ON_CLOSE)); ASSERT_TRUE(file_.IsValid()); } - // Writes data to the file, leaving the current position at the end of the - // write. - void PopulateFile() { - static const char kSomeData[] = "this sure is some data."; - static const size_t kSomeDataLen = sizeof(kSomeData) - 1; - ASSERT_NE(-1LL, file_.Write(0LL, kSomeData, kSomeDataLen)); - } - base::FilePath temp_file_path_; base::File file_; }; -TEST_F(FileWriterDelegateTest, WriteToStartAndTruncate) { - // Write stuff and advance. - PopulateFile(); +TEST_F(FileWriterDelegateTest, WriteToEnd) { + const std::string payload = "This is the actualy payload data.\n"; - // This should rewind, write, then truncate. - static const char kSomeData[] = "short"; - static const int kSomeDataLen = sizeof(kSomeData) - 1; { FileWriterDelegate writer(&file_); + EXPECT_EQ(0, writer.file_length()); ASSERT_TRUE(writer.PrepareOutput()); - ASSERT_TRUE(writer.WriteBytes(kSomeData, kSomeDataLen)); + ASSERT_TRUE(writer.WriteBytes(payload.data(), payload.size())); + EXPECT_EQ(payload.size(), writer.file_length()); } - ASSERT_EQ(kSomeDataLen, file_.GetLength()); - char buf[kSomeDataLen] = {}; - ASSERT_EQ(kSomeDataLen, file_.Read(0LL, buf, kSomeDataLen)); - ASSERT_EQ(std::string(kSomeData), std::string(buf, kSomeDataLen)); + + EXPECT_EQ(payload.size(), file_.GetLength()); +} + +TEST_F(FileWriterDelegateTest, EmptyOnError) { + const std::string payload = "This is the actualy payload data.\n"; + + { + FileWriterDelegate writer(&file_); + EXPECT_EQ(0, writer.file_length()); + ASSERT_TRUE(writer.PrepareOutput()); + ASSERT_TRUE(writer.WriteBytes(payload.data(), payload.size())); + EXPECT_EQ(payload.size(), writer.file_length()); + EXPECT_EQ(payload.size(), file_.GetLength()); + writer.OnError(); + EXPECT_EQ(0, writer.file_length()); + } + + EXPECT_EQ(0, file_.GetLength()); } } // namespace zip diff --git a/google/zip_unittest.cc b/google/zip_unittest.cc index 10f2ef7..8fbec32 100644 --- a/google/zip_unittest.cc +++ b/google/zip_unittest.cc @@ -5,9 +5,11 @@ #include <stddef.h> #include <stdint.h> -#include <map> -#include <set> +#include <iomanip> +#include <limits> #include <string> +#include <unordered_map> +#include <unordered_set> #include <vector> #include "base/bind.h" @@ -17,18 +19,40 @@ #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" #include "base/logging.h" -#include "base/macros.h" #include "base/path_service.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" +#include "base/test/bind.h" +#include "base/time/time.h" #include "build/build_config.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" #include "third_party/zlib/google/zip.h" +#include "third_party/zlib/google/zip_internal.h" #include "third_party/zlib/google/zip_reader.h" +// Convenience macro to create a file path from a string literal. +#define FP(path) base::FilePath(FILE_PATH_LITERAL(path)) + namespace { +using testing::UnorderedElementsAre; + +std::vector<std::string> GetRelativePaths(const base::FilePath& dir, + base::FileEnumerator::FileType type) { + std::vector<std::string> got_paths; + base::FileEnumerator files(dir, true, type); + for (base::FilePath path = files.Next(); !path.empty(); path = files.Next()) { + base::FilePath relative; + EXPECT_TRUE(dir.AppendRelativePath(path, &relative)); + got_paths.push_back(relative.AsUTF8Unsafe()); + } + + EXPECT_EQ(base::File::FILE_OK, files.GetError()); + return got_paths; +} + bool CreateFile(const std::string& content, base::FilePath* file_path, base::File* file) { @@ -43,6 +67,46 @@ bool CreateFile(const std::string& content, return file->IsValid(); } +// A WriterDelegate that logs progress once per second. +class ProgressWriterDelegate : public zip::WriterDelegate { + public: + explicit ProgressWriterDelegate(int64_t expected_size) + : expected_size_(expected_size) { + CHECK_GT(expected_size_, 0); + } + + bool WriteBytes(const char* data, int num_bytes) override { + received_bytes_ += num_bytes; + LogProgressIfNecessary(); + return true; + } + + void SetTimeModified(const base::Time& time) override { LogProgress(); } + + int64_t received_bytes() const { return received_bytes_; } + + private: + void LogProgressIfNecessary() { + const base::TimeTicks now = base::TimeTicks::Now(); + if (next_progress_report_time_ > now) + return; + + next_progress_report_time_ = now + progress_period_; + LogProgress(); + } + + void LogProgress() const { + LOG(INFO) << "Unzipping... " << std::setw(3) + << (100 * received_bytes_ / expected_size_) << "%"; + } + + const base::TimeDelta progress_period_ = base::Seconds(1); + base::TimeTicks next_progress_report_time_ = + base::TimeTicks::Now() + progress_period_; + const uint64_t expected_size_; + int64_t received_bytes_ = 0; +}; + // A virtual file system containing: // /test // /test/foo.txt @@ -56,8 +120,8 @@ class VirtualFileSystem : public zip::FileAccessor { static constexpr char kBar2Content[] = "This is bar too."; VirtualFileSystem() { - base::FilePath test_dir(FILE_PATH_LITERAL("/test")); - base::FilePath foo_txt_path = test_dir.Append(FILE_PATH_LITERAL("foo.txt")); + base::FilePath test_dir; + base::FilePath foo_txt_path = test_dir.AppendASCII("foo.txt"); base::FilePath file_path; base::File file; @@ -65,62 +129,92 @@ class VirtualFileSystem : public zip::FileAccessor { DCHECK(success); files_[foo_txt_path] = std::move(file); - base::FilePath bar_dir = test_dir.Append(FILE_PATH_LITERAL("bar")); - base::FilePath bar1_txt_path = - bar_dir.Append(FILE_PATH_LITERAL("bar1.txt")); + base::FilePath bar_dir = test_dir.AppendASCII("bar"); + base::FilePath bar1_txt_path = bar_dir.AppendASCII("bar1.txt"); success = CreateFile(kBar1Content, &file_path, &file); DCHECK(success); files_[bar1_txt_path] = std::move(file); - base::FilePath bar2_txt_path = - bar_dir.Append(FILE_PATH_LITERAL("bar2.txt")); + base::FilePath bar2_txt_path = bar_dir.AppendASCII("bar2.txt"); success = CreateFile(kBar2Content, &file_path, &file); DCHECK(success); files_[bar2_txt_path] = std::move(file); - file_tree_[test_dir] = std::vector<DirectoryContentEntry>{ - DirectoryContentEntry(foo_txt_path, /*is_dir=*/false), - DirectoryContentEntry(bar_dir, /*is_dir=*/true)}; - file_tree_[bar_dir] = std::vector<DirectoryContentEntry>{ - DirectoryContentEntry(bar1_txt_path, /*is_dir=*/false), - DirectoryContentEntry(bar2_txt_path, /*is_dir=*/false)}; + file_tree_[base::FilePath()] = {{foo_txt_path}, {bar_dir}}; + file_tree_[bar_dir] = {{bar1_txt_path, bar2_txt_path}}; + file_tree_[foo_txt_path] = {}; + file_tree_[bar1_txt_path] = {}; + file_tree_[bar2_txt_path] = {}; } + + VirtualFileSystem(const VirtualFileSystem&) = delete; + VirtualFileSystem& operator=(const VirtualFileSystem&) = delete; + ~VirtualFileSystem() override = default; private: - std::vector<base::File> OpenFilesForReading( - const std::vector<base::FilePath>& paths) override { - std::vector<base::File> files; - for (const auto& path : paths) { - auto iter = files_.find(path); - files.push_back(iter == files_.end() ? base::File() - : std::move(iter->second)); + bool Open(const zip::Paths paths, + std::vector<base::File>* const files) override { + DCHECK(files); + files->reserve(files->size() + paths.size()); + + for (const base::FilePath& path : paths) { + const auto it = files_.find(path); + if (it == files_.end()) { + files->emplace_back(); + } else { + EXPECT_TRUE(it->second.IsValid()); + files->push_back(std::move(it->second)); + } } - return files; - } - bool DirectoryExists(const base::FilePath& file) override { - return file_tree_.count(file) == 1 && files_.count(file) == 0; + return true; } - std::vector<DirectoryContentEntry> ListDirectoryContent( - const base::FilePath& dir) override { - auto iter = file_tree_.find(dir); - if (iter == file_tree_.end()) { - NOTREACHED(); - return std::vector<DirectoryContentEntry>(); + bool List(const base::FilePath& path, + std::vector<base::FilePath>* const files, + std::vector<base::FilePath>* const subdirs) override { + DCHECK(!path.IsAbsolute()); + DCHECK(files); + DCHECK(subdirs); + + const auto it = file_tree_.find(path); + if (it == file_tree_.end()) + return false; + + for (const base::FilePath& file : it->second.files) { + DCHECK(!file.empty()); + files->push_back(file); + } + + for (const base::FilePath& subdir : it->second.subdirs) { + DCHECK(!subdir.empty()); + subdirs->push_back(subdir); } - return iter->second; + + return true; } - base::Time GetLastModifiedTime(const base::FilePath& path) override { - return base::Time::FromDoubleT(172097977); // Some random date. + bool GetInfo(const base::FilePath& path, Info* const info) override { + DCHECK(!path.IsAbsolute()); + DCHECK(info); + + if (!file_tree_.count(path)) + return false; + + info->is_directory = !files_.count(path); + info->last_modified = + base::Time::FromDoubleT(172097977); // Some random date. + + return true; } - std::map<base::FilePath, std::vector<DirectoryContentEntry>> file_tree_; - std::map<base::FilePath, base::File> files_; + struct DirContents { + std::vector<base::FilePath> files, subdirs; + }; - DISALLOW_COPY_AND_ASSIGN(VirtualFileSystem); + std::unordered_map<base::FilePath, DirContents> file_tree_; + std::unordered_map<base::FilePath, base::File> files_; }; // static @@ -131,10 +225,7 @@ constexpr char VirtualFileSystem::kBar2Content[]; // Make the test a PlatformTest to setup autorelease pools properly on Mac. class ZipTest : public PlatformTest { protected: - enum ValidYearType { - VALID_YEAR, - INVALID_YEAR - }; + enum ValidYearType { VALID_YEAR, INVALID_YEAR }; virtual void SetUp() { PlatformTest::SetUp(); @@ -143,94 +234,77 @@ class ZipTest : public PlatformTest { test_dir_ = temp_dir_.GetPath(); base::FilePath zip_path(test_dir_); - zip_contents_.insert(zip_path.Append(FILE_PATH_LITERAL("foo.txt"))); - zip_path = zip_path.Append(FILE_PATH_LITERAL("foo")); + zip_contents_.insert(zip_path.AppendASCII("foo.txt")); + zip_path = zip_path.AppendASCII("foo"); zip_contents_.insert(zip_path); - zip_contents_.insert(zip_path.Append(FILE_PATH_LITERAL("bar.txt"))); - zip_path = zip_path.Append(FILE_PATH_LITERAL("bar")); + zip_contents_.insert(zip_path.AppendASCII("bar.txt")); + zip_path = zip_path.AppendASCII("bar"); zip_contents_.insert(zip_path); - zip_contents_.insert(zip_path.Append(FILE_PATH_LITERAL("baz.txt"))); - zip_contents_.insert(zip_path.Append(FILE_PATH_LITERAL("quux.txt"))); - zip_contents_.insert(zip_path.Append(FILE_PATH_LITERAL(".hidden"))); + zip_contents_.insert(zip_path.AppendASCII("baz.txt")); + zip_contents_.insert(zip_path.AppendASCII("quux.txt")); + zip_contents_.insert(zip_path.AppendASCII(".hidden")); // Include a subset of files in |zip_file_list_| to test ZipFiles(). - zip_file_list_.push_back(base::FilePath(FILE_PATH_LITERAL("foo.txt"))); - zip_file_list_.push_back( - base::FilePath(FILE_PATH_LITERAL("foo/bar/quux.txt"))); - zip_file_list_.push_back( - base::FilePath(FILE_PATH_LITERAL("foo/bar/.hidden"))); + zip_file_list_.push_back(FP("foo.txt")); + zip_file_list_.push_back(FP("foo/bar/quux.txt")); + zip_file_list_.push_back(FP("foo/bar/.hidden")); } - virtual void TearDown() { - PlatformTest::TearDown(); - } + virtual void TearDown() { PlatformTest::TearDown(); } - bool GetTestDataDirectory(base::FilePath* path) { - bool success = base::PathService::Get(base::DIR_SOURCE_ROOT, path); + static base::FilePath GetDataDirectory() { + base::FilePath path; + bool success = base::PathService::Get(base::DIR_SOURCE_ROOT, &path); EXPECT_TRUE(success); - if (!success) - return false; - *path = path->AppendASCII("third_party"); - *path = path->AppendASCII("zlib"); - *path = path->AppendASCII("google"); - *path = path->AppendASCII("test"); - *path = path->AppendASCII("data"); - return true; + return std::move(path) + .AppendASCII("third_party") + .AppendASCII("zlib") + .AppendASCII("google") + .AppendASCII("test") + .AppendASCII("data"); } void TestUnzipFile(const base::FilePath::StringType& filename, bool expect_hidden_files) { - base::FilePath test_dir; - ASSERT_TRUE(GetTestDataDirectory(&test_dir)); - TestUnzipFile(test_dir.Append(filename), expect_hidden_files); + TestUnzipFile(GetDataDirectory().Append(filename), expect_hidden_files); } void TestUnzipFile(const base::FilePath& path, bool expect_hidden_files) { - ASSERT_TRUE(base::PathExists(path)) << "no file " << path.value(); + ASSERT_TRUE(base::PathExists(path)) << "no file " << path; ASSERT_TRUE(zip::Unzip(path, test_dir_)); - base::FilePath original_dir; - ASSERT_TRUE(GetTestDataDirectory(&original_dir)); - original_dir = original_dir.AppendASCII("test"); + base::FilePath original_dir = GetDataDirectory().AppendASCII("test"); - base::FileEnumerator files(test_dir_, true, + base::FileEnumerator files( + test_dir_, true, base::FileEnumerator::FILES | base::FileEnumerator::DIRECTORIES); - base::FilePath unzipped_entry_path = files.Next(); + size_t count = 0; - while (!unzipped_entry_path.value().empty()) { + for (base::FilePath unzipped_entry_path = files.Next(); + !unzipped_entry_path.empty(); unzipped_entry_path = files.Next()) { EXPECT_EQ(zip_contents_.count(unzipped_entry_path), 1U) - << "Couldn't find " << unzipped_entry_path.value(); + << "Couldn't find " << unzipped_entry_path; count++; if (base::PathExists(unzipped_entry_path) && !base::DirectoryExists(unzipped_entry_path)) { // It's a file, check its contents are what we zipped. - // TODO(774156): figure out why the commented out EXPECT_TRUE below - // fails on the build bots (but not on the try-bots). base::FilePath relative_path; - bool append_relative_path_success = - test_dir_.AppendRelativePath(unzipped_entry_path, &relative_path); - if (!append_relative_path_success) { - LOG(ERROR) << "Append relative path failed, params: " - << test_dir_.value() << " and " - << unzipped_entry_path.value(); - } + ASSERT_TRUE( + test_dir_.AppendRelativePath(unzipped_entry_path, &relative_path)) + << "Cannot append relative path failed, params: '" << test_dir_ + << "' and '" << unzipped_entry_path << "'"; base::FilePath original_path = original_dir.Append(relative_path); - LOG(ERROR) << "Comparing original " << original_path.value() - << " and unzipped file " << unzipped_entry_path.value() - << " result: " - << base::ContentsEqual(original_path, unzipped_entry_path); - // EXPECT_TRUE(base::ContentsEqual(original_path, unzipped_entry_path)) - // << "Contents differ between original " << original_path.value() - // << " and unzipped file " << unzipped_entry_path.value(); + EXPECT_TRUE(base::ContentsEqual(original_path, unzipped_entry_path)) + << "Original file '" << original_path << "' and unzipped file '" + << unzipped_entry_path << "' have different contents"; } - unzipped_entry_path = files.Next(); } + EXPECT_EQ(base::File::FILE_OK, files.GetError()); size_t expected_count = 0; - for (std::set<base::FilePath>::iterator iter = zip_contents_.begin(); - iter != zip_contents_.end(); ++iter) { - if (expect_hidden_files || iter->BaseName().value()[0] != '.') + for (const base::FilePath& path : zip_contents_) { + if (expect_hidden_files || path.BaseName().value()[0] != '.') ++expected_count; } @@ -266,11 +340,11 @@ class ZipTest : public PlatformTest { // supports, which is 2 seconds. Note that between this call to Time::Now() // and zip::Zip() the clock can advance a bit, hence the use of EXPECT_GE. base::Time::Exploded now_parts; - base::Time::Now().LocalExplode(&now_parts); + base::Time::Now().UTCExplode(&now_parts); now_parts.second = now_parts.second & ~1; now_parts.millisecond = 0; base::Time now_time; - EXPECT_TRUE(base::Time::FromLocalExploded(now_parts, &now_time)); + EXPECT_TRUE(base::Time::FromUTCExploded(now_parts, &now_time)); EXPECT_EQ(1, base::WriteFile(src_file, "1", 1)); EXPECT_TRUE(base::TouchFile(src_file, base::Time::Now(), test_mtime)); @@ -296,12 +370,23 @@ class ZipTest : public PlatformTest { base::ScopedTempDir temp_dir_; // Hard-coded contents of a known zip file. - std::set<base::FilePath> zip_contents_; + std::unordered_set<base::FilePath> zip_contents_; // Hard-coded list of relative paths for a zip file created with ZipFiles. std::vector<base::FilePath> zip_file_list_; }; +TEST_F(ZipTest, UnzipNoSuchFile) { + EXPECT_FALSE(zip::Unzip(GetDataDirectory().AppendASCII("No Such File.zip"), + test_dir_)); + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre()); + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::DIRECTORIES), + UnorderedElementsAre()); +} + TEST_F(ZipTest, Unzip) { TestUnzipFile(FILE_PATH_LITERAL("test.zip"), true); } @@ -311,9 +396,7 @@ TEST_F(ZipTest, UnzipUncompressed) { } TEST_F(ZipTest, UnzipEvil) { - base::FilePath path; - ASSERT_TRUE(GetTestDataDirectory(&path)); - path = path.AppendASCII("evil.zip"); + base::FilePath path = GetDataDirectory().AppendASCII("evil.zip"); // 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. @@ -326,93 +409,509 @@ TEST_F(ZipTest, UnzipEvil) { } TEST_F(ZipTest, UnzipEvil2) { - base::FilePath path; - ASSERT_TRUE(GetTestDataDirectory(&path)); - // The zip file contains an evil file with invalid UTF-8 in its file - // name. - path = path.AppendASCII("evil_via_invalid_utf8.zip"); + // The ZIP file contains a file with invalid UTF-8 in its file name. + base::FilePath path = + GetDataDirectory().AppendASCII("evil_via_invalid_utf8.zip"); // 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(path, output_dir)); - base::FilePath evil_file = output_dir; - evil_file = evil_file.AppendASCII("../evil.txt"); - ASSERT_FALSE(base::PathExists(evil_file)); + ASSERT_TRUE(zip::Unzip(path, output_dir)); + ASSERT_TRUE(base::PathExists( + output_dir.Append(base::FilePath::FromUTF8Unsafe(".�.\\evil.txt")))); + ASSERT_FALSE(base::PathExists(output_dir.AppendASCII("../evil.txt"))); } TEST_F(ZipTest, UnzipWithFilter) { auto filter = base::BindRepeating([](const base::FilePath& path) { return path.BaseName().MaybeAsASCII() == "foo.txt"; }); - base::FilePath path; - ASSERT_TRUE(GetTestDataDirectory(&path)); - ASSERT_TRUE(zip::UnzipWithFilterCallback(path.AppendASCII("test.zip"), - 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); + ASSERT_TRUE(zip::Unzip(GetDataDirectory().AppendASCII("test.zip"), test_dir_, + {.filter = std::move(filter)})); + // Only foo.txt should have been extracted. + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("foo.txt")); + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::DIRECTORIES), + UnorderedElementsAre()); +} + +TEST_F(ZipTest, UnzipEncryptedWithRightPassword) { + // TODO(crbug.com/1296838) Also check the AES-encrypted files. + auto filter = base::BindRepeating([](const base::FilePath& path) { + return !base::StartsWith(path.MaybeAsASCII(), "Encrypted AES"); + }); + + ASSERT_TRUE(zip::Unzip( + GetDataDirectory().AppendASCII("Different Encryptions.zip"), test_dir_, + {.filter = std::move(filter), .password = "password"})); + + std::string contents; + ASSERT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("ClearText.txt"), + &contents)); + EXPECT_EQ("This is not encrypted.\n", contents); + + ASSERT_TRUE(base::ReadFileToString( + test_dir_.AppendASCII("Encrypted ZipCrypto.txt"), &contents)); + EXPECT_EQ("This is encrypted with ZipCrypto.\n", contents); +} + +TEST_F(ZipTest, UnzipEncryptedWithWrongPassword) { + // TODO(crbug.com/1296838) Also check the AES-encrypted files. + auto filter = base::BindRepeating([](const base::FilePath& path) { + return !base::StartsWith(path.MaybeAsASCII(), "Encrypted AES"); + }); + + ASSERT_FALSE(zip::Unzip( + GetDataDirectory().AppendASCII("Different Encryptions.zip"), test_dir_, + {.filter = std::move(filter), .password = "wrong"})); + + std::string contents; + ASSERT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("ClearText.txt"), + &contents)); + EXPECT_EQ("This is not encrypted.\n", contents); + + // No rubbish file should be left behind. + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("ClearText.txt")); +} + +TEST_F(ZipTest, UnzipEncryptedWithNoPassword) { + // TODO(crbug.com/1296838) Also check the AES-encrypted files. + auto filter = base::BindRepeating([](const base::FilePath& path) { + return !base::StartsWith(path.MaybeAsASCII(), "Encrypted AES"); + }); + + ASSERT_FALSE( + zip::Unzip(GetDataDirectory().AppendASCII("Different Encryptions.zip"), + test_dir_, {.filter = std::move(filter)})); + + std::string contents; + ASSERT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("ClearText.txt"), + &contents)); + EXPECT_EQ("This is not encrypted.\n", contents); + + // No rubbish file should be left behind. + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("ClearText.txt")); +} + +TEST_F(ZipTest, UnzipEncryptedContinueOnError) { + EXPECT_TRUE( + zip::Unzip(GetDataDirectory().AppendASCII("Different Encryptions.zip"), + test_dir_, {.continue_on_error = true})); + + std::string contents; + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("ClearText.txt"), + &contents)); + EXPECT_EQ("This is not encrypted.\n", contents); + + // No rubbish file should be left behind. + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("ClearText.txt")); +} + +TEST_F(ZipTest, UnzipWrongCrc) { + ASSERT_FALSE( + zip::Unzip(GetDataDirectory().AppendASCII("Wrong CRC.zip"), test_dir_)); + + // No rubbish file should be left behind. + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre()); +} + +TEST_F(ZipTest, UnzipRepeatedDirName) { + EXPECT_TRUE(zip::Unzip( + GetDataDirectory().AppendASCII("Repeated Dir Name.zip"), test_dir_)); + + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre()); + + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::DIRECTORIES), + UnorderedElementsAre("repeated")); +} + +TEST_F(ZipTest, UnzipRepeatedFileName) { + EXPECT_FALSE(zip::Unzip( + GetDataDirectory().AppendASCII("Repeated File Name.zip"), test_dir_)); + + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("repeated")); + + std::string contents; + EXPECT_TRUE( + base::ReadFileToString(test_dir_.AppendASCII("repeated"), &contents)); + EXPECT_EQ("First file", contents); +} + +TEST_F(ZipTest, UnzipCannotCreateEmptyDir) { + EXPECT_FALSE(zip::Unzip( + GetDataDirectory().AppendASCII("Empty Dir Same Name As File.zip"), + test_dir_)); + + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("repeated")); + + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::DIRECTORIES), + UnorderedElementsAre()); + + std::string contents; + EXPECT_TRUE( + base::ReadFileToString(test_dir_.AppendASCII("repeated"), &contents)); + EXPECT_EQ("First file", contents); +} + +TEST_F(ZipTest, UnzipCannotCreateParentDir) { + EXPECT_FALSE(zip::Unzip( + GetDataDirectory().AppendASCII("Parent Dir Same Name As File.zip"), + test_dir_)); + + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("repeated")); + + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::DIRECTORIES), + UnorderedElementsAre()); + + std::string contents; + EXPECT_TRUE( + base::ReadFileToString(test_dir_.AppendASCII("repeated"), &contents)); + EXPECT_EQ("First file", contents); +} + +TEST_F(ZipTest, UnzipWindowsSpecialNames) { + EXPECT_TRUE(zip::Unzip( + GetDataDirectory().AppendASCII("Windows Special Names.zip"), test_dir_)); + + std::string contents; + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("Last"), &contents)); + EXPECT_EQ("Last file", contents); + +#ifdef OS_WIN + // On Windows, the NUL* files are simply missing. No error is reported. Not + // even an error message in the logs. + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("First", "Last")); +#else + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("First", "Last", "NUL", "Nul.txt", + "nul.very long extension")); + + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("NUL"), &contents)); + EXPECT_EQ("This is: NUL", contents); + + EXPECT_TRUE( + base::ReadFileToString(test_dir_.AppendASCII("Nul.txt"), &contents)); + EXPECT_EQ("This is: Nul.txt", contents); + + EXPECT_TRUE(base::ReadFileToString( + test_dir_.AppendASCII("nul.very long extension"), &contents)); + EXPECT_EQ("This is: nul.very long extension", contents); +#endif +} + +TEST_F(ZipTest, UnzipDifferentCases) { +#if defined(OS_WIN) || defined(OS_MAC) + // Only the first file (with mixed case) is extracted. + EXPECT_FALSE(zip::Unzip(GetDataDirectory().AppendASCII( + "Repeated File Name With Different Cases.zip"), + test_dir_)); + + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("Case")); + + std::string contents; + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("Case"), &contents)); + EXPECT_EQ("Mixed case 111", contents); +#else + // All the files are extracted. + EXPECT_TRUE(zip::Unzip(GetDataDirectory().AppendASCII( + "Repeated File Name With Different Cases.zip"), + test_dir_)); + + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("Case", "case", "CASE")); + + std::string contents; + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("Case"), &contents)); + EXPECT_EQ("Mixed case 111", contents); + + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("case"), &contents)); + EXPECT_EQ("Lower case 22", contents); + + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("CASE"), &contents)); + EXPECT_EQ("Upper case 3", contents); +#endif +} + +TEST_F(ZipTest, UnzipDifferentCasesContinueOnError) { + EXPECT_TRUE(zip::Unzip(GetDataDirectory().AppendASCII( + "Repeated File Name With Different Cases.zip"), + test_dir_, {.continue_on_error = true})); + + std::string contents; + +#if defined(OS_WIN) || defined(OS_MAC) + // Only the first file (with mixed case) has been extracted. + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("Case")); + + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("Case"), &contents)); + EXPECT_EQ("Mixed case 111", contents); +#else + // All the files have been extracted. + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES), + UnorderedElementsAre("Case", "case", "CASE")); + + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("Case"), &contents)); + EXPECT_EQ("Mixed case 111", contents); + + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("case"), &contents)); + EXPECT_EQ("Lower case 22", contents); + + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("CASE"), &contents)); + EXPECT_EQ("Upper case 3", contents); +#endif +} + +TEST_F(ZipTest, UnzipMixedPaths) { + EXPECT_TRUE(zip::Unzip(GetDataDirectory().AppendASCII("Mixed Paths.zip"), + test_dir_, {.continue_on_error = true})); + + std::unordered_set<std::string> want_paths = { +#ifdef OS_WIN + "Dot", // + "Space→", // + "a\\b", // + "u\\v\\w\\x\\y\\z", // + "←Backslash2", // +#else + " ", // Invalid on Windows + "Angle <>", // Invalid on Windows + "Backslash1→\\", // + "Backspace \x08", // Invalid on Windows + "Bell \a", // Invalid on Windows + "C:", // Invalid on Windows + "C:\\", // Absolute path on Windows + "C:\\Temp", // Absolute path on Windows + "C:\\Temp\\", // Absolute path on Windows + "C:\\Temp\\File", // Absolute path on Windows + "Carriage Return \r", // Invalid on Windows + "Colon :", // Invalid on Windows + "Dot .", // Becomes "Dot" on Windows + "Double quote \"", // Invalid on Windows + "Escape \x1B", // Invalid on Windows + "Line Feed \n", // Invalid on Windows + "NUL .txt", // Disappears on Windows + "NUL", // Disappears on Windows + "NUL..txt", // Disappears on Windows + "NUL.tar.gz", // Disappears on Windows + "NUL.txt", // Disappears on Windows + "Pipe |", // Invalid on Windows + "Question ?", // Invalid on Windows + "Space→ ", // Becomes "Space→" on Windows + "Star *", // Invalid on Windows + "Tab \t", // Invalid on Windows + "\\\\server\\share\\file", // Absolute path on Windows + "\\←Backslash2", // Becomes "←Backslash2" on Windows + "a/b", // + "u/v/w/x/y/z", // +#ifndef OS_MAC + "CASE", // + "Case", // +#endif +#endif + " NUL.txt", // + " ←Space", // + "$HOME", // + "%TMP", // + "-", // + "...Tree", // + "..Two", // + ".One", // + "Ampersand &", // + "At @", // + "Backslash3→\\←Backslash4", // + "Backtick `", // + "Caret ^", // + "Comma ,", // + "Curly {}", // + "Dash -", // + "Delete \x7F", // + "Dollar $", // + "Equal =", // + "Euro €", // + "Exclamation !", // + "FileOrDir", // + "First", // + "Hash #", // + "Last", // + "Percent %", // + "Plus +", // + "Quote '", // + "Round ()", // + "Semicolon ;", // + "Smile \U0001F642", // + "Square []", // + "String Terminator \u009C", // + "Tilde ~", // + "Underscore _", // + "case", // + "~", // + }; + + const std::vector<std::string> got_paths = + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES); + + for (const std::string& path : got_paths) { + EXPECT_TRUE(want_paths.erase(path)) + << "Found unexpected file: " << std::quoted(path); + } + + for (const std::string& path : want_paths) { + EXPECT_TRUE(false) << "Cannot find expected file: " << std::quoted(path); + } + + EXPECT_THAT( + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::DIRECTORIES), + UnorderedElementsAre( +#ifdef OS_WIN + "Backslash3→", "Empty", "a", "u", "u\\v", "u\\v\\w", "u\\v\\w\\x", + "u\\v\\w\\x\\y" +#else + "Empty", "a", "u", "u/v", "u/v/w", "u/v/w/x", "u/v/w/x/y" +#endif + )); } TEST_F(ZipTest, UnzipWithDelegates) { - 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<zip::WriterDelegate> { + auto dir_creator = + base::BindLambdaForTesting([this](const base::FilePath& entry_path) { + return base::CreateDirectory(test_dir_.Append(entry_path)); + }); + auto writer = + base::BindLambdaForTesting([this](const base::FilePath& entry_path) + -> std::unique_ptr<zip::WriterDelegate> { return std::make_unique<zip::FilePathWriterDelegate>( - extract_dir.Append(entry_path)); - }, - test_dir_); - base::FilePath path; - ASSERT_TRUE(GetTestDataDirectory(&path)); - base::File file(path.AppendASCII("test.zip"), + test_dir_.Append(entry_path)); + }); + + base::File file(GetDataDirectory().AppendASCII("test.zip"), base::File::Flags::FLAG_OPEN | base::File::Flags::FLAG_READ); - ASSERT_TRUE(zip::UnzipWithFilterAndWriters(file.GetPlatformFile(), writer, - dir_creator, filter, false)); + EXPECT_TRUE(zip::Unzip(file.GetPlatformFile(), writer, dir_creator)); 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"))); + EXPECT_TRUE(base::PathExists(dir.AppendASCII("foo.txt"))); + EXPECT_TRUE(base::DirectoryExists(dir_foo)); + EXPECT_TRUE(base::PathExists(dir_foo.AppendASCII("bar.txt"))); + EXPECT_TRUE(base::DirectoryExists(dir_foo_bar)); + EXPECT_TRUE(base::PathExists(dir_foo_bar.AppendASCII(".hidden"))); + EXPECT_TRUE(base::PathExists(dir_foo_bar.AppendASCII("baz.txt"))); + EXPECT_TRUE(base::PathExists(dir_foo_bar.AppendASCII("quux.txt"))); +} + +TEST_F(ZipTest, UnzipOnlyDirectories) { + auto dir_creator = + base::BindLambdaForTesting([this](const base::FilePath& entry_path) { + return base::CreateDirectory(test_dir_.Append(entry_path)); + }); + + // Always return a null WriterDelegate. + auto writer = + base::BindLambdaForTesting([](const base::FilePath& entry_path) { + return std::unique_ptr<zip::WriterDelegate>(); + }); + + base::File file(GetDataDirectory().AppendASCII("test.zip"), + base::File::Flags::FLAG_OPEN | base::File::Flags::FLAG_READ); + EXPECT_TRUE(zip::Unzip(file.GetPlatformFile(), writer, dir_creator, + {.continue_on_error = true})); + base::FilePath dir = test_dir_; + base::FilePath dir_foo = dir.AppendASCII("foo"); + base::FilePath dir_foo_bar = dir_foo.AppendASCII("bar"); + EXPECT_FALSE(base::PathExists(dir.AppendASCII("foo.txt"))); + EXPECT_TRUE(base::DirectoryExists(dir_foo)); + EXPECT_FALSE(base::PathExists(dir_foo.AppendASCII("bar.txt"))); + EXPECT_TRUE(base::DirectoryExists(dir_foo_bar)); + EXPECT_FALSE(base::PathExists(dir_foo_bar.AppendASCII(".hidden"))); + EXPECT_FALSE(base::PathExists(dir_foo_bar.AppendASCII("baz.txt"))); + EXPECT_FALSE(base::PathExists(dir_foo_bar.AppendASCII("quux.txt"))); +} + +// Tests that a ZIP archive containing SJIS-encoded file names can be correctly +// extracted if the encoding is specified. +TEST_F(ZipTest, UnzipSjis) { + ASSERT_TRUE(zip::Unzip(GetDataDirectory().AppendASCII("SJIS Bug 846195.zip"), + test_dir_, {.encoding = "Shift_JIS"})); + + const base::FilePath dir = + test_dir_.Append(base::FilePath::FromUTF8Unsafe("新しいフォルダ")); + EXPECT_TRUE(base::DirectoryExists(dir)); + + std::string contents; + ASSERT_TRUE(base::ReadFileToString( + dir.Append(base::FilePath::FromUTF8Unsafe("SJIS_835C_ソ.txt")), + &contents)); + EXPECT_EQ( + "This file's name contains 0x5c (backslash) as the 2nd byte of Japanese " + "characater \"\x83\x5c\" when encoded in Shift JIS.", + contents); + + ASSERT_TRUE(base::ReadFileToString(dir.Append(base::FilePath::FromUTF8Unsafe( + "新しいテキスト ドキュメント.txt")), + &contents)); + EXPECT_EQ("This file name is coded in Shift JIS in the archive.", contents); +} + +// Tests that a ZIP archive containing SJIS-encoded file names can be extracted +// even if the encoding is not specified. In this case, file names are +// interpreted as UTF-8, which leads to garbled names where invalid UTF-8 +// sequences are replaced with the character �. Nevertheless, the files are +// safely extracted and readable. +TEST_F(ZipTest, UnzipSjisAsUtf8) { + ASSERT_TRUE(zip::Unzip(GetDataDirectory().AppendASCII("SJIS Bug 846195.zip"), + test_dir_)); + + EXPECT_FALSE(base::DirectoryExists( + test_dir_.Append(base::FilePath::FromUTF8Unsafe("新しいフォルダ")))); + + const base::FilePath dir = + test_dir_.Append(base::FilePath::FromUTF8Unsafe("�V�����t�H���_")); + EXPECT_TRUE(base::DirectoryExists(dir)); + + std::string contents; + ASSERT_TRUE(base::ReadFileToString( + dir.Append(base::FilePath::FromUTF8Unsafe("SJIS_835C_�\\.txt")), + &contents)); + EXPECT_EQ( + "This file's name contains 0x5c (backslash) as the 2nd byte of Japanese " + "characater \"\x83\x5c\" when encoded in Shift JIS.", + contents); + + ASSERT_TRUE(base::ReadFileToString(dir.Append(base::FilePath::FromUTF8Unsafe( + "�V�����e�L�X�g �h�L�������g.txt")), + &contents)); + EXPECT_EQ("This file name is coded in Shift JIS in the archive.", contents); } TEST_F(ZipTest, Zip) { - base::FilePath src_dir; - ASSERT_TRUE(GetTestDataDirectory(&src_dir)); - src_dir = src_dir.AppendASCII("test"); + base::FilePath src_dir = GetDataDirectory().AppendASCII("test"); base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); @@ -423,9 +922,7 @@ TEST_F(ZipTest, Zip) { } TEST_F(ZipTest, ZipIgnoreHidden) { - base::FilePath src_dir; - ASSERT_TRUE(GetTestDataDirectory(&src_dir)); - src_dir = src_dir.AppendASCII("test"); + base::FilePath src_dir = GetDataDirectory().AppendASCII("test"); base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); @@ -436,9 +933,7 @@ TEST_F(ZipTest, ZipIgnoreHidden) { } TEST_F(ZipTest, ZipNonASCIIDir) { - base::FilePath src_dir; - ASSERT_TRUE(GetTestDataDirectory(&src_dir)); - src_dir = src_dir.AppendASCII("test"); + base::FilePath src_dir = GetDataDirectory().AppendASCII("test"); base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); @@ -474,11 +969,9 @@ TEST_F(ZipTest, ZipTimeStamp) { TestTimeStamp("02 Jan 2038 23:59:58", VALID_YEAR); } -#if defined(OS_POSIX) +#if defined(OS_POSIX) || defined(OS_FUCHSIA) TEST_F(ZipTest, ZipFiles) { - base::FilePath src_dir; - ASSERT_TRUE(GetTestDataDirectory(&src_dir)); - src_dir = src_dir.AppendASCII("test"); + base::FilePath src_dir = GetDataDirectory().AppendASCII("test"); base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); @@ -487,33 +980,28 @@ TEST_F(ZipTest, ZipFiles) { base::File zip_file(zip_name, base::File::FLAG_CREATE | base::File::FLAG_WRITE); ASSERT_TRUE(zip_file.IsValid()); - EXPECT_TRUE(zip::ZipFiles(src_dir, zip_file_list_, - zip_file.GetPlatformFile())); + EXPECT_TRUE( + zip::ZipFiles(src_dir, zip_file_list_, zip_file.GetPlatformFile())); zip_file.Close(); zip::ZipReader reader; EXPECT_TRUE(reader.Open(zip_name)); EXPECT_EQ(zip_file_list_.size(), static_cast<size_t>(reader.num_entries())); for (size_t i = 0; i < zip_file_list_.size(); ++i) { - EXPECT_TRUE(reader.HasMore()); - EXPECT_TRUE(reader.OpenCurrentEntryInZip()); - const zip::ZipReader::EntryInfo* entry_info = reader.current_entry_info(); - EXPECT_EQ(entry_info->file_path(), zip_file_list_[i]); - reader.AdvanceToNextEntry(); + const zip::ZipReader::Entry* const entry = reader.Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(entry->path, zip_file_list_[i]); } } -#endif // defined(OS_POSIX) +#endif // defined(OS_POSIX) || defined(OS_FUCHSIA) TEST_F(ZipTest, UnzipFilesWithIncorrectSize) { - base::FilePath test_data_folder; - ASSERT_TRUE(GetTestDataDirectory(&test_data_folder)); - // test_mismatch_size.zip contains files with names from 0.txt to 7.txt with // sizes from 0 to 7 bytes respectively, but the metadata in the zip file says // the uncompressed size is 3 bytes. The ZipReader and minizip code needs to // be clever enough to get all the data out. base::FilePath test_zip_file = - test_data_folder.AppendASCII("test_mismatch_size.zip"); + GetDataDirectory().AppendASCII("test_mismatch_size.zip"); base::ScopedTempDir scoped_temp_dir; ASSERT_TRUE(scoped_temp_dir.CreateUniqueTempDir()); @@ -524,8 +1012,8 @@ TEST_F(ZipTest, UnzipFilesWithIncorrectSize) { for (int i = 0; i < 8; i++) { SCOPED_TRACE(base::StringPrintf("Processing %d.txt", i)); - base::FilePath file_path = temp_dir.AppendASCII( - base::StringPrintf("%d.txt", i)); + base::FilePath file_path = + temp_dir.AppendASCII(base::StringPrintf("%d.txt", i)); int64_t file_size = -1; EXPECT_TRUE(base::GetFileSize(file_path, &file_size)); EXPECT_EQ(static_cast<int64_t>(i), file_size); @@ -535,26 +1023,309 @@ TEST_F(ZipTest, UnzipFilesWithIncorrectSize) { TEST_F(ZipTest, ZipWithFileAccessor) { base::FilePath zip_file; ASSERT_TRUE(base::CreateTemporaryFile(&zip_file)); - zip::ZipParams params(base::FilePath(FILE_PATH_LITERAL("/test")), zip_file); - params.set_file_accessor(std::make_unique<VirtualFileSystem>()); + VirtualFileSystem file_accessor; + const zip::ZipParams params{.file_accessor = &file_accessor, + .dest_file = zip_file}; ASSERT_TRUE(zip::Zip(params)); base::ScopedTempDir scoped_temp_dir; ASSERT_TRUE(scoped_temp_dir.CreateUniqueTempDir()); const base::FilePath& temp_dir = scoped_temp_dir.GetPath(); ASSERT_TRUE(zip::Unzip(zip_file, temp_dir)); - base::FilePath bar_dir = temp_dir.Append(FILE_PATH_LITERAL("bar")); + base::FilePath bar_dir = temp_dir.AppendASCII("bar"); EXPECT_TRUE(base::DirectoryExists(bar_dir)); std::string file_content; - EXPECT_TRUE(base::ReadFileToString( - temp_dir.Append(FILE_PATH_LITERAL("foo.txt")), &file_content)); + EXPECT_TRUE( + base::ReadFileToString(temp_dir.AppendASCII("foo.txt"), &file_content)); EXPECT_EQ(VirtualFileSystem::kFooContent, file_content); - EXPECT_TRUE(base::ReadFileToString( - bar_dir.Append(FILE_PATH_LITERAL("bar1.txt")), &file_content)); + EXPECT_TRUE( + base::ReadFileToString(bar_dir.AppendASCII("bar1.txt"), &file_content)); EXPECT_EQ(VirtualFileSystem::kBar1Content, file_content); - EXPECT_TRUE(base::ReadFileToString( - bar_dir.Append(FILE_PATH_LITERAL("bar2.txt")), &file_content)); + EXPECT_TRUE( + base::ReadFileToString(bar_dir.AppendASCII("bar2.txt"), &file_content)); EXPECT_EQ(VirtualFileSystem::kBar2Content, file_content); } +// Tests progress reporting while zipping files. +TEST_F(ZipTest, ZipProgress) { + base::FilePath src_dir = GetDataDirectory().AppendASCII("test"); + + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + base::FilePath zip_file = temp_dir.GetPath().AppendASCII("out.zip"); + + int progress_count = 0; + zip::Progress last_progress; + + zip::ProgressCallback progress_callback = + base::BindLambdaForTesting([&](const zip::Progress& progress) { + progress_count++; + LOG(INFO) << "Progress #" << progress_count << ": " << progress; + + // Progress should only go forwards. + EXPECT_GE(progress.bytes, last_progress.bytes); + EXPECT_GE(progress.files, last_progress.files); + EXPECT_GE(progress.directories, last_progress.directories); + + last_progress = progress; + return true; + }); + + EXPECT_TRUE(zip::Zip({.src_dir = src_dir, + .dest_file = zip_file, + .progress_callback = std::move(progress_callback)})); + + EXPECT_EQ(progress_count, 14); + EXPECT_EQ(last_progress.bytes, 13546); + EXPECT_EQ(last_progress.files, 5); + EXPECT_EQ(last_progress.directories, 2); + + TestUnzipFile(zip_file, true); +} + +// Tests throttling of progress reporting while zipping files. +TEST_F(ZipTest, ZipProgressPeriod) { + base::FilePath src_dir = GetDataDirectory().AppendASCII("test"); + + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + base::FilePath zip_file = temp_dir.GetPath().AppendASCII("out.zip"); + + int progress_count = 0; + zip::Progress last_progress; + + zip::ProgressCallback progress_callback = + base::BindLambdaForTesting([&](const zip::Progress& progress) { + progress_count++; + LOG(INFO) << "Progress #" << progress_count << ": " << progress; + + // Progress should only go forwards. + EXPECT_GE(progress.bytes, last_progress.bytes); + EXPECT_GE(progress.files, last_progress.files); + EXPECT_GE(progress.directories, last_progress.directories); + + last_progress = progress; + return true; + }); + + EXPECT_TRUE(zip::Zip({.src_dir = src_dir, + .dest_file = zip_file, + .progress_callback = std::move(progress_callback), + .progress_period = base::Hours(1)})); + + // We expect only 2 progress reports: the first one, and the last one. + EXPECT_EQ(progress_count, 2); + EXPECT_EQ(last_progress.bytes, 13546); + EXPECT_EQ(last_progress.files, 5); + EXPECT_EQ(last_progress.directories, 2); + + TestUnzipFile(zip_file, true); +} + +// Tests cancellation while zipping files. +TEST_F(ZipTest, ZipCancel) { + base::FilePath src_dir = GetDataDirectory().AppendASCII("test"); + + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + base::FilePath zip_file = temp_dir.GetPath().AppendASCII("out.zip"); + + // First: establish the number of possible interruption points. + int progress_count = 0; + + EXPECT_TRUE(zip::Zip({.src_dir = src_dir, + .dest_file = zip_file, + .progress_callback = base::BindLambdaForTesting( + [&progress_count](const zip::Progress&) { + progress_count++; + return true; + })})); + + EXPECT_EQ(progress_count, 14); + + // Second: exercise each and every interruption point. + for (int i = progress_count; i > 0; i--) { + int j = 0; + EXPECT_FALSE(zip::Zip({.src_dir = src_dir, + .dest_file = zip_file, + .progress_callback = base::BindLambdaForTesting( + [i, &j](const zip::Progress&) { + j++; + // Callback shouldn't be called again after + // having returned false once. + EXPECT_LE(j, i); + return j < i; + })})); + + EXPECT_EQ(j, i); + } +} + +// Tests zip::internal::GetCompressionMethod() +TEST_F(ZipTest, GetCompressionMethod) { + using zip::internal::GetCompressionMethod; + using zip::internal::kDeflated; + using zip::internal::kStored; + + EXPECT_EQ(GetCompressionMethod(FP("")), kDeflated); + EXPECT_EQ(GetCompressionMethod(FP("NoExtension")), kDeflated); + EXPECT_EQ(GetCompressionMethod(FP("Folder.zip").Append(FP("NoExtension"))), + kDeflated); + EXPECT_EQ(GetCompressionMethod(FP("Name.txt")), kDeflated); + EXPECT_EQ(GetCompressionMethod(FP("Name.zip")), kStored); + EXPECT_EQ(GetCompressionMethod(FP("Name....zip")), kStored); + EXPECT_EQ(GetCompressionMethod(FP("Name.zip")), kStored); + EXPECT_EQ(GetCompressionMethod(FP("NAME.ZIP")), kStored); + EXPECT_EQ(GetCompressionMethod(FP("Name.gz")), kStored); + EXPECT_EQ(GetCompressionMethod(FP("Name.tar.gz")), kStored); + EXPECT_EQ(GetCompressionMethod(FP("Name.tar")), kDeflated); + + // This one is controversial. + EXPECT_EQ(GetCompressionMethod(FP(".zip")), kStored); +} + +// Tests that files put inside a ZIP are effectively compressed. +TEST_F(ZipTest, Compressed) { + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + + const base::FilePath src_dir = temp_dir.GetPath().AppendASCII("input"); + EXPECT_TRUE(base::CreateDirectory(src_dir)); + + // Create some dummy source files. + for (const base::StringPiece s : {"foo", "bar.txt", ".hidden"}) { + base::File f(src_dir.AppendASCII(s), + base::File::FLAG_CREATE | base::File::FLAG_WRITE); + ASSERT_TRUE(f.SetLength(5000)); + } + + // Zip the source files. + const base::FilePath dest_file = temp_dir.GetPath().AppendASCII("dest.zip"); + EXPECT_TRUE(zip::Zip({.src_dir = src_dir, + .dest_file = dest_file, + .include_hidden_files = true})); + + // Since the source files compress well, the destination ZIP file should be + // smaller than the source files. + int64_t dest_file_size; + ASSERT_TRUE(base::GetFileSize(dest_file, &dest_file_size)); + EXPECT_GT(dest_file_size, 300); + EXPECT_LT(dest_file_size, 1000); +} + +// Tests that a ZIP put inside a ZIP is simply stored instead of being +// compressed. +TEST_F(ZipTest, NestedZip) { + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + + const base::FilePath src_dir = temp_dir.GetPath().AppendASCII("input"); + EXPECT_TRUE(base::CreateDirectory(src_dir)); + + // Create a dummy ZIP file. This is not a valid ZIP file, but for the purpose + // of this test, it doesn't really matter. + const int64_t src_size = 5000; + + { + base::File f(src_dir.AppendASCII("src.zip"), + base::File::FLAG_CREATE | base::File::FLAG_WRITE); + ASSERT_TRUE(f.SetLength(src_size)); + } + + // Zip the dummy ZIP file. + const base::FilePath dest_file = temp_dir.GetPath().AppendASCII("dest.zip"); + EXPECT_TRUE(zip::Zip({.src_dir = src_dir, .dest_file = dest_file})); + + // Since the dummy source (inner) ZIP file should simply be stored in the + // destination (outer) ZIP file, the destination file should be bigger than + // the source file, but not much bigger. + int64_t dest_file_size; + ASSERT_TRUE(base::GetFileSize(dest_file, &dest_file_size)); + EXPECT_GT(dest_file_size, src_size + 100); + EXPECT_LT(dest_file_size, src_size + 300); +} + +// Tests that there is no 2GB or 4GB limits. Tests that big files can be zipped +// (crbug.com/1207737) and that big ZIP files can be created +// (crbug.com/1221447). Tests that the big ZIP can be opened, that its entries +// are correctly enumerated (crbug.com/1298347), and that the big file can be +// extracted. +// +// Because this test is dealing with big files, it tends to take a lot of disk +// space and time (crbug.com/1299736). Therefore, it only gets run on a few bots +// (ChromeOS and Windows). +// +// This test is too slow with TSAN. +// OS Fuchsia does not seem to support large files. +// Some 32-bit Android waterfall and CQ try bots are running out of space when +// performing this test (android-asan, android-11-x86-rel, +// android-marshmallow-x86-rel-non-cq). +// Some Mac, Linux and Debug (dbg) bots tend to time out when performing this +// test (crbug.com/1299736, crbug.com/1300448). +#if defined(THREAD_SANITIZER) || BUILDFLAG(IS_FUCHSIA) || \ + BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) || \ + BUILDFLAG(IS_CHROMEOS_LACROS) || !defined(NDEBUG) +TEST_F(ZipTest, DISABLED_BigFile) { +#else +TEST_F(ZipTest, BigFile) { +#endif + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + + const base::FilePath src_dir = temp_dir.GetPath().AppendASCII("input"); + EXPECT_TRUE(base::CreateDirectory(src_dir)); + + // Create a big dummy ZIP file. This is not a valid ZIP file, but for the + // purpose of this test, it doesn't really matter. + const int64_t src_size = 5'000'000'000; + + const base::FilePath src_file = src_dir.AppendASCII("src.zip"); + LOG(INFO) << "Creating big file " << src_file; + { + base::File f(src_file, base::File::FLAG_CREATE | base::File::FLAG_WRITE); + ASSERT_TRUE(f.SetLength(src_size)); + } + + // Zip the dummy ZIP file. + const base::FilePath dest_file = temp_dir.GetPath().AppendASCII("dest.zip"); + LOG(INFO) << "Zipping big file into " << dest_file; + zip::ProgressCallback progress_callback = + base::BindLambdaForTesting([&](const zip::Progress& progress) { + LOG(INFO) << "Zipping... " << std::setw(3) + << (100 * progress.bytes / src_size) << "%"; + return true; + }); + EXPECT_TRUE(zip::Zip({.src_dir = src_dir, + .dest_file = dest_file, + .progress_callback = std::move(progress_callback), + .progress_period = base::Seconds(1)})); + + // Since the dummy source (inner) ZIP file should simply be stored in the + // destination (outer) ZIP file, the destination file should be bigger than + // the source file, but not much bigger. + int64_t dest_file_size; + ASSERT_TRUE(base::GetFileSize(dest_file, &dest_file_size)); + EXPECT_GT(dest_file_size, src_size + 100); + EXPECT_LT(dest_file_size, src_size + 300); + + LOG(INFO) << "Reading big ZIP " << dest_file; + zip::ZipReader reader; + ASSERT_TRUE(reader.Open(dest_file)); + + const zip::ZipReader::Entry* const entry = reader.Next(); + ASSERT_TRUE(entry); + EXPECT_EQ(FP("src.zip"), entry->path); + EXPECT_EQ(src_size, entry->original_size); + EXPECT_FALSE(entry->is_directory); + EXPECT_FALSE(entry->is_encrypted); + + ProgressWriterDelegate writer(src_size); + EXPECT_TRUE(reader.ExtractCurrentEntry(&writer, + std::numeric_limits<uint64_t>::max())); + EXPECT_EQ(src_size, writer.received_bytes()); + + EXPECT_FALSE(reader.Next()); + EXPECT_TRUE(reader.ok()); +} + } // namespace diff --git a/google/zip_writer.cc b/google/zip_writer.cc index 6f38d42..e3f677f 100644 --- a/google/zip_writer.cc +++ b/google/zip_writer.cc @@ -4,201 +4,305 @@ #include "third_party/zlib/google/zip_writer.h" +#include <algorithm> + #include "base/files/file.h" #include "base/logging.h" +#include "base/strings/strcat.h" #include "base/strings/string_util.h" +#include "third_party/zlib/google/redact.h" #include "third_party/zlib/google/zip_internal.h" namespace zip { namespace internal { -namespace { +bool ZipWriter::ShouldContinue() { + if (!progress_callback_) + return true; + + const base::TimeTicks now = base::TimeTicks::Now(); + if (next_progress_report_time_ > now) + return true; + + next_progress_report_time_ = now + progress_period_; + if (progress_callback_.Run(progress_)) + return true; -// Numbers of pending entries that trigger writting them to the ZIP file. -constexpr size_t kMaxPendingEntriesCount = 50; + LOG(ERROR) << "Cancelling ZIP creation"; + return false; +} -bool AddFileContentToZip(zipFile zip_file, - base::File file, - const base::FilePath& file_path) { - int num_bytes; +bool ZipWriter::AddFileContent(const base::FilePath& path, base::File file) { char buf[zip::internal::kZipBufSize]; - do { - num_bytes = file.ReadAtCurrentPos(buf, zip::internal::kZipBufSize); - if (num_bytes > 0) { - if (zipWriteInFileInZip(zip_file, buf, num_bytes) != ZIP_OK) { - DLOG(ERROR) << "Could not write data to zip for path " - << file_path.value(); - return false; - } + while (ShouldContinue()) { + const int num_bytes = + file.ReadAtCurrentPos(buf, zip::internal::kZipBufSize); + + if (num_bytes < 0) { + PLOG(ERROR) << "Cannot read file " << Redact(path); + return false; } - } while (num_bytes > 0); - return true; + if (num_bytes == 0) + return true; + + if (zipWriteInFileInZip(zip_file_, buf, num_bytes) != ZIP_OK) { + PLOG(ERROR) << "Cannot write data from file " << Redact(path) + << " to ZIP"; + return false; + } + + progress_.bytes += num_bytes; + } + + return false; } -bool OpenNewFileEntry(zipFile zip_file, - const base::FilePath& path, - bool is_directory, - base::Time last_modified) { +bool ZipWriter::OpenNewFileEntry(const base::FilePath& path, + bool is_directory, + base::Time last_modified) { std::string str_path = path.AsUTF8Unsafe(); + #if defined(OS_WIN) base::ReplaceSubstringsAfterOffset(&str_path, 0u, "\\", "/"); #endif - if (is_directory) + + Compression compression = kDeflated; + + if (is_directory) { str_path += "/"; + } else { + compression = GetCompressionMethod(path); + } - return zip::internal::ZipOpenNewFileInZip(zip_file, str_path, last_modified); + return zip::internal::ZipOpenNewFileInZip(zip_file_, str_path, last_modified, + compression); } -bool CloseNewFileEntry(zipFile zip_file) { - return zipCloseFileInZip(zip_file) == ZIP_OK; +bool ZipWriter::CloseNewFileEntry() { + return zipCloseFileInZip(zip_file_) == ZIP_OK; } -bool AddFileEntryToZip(zipFile zip_file, - const base::FilePath& path, - base::File file) { - base::File::Info file_info; - if (!file.GetInfo(&file_info)) +bool ZipWriter::AddFileEntry(const base::FilePath& path, base::File file) { + base::File::Info info; + if (!file.GetInfo(&info)) return false; - if (!OpenNewFileEntry(zip_file, path, /*is_directory=*/false, - file_info.last_modified)) + if (!OpenNewFileEntry(path, /*is_directory=*/false, info.last_modified)) return false; - bool success = AddFileContentToZip(zip_file, std::move(file), path); - if (!CloseNewFileEntry(zip_file)) + if (!AddFileContent(path, std::move(file))) return false; - return success; + progress_.files++; + return CloseNewFileEntry(); } -bool AddDirectoryEntryToZip(zipFile zip_file, - const base::FilePath& path, - base::Time last_modified) { - return OpenNewFileEntry(zip_file, path, /*is_directory=*/true, - last_modified) && - CloseNewFileEntry(zip_file); -} +bool ZipWriter::AddDirectoryEntry(const base::FilePath& path) { + FileAccessor::Info info; + if (!file_accessor_->GetInfo(path, &info) || !info.is_directory) { + LOG(ERROR) << "Not a directory: " << Redact(path); + progress_.errors++; + return continue_on_error_; + } -} // namespace + if (!OpenNewFileEntry(path, /*is_directory=*/true, info.last_modified)) + return false; + + if (!CloseNewFileEntry()) + return false; -#if defined(OS_POSIX) + progress_.directories++; + if (!ShouldContinue()) + return false; + + if (!recursive_) + return true; + + return AddDirectoryContents(path); +} + +#if defined(OS_POSIX) || defined(OS_FUCHSIA) // static std::unique_ptr<ZipWriter> ZipWriter::CreateWithFd( int zip_file_fd, - const base::FilePath& root_dir, FileAccessor* file_accessor) { DCHECK(zip_file_fd != base::kInvalidPlatformFile); zipFile zip_file = internal::OpenFdForZipping(zip_file_fd, APPEND_STATUS_CREATE); + if (!zip_file) { - DLOG(ERROR) << "Couldn't create ZIP file for FD " << zip_file_fd; + DLOG(ERROR) << "Cannot create ZIP file for FD " << zip_file_fd; return nullptr; } - return std::unique_ptr<ZipWriter>( - new ZipWriter(zip_file, root_dir, file_accessor)); + + return std::unique_ptr<ZipWriter>(new ZipWriter(zip_file, file_accessor)); } #endif // static std::unique_ptr<ZipWriter> ZipWriter::Create( const base::FilePath& zip_file_path, - const base::FilePath& root_dir, FileAccessor* file_accessor) { DCHECK(!zip_file_path.empty()); zipFile zip_file = internal::OpenForZipping(zip_file_path.AsUTF8Unsafe(), APPEND_STATUS_CREATE); + if (!zip_file) { - DLOG(ERROR) << "Couldn't create ZIP file at path " << zip_file_path; + PLOG(ERROR) << "Cannot create ZIP file " << Redact(zip_file_path); return nullptr; } - return std::unique_ptr<ZipWriter>( - new ZipWriter(zip_file, root_dir, file_accessor)); -} - -ZipWriter::ZipWriter(zipFile zip_file, - const base::FilePath& root_dir, - FileAccessor* file_accessor) - : zip_file_(zip_file), root_dir_(root_dir), file_accessor_(file_accessor) {} -ZipWriter::~ZipWriter() { - DCHECK(pending_entries_.empty()); + return std::unique_ptr<ZipWriter>(new ZipWriter(zip_file, file_accessor)); } -bool ZipWriter::WriteEntries(const std::vector<base::FilePath>& paths) { - return AddEntries(paths) && Close(); -} +ZipWriter::ZipWriter(zipFile zip_file, FileAccessor* file_accessor) + : zip_file_(zip_file), file_accessor_(file_accessor) {} -bool ZipWriter::AddEntries(const std::vector<base::FilePath>& paths) { - DCHECK(zip_file_); - pending_entries_.insert(pending_entries_.end(), paths.begin(), paths.end()); - return FlushEntriesIfNeeded(/*force=*/false); +ZipWriter::~ZipWriter() { + if (zip_file_) + zipClose(zip_file_, nullptr); } bool ZipWriter::Close() { - bool success = FlushEntriesIfNeeded(/*force=*/true) && - zipClose(zip_file_, nullptr) == ZIP_OK; + const bool success = zipClose(zip_file_, nullptr) == ZIP_OK; zip_file_ = nullptr; + + // Call the progress callback one last time with the final progress status. + if (progress_callback_ && !progress_callback_.Run(progress_)) { + LOG(ERROR) << "Cancelling ZIP creation at the end"; + return false; + } + return success; } -bool ZipWriter::FlushEntriesIfNeeded(bool force) { - if (pending_entries_.size() < kMaxPendingEntriesCount && !force) - return true; +bool ZipWriter::AddMixedEntries(Paths paths) { + // Pointers to directory paths in |paths|. + std::vector<const base::FilePath*> directories; - while (pending_entries_.size() >= kMaxPendingEntriesCount || - (force && !pending_entries_.empty())) { - size_t entry_count = - std::min(pending_entries_.size(), kMaxPendingEntriesCount); - std::vector<base::FilePath> relative_paths; - std::vector<base::FilePath> absolute_paths; - relative_paths.insert(relative_paths.begin(), pending_entries_.begin(), - pending_entries_.begin() + entry_count); - for (auto iter = pending_entries_.begin(); - iter != pending_entries_.begin() + entry_count; ++iter) { - // The FileAccessor requires absolute paths. - absolute_paths.push_back(root_dir_.Append(*iter)); - } - pending_entries_.erase(pending_entries_.begin(), - pending_entries_.begin() + entry_count); - - // We don't know which paths are files and which ones are directories, and - // we want to avoid making a call to file_accessor_ for each entry. Open the - // files instead, invalid files are returned for directories. - std::vector<base::File> files = - file_accessor_->OpenFilesForReading(absolute_paths); - DCHECK_EQ(files.size(), relative_paths.size()); - for (size_t i = 0; i < files.size(); i++) { + // Declared outside loop to reuse internal buffer. + std::vector<base::File> files; + + // First pass. We don't know which paths are files and which ones are + // directories, and we want to avoid making a call to file_accessor_ for each + // path. Try to open all of the paths as files. We'll get invalid file + // descriptors for directories, and we'll process these directories in the + // second pass. + while (!paths.empty()) { + // Work with chunks of 50 paths at most. + const size_t n = std::min<size_t>(paths.size(), 50); + const Paths relative_paths = paths.subspan(0, n); + paths = paths.subspan(n, paths.size() - n); + + files.clear(); + if (!file_accessor_->Open(relative_paths, &files) || files.size() != n) + return false; + + for (size_t i = 0; i < n; i++) { const base::FilePath& relative_path = relative_paths[i]; - const base::FilePath& absolute_path = absolute_paths[i]; - base::File file = std::move(files[i]); + DCHECK(!relative_path.empty()); + base::File& file = files[i]; + if (file.IsValid()) { - if (!AddFileEntryToZip(zip_file_, relative_path, std::move(file))) { - LOG(ERROR) << "Failed to write file " << relative_path.value() - << " to ZIP file."; + // It's a file. + if (!AddFileEntry(relative_path, std::move(file))) return false; - } } else { - // Missing file or directory case. - base::Time last_modified = - file_accessor_->GetLastModifiedTime(absolute_path); - if (last_modified.is_null()) { - LOG(ERROR) << "Failed to write entry " << relative_path.value() - << " to ZIP file."; - return false; - } - DCHECK(file_accessor_->DirectoryExists(absolute_path)); - if (!AddDirectoryEntryToZip(zip_file_, relative_path, last_modified)) { - LOG(ERROR) << "Failed to write directory " << relative_path.value() - << " to ZIP file."; - return false; - } + // It's probably a directory. Remember its path for the second pass. + directories.push_back(&relative_path); } } } + + // Second pass for directories discovered during the first pass. + for (const base::FilePath* const path : directories) { + DCHECK(path); + if (!AddDirectoryEntry(*path)) + return false; + } + return true; } +bool ZipWriter::AddFileEntries(Paths paths) { + // Declared outside loop to reuse internal buffer. + std::vector<base::File> files; + + while (!paths.empty()) { + // Work with chunks of 50 paths at most. + const size_t n = std::min<size_t>(paths.size(), 50); + const Paths relative_paths = paths.subspan(0, n); + paths = paths.subspan(n, paths.size() - n); + + DCHECK_EQ(relative_paths.size(), n); + + files.clear(); + if (!file_accessor_->Open(relative_paths, &files) || files.size() != n) + return false; + + for (size_t i = 0; i < n; i++) { + const base::FilePath& relative_path = relative_paths[i]; + DCHECK(!relative_path.empty()); + base::File& file = files[i]; + + if (!file.IsValid()) { + LOG(ERROR) << "Cannot open " << Redact(relative_path); + progress_.errors++; + + if (continue_on_error_) + continue; + + return false; + } + + if (!AddFileEntry(relative_path, std::move(file))) + return false; + } + } + + return true; +} + +bool ZipWriter::AddDirectoryEntries(Paths paths) { + for (const base::FilePath& path : paths) { + if (!AddDirectoryEntry(path)) + return false; + } + + return true; +} + +bool ZipWriter::AddDirectoryContents(const base::FilePath& path) { + std::vector<base::FilePath> files, subdirs; + + if (!file_accessor_->List(path, &files, &subdirs)) { + progress_.errors++; + return continue_on_error_; + } + + Filter(&files); + Filter(&subdirs); + + if (!AddFileEntries(files)) + return false; + + return AddDirectoryEntries(subdirs); +} + +void ZipWriter::Filter(std::vector<base::FilePath>* const paths) { + DCHECK(paths); + + if (!filter_callback_) + return; + + const auto end = std::remove_if(paths->begin(), paths->end(), + [this](const base::FilePath& path) { + return !filter_callback_.Run(path); + }); + paths->erase(end, paths->end()); +} + } // namespace internal } // namespace zip diff --git a/google/zip_writer.h b/google/zip_writer.h index bd2a727..aa3c965 100644 --- a/google/zip_writer.h +++ b/google/zip_writer.h @@ -9,6 +9,7 @@ #include <vector> #include "base/files/file_path.h" +#include "base/time/time.h" #include "build/build_config.h" #include "third_party/zlib/google/zip.h" @@ -28,64 +29,126 @@ namespace internal { // performance reasons as these calls may be expensive when IPC based). // This class is so far internal and only used by zip.cc, but could be made // public if needed. +// +// All methods returning a bool return true on success and false on error. class ZipWriter { public: -// Creates a writer that will write a ZIP file to |zip_file_fd|/|zip_file| -// and which entries (specifies with AddEntries) are relative to |root_dir|. +// Creates a writer that will write a ZIP file to |zip_file_fd| or |zip_file| +// and which entries are relative to |file_accessor|'s source directory. // All file reads are performed using |file_accessor|. -#if defined(OS_POSIX) +#if defined(OS_POSIX) || defined(OS_FUCHSIA) static std::unique_ptr<ZipWriter> CreateWithFd(int zip_file_fd, - const base::FilePath& root_dir, FileAccessor* file_accessor); #endif + static std::unique_ptr<ZipWriter> Create(const base::FilePath& zip_file, - const base::FilePath& root_dir, FileAccessor* file_accessor); + + ZipWriter(const ZipWriter&) = delete; + ZipWriter& operator=(const ZipWriter&) = delete; + ~ZipWriter(); - // Writes the files at |paths| to the ZIP file and closes this Zip file. - // Note that the the FilePaths must be relative to |root_dir| specified in the - // Create method. - // Returns true if all entries were written successfuly. - bool WriteEntries(const std::vector<base::FilePath>& paths); + // Sets the optional progress callback. The callback is called once for each + // time |period|. The final callback is always called when the ZIP operation + // completes. + void SetProgressCallback(ProgressCallback callback, base::TimeDelta period) { + progress_callback_ = std::move(callback); + progress_period_ = std::move(period); + } + + // Should ignore missing files and directories? + void ContinueOnError(bool continue_on_error) { + continue_on_error_ = continue_on_error; + } + + // Sets the recursive flag, indicating whether the contents of subdirectories + // should be included. + void SetRecursive(bool b) { recursive_ = b; } + + // Sets the filter callback. + void SetFilterCallback(FilterCallback callback) { + filter_callback_ = std::move(callback); + } + + // Adds the contents of a directory. If the recursive flag is set, the + // contents of subdirectories are also added. + bool AddDirectoryContents(const base::FilePath& path); + + // Adds the entries at |paths| to the ZIP file. These can be a mixed bag of + // files and directories. If the recursive flag is set, the contents of + // subdirectories is also added. + bool AddMixedEntries(Paths paths); + + // Closes the ZIP file. + bool Close(); private: - ZipWriter(zipFile zip_file, - const base::FilePath& root_dir, - FileAccessor* file_accessor); + // Takes ownership of |zip_file|. + ZipWriter(zipFile zip_file, FileAccessor* file_accessor); - // Writes the pending entries to the ZIP file if there are at least - // |kMaxPendingEntriesCount| of them. If |force| is true, all pending entries - // are written regardless of how many there are. - // Returns false if writing an entry fails, true if no entry was written or - // there was no error writing entries. - bool FlushEntriesIfNeeded(bool force); + // Regularly called during processing to check whether zipping should continue + // or should be cancelled. + bool ShouldContinue(); - // Adds the files at |paths| to the ZIP file. These FilePaths must be relative - // to |root_dir| specified in the Create method. - bool AddEntries(const std::vector<base::FilePath>& paths); + // Adds file content to currently open file entry. + bool AddFileContent(const base::FilePath& path, base::File file); - // Closes the ZIP file. - // Returns true if successful, false otherwise (typically if an entry failed - // to be written). - bool Close(); + // Adds a file entry (including file contents). + bool AddFileEntry(const base::FilePath& path, base::File file); + + // Adds file entries. All the paths should be existing files. + bool AddFileEntries(Paths paths); + + // Adds a directory entry. If the recursive flag is set, the contents of this + // directory are also added. + bool AddDirectoryEntry(const base::FilePath& path); - // The entries that have been added but not yet written to the ZIP file. - std::vector<base::FilePath> pending_entries_; + // Adds directory entries. All the paths should be existing directories. If + // the recursive flag is set, the contents of these directories are also + // added. + bool AddDirectoryEntries(Paths paths); + + // Opens a file or directory entry. + bool OpenNewFileEntry(const base::FilePath& path, + bool is_directory, + base::Time last_modified); + + // Closes the currently open entry. + bool CloseNewFileEntry(); + + // Filters entries. + void Filter(std::vector<base::FilePath>* paths); // The actual zip file. zipFile zip_file_; - // Path to the directory entry paths are relative to. - base::FilePath root_dir_; - // Abstraction over file access methods used to read files. - FileAccessor* file_accessor_; + FileAccessor* const file_accessor_; + + // Progress stats. + Progress progress_; + + // Optional progress callback. + ProgressCallback progress_callback_; + + // Optional progress reporting period. + base::TimeDelta progress_period_; + + // Next time to report progress. + base::TimeTicks next_progress_report_time_ = base::TimeTicks::Now(); + + // Filter used to exclude files from the ZIP file. + FilterCallback filter_callback_; + + // Should recursively add directories? + bool recursive_ = false; - DISALLOW_COPY_AND_ASSIGN(ZipWriter); + // Should ignore missing files and directories? + bool continue_on_error_ = false; }; } // namespace internal } // namespace zip -#endif // THIRD_PARTY_ZLIB_GOOGLE_ZIP_WRITER_H_
\ No newline at end of file +#endif // THIRD_PARTY_ZLIB_GOOGLE_ZIP_WRITER_H_ diff --git a/patches/0008-minizip-zip-unzip-tools.patch b/patches/0008-minizip-zip-unzip-tools.patch new file mode 100644 index 0000000..48ceb02 --- /dev/null +++ b/patches/0008-minizip-zip-unzip-tools.patch @@ -0,0 +1,98 @@ +From 0c7de17000659f4f79de878296892c46be0aff77 Mon Sep 17 00:00:00 2001 +From: Noel Gordon <noel@chromium.org> +Date: Wed, 26 May 2021 21:57:43 +1000 +Subject: [PATCH] Build minizip zip and unzip tools + +--- + third_party/zlib/contrib/minizip/miniunz.c | 13 ++++++------- + third_party/zlib/contrib/minizip/minizip.c | 7 +++---- + 2 files changed, 9 insertions(+), 11 deletions(-) + +diff --git a/third_party/zlib/contrib/minizip/miniunz.c b/third_party/zlib/contrib/minizip/miniunz.c +index 3d65401be5cd..08737f689a96 100644 +--- a/third_party/zlib/contrib/minizip/miniunz.c ++++ b/third_party/zlib/contrib/minizip/miniunz.c +@@ -12,7 +12,7 @@ + Copyright (C) 2009-2010 Mathias Svensson ( http://result42.com ) + */ + +-#if (!defined(_WIN32)) && (!defined(WIN32)) && (!defined(__APPLE__)) ++#if (!defined(_WIN32)) && (!defined(WIN32)) && (!defined(__APPLE__)) && (!defined(__ANDROID_API__)) + #ifndef __USE_FILE_OFFSET64 + #define __USE_FILE_OFFSET64 + #endif +@@ -27,7 +27,7 @@ + #endif + #endif + +-#ifdef __APPLE__ ++#if defined(__APPLE__) || defined(__Fuchsia__) || defined(__ANDROID_API__) + // In darwin and perhaps other BSD variants off_t is a 64 bit value, hence no need for specific 64 bit functions + #define FOPEN_FUNC(filename, mode) fopen(filename, mode) + #define FTELLO_FUNC(stream) ftello(stream) +@@ -45,6 +45,7 @@ + #include <time.h> + #include <errno.h> + #include <fcntl.h> ++#include <sys/stat.h> + + #ifdef _WIN32 + # include <direct.h> +@@ -97,7 +98,7 @@ void change_file_date(filename,dosdate,tmu_date) + SetFileTime(hFile,&ftm,&ftLastAcc,&ftm); + CloseHandle(hFile); + #else +-#ifdef unix || __APPLE__ ++#if defined(unix) || defined(__APPLE__) || defined(__Fuchsia__) || defined(__ANDROID_API__) + struct utimbuf ut; + struct tm newdate; + newdate.tm_sec = tmu_date.tm_sec; +@@ -125,11 +126,9 @@ int mymkdir(dirname) + const char* dirname; + { + int ret=0; +-#ifdef _WIN32 ++#if defined(_WIN32) + ret = _mkdir(dirname); +-#elif unix +- ret = mkdir (dirname,0775); +-#elif __APPLE__ ++#elif defined(unix) || defined(__APPLE__) || defined(__Fuchsia__) || defined(__ANDROID_API__) + ret = mkdir (dirname,0775); + #endif + return ret; +diff --git a/third_party/zlib/contrib/minizip/minizip.c b/third_party/zlib/contrib/minizip/minizip.c +index 4288962ecef0..b794953c5c23 100644 +--- a/third_party/zlib/contrib/minizip/minizip.c ++++ b/third_party/zlib/contrib/minizip/minizip.c +@@ -12,8 +12,7 @@ + Copyright (C) 2009-2010 Mathias Svensson ( http://result42.com ) + */ + +- +-#if (!defined(_WIN32)) && (!defined(WIN32)) && (!defined(__APPLE__)) ++#if (!defined(_WIN32)) && (!defined(WIN32)) && (!defined(__APPLE__)) && (!defined(__ANDROID_API__)) + #ifndef __USE_FILE_OFFSET64 + #define __USE_FILE_OFFSET64 + #endif +@@ -28,7 +27,7 @@ + #endif + #endif + +-#ifdef __APPLE__ ++#if defined(__APPLE__) || defined(__Fuchsia__) || defined(__ANDROID_API__) + // In darwin and perhaps other BSD variants off_t is a 64 bit value, hence no need for specific 64 bit functions + #define FOPEN_FUNC(filename, mode) fopen(filename, mode) + #define FTELLO_FUNC(stream) ftello(stream) +@@ -94,7 +93,7 @@ uLong filetime(f, tmzip, dt) + return ret; + } + #else +-#ifdef unix || __APPLE__ ++#if defined(unix) || defined(__APPLE__) || defined(__Fuchsia__) || defined(__ANDROID_API__) + uLong filetime(f, tmzip, dt) + char *f; /* name of file to get info on */ + tm_zip *tmzip; /* return value: access, modific. and creation times */ +-- +2.31.1.818.g46aad6cb9e-goog + diff --git a/patches/0009-infcover-oob.patch b/patches/0009-infcover-oob.patch new file mode 100644 index 0000000..648360f --- /dev/null +++ b/patches/0009-infcover-oob.patch @@ -0,0 +1,24 @@ +From 75690b2683667be5535ac6243438115dc9c40f6a Mon Sep 17 00:00:00 2001 +From: Florian Mayer <fmayer@google.com> +Date: Wed, 16 Mar 2022 16:38:36 -0700 +Subject: [PATCH] Fix out of bounds in infcover.c. + +--- + test/infcover.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/test/infcover.c b/test/infcover.c +index 2be01646c..a6d83693c 100644 +--- a/test/infcover.c ++++ b/test/infcover.c +@@ -373,7 +373,9 @@ local void cover_support(void) + mem_setup(&strm); + strm.avail_in = 0; + strm.next_in = Z_NULL; +- ret = inflateInit_(&strm, ZLIB_VERSION - 1, (int)sizeof(z_stream)); ++ char versioncpy[] = ZLIB_VERSION; ++ versioncpy[0] -= 1; ++ ret = inflateInit_(&strm, versioncpy, (int)sizeof(z_stream)); + assert(ret == Z_VERSION_ERROR); + mem_done(&strm, "wrong version"); + |