diff options
Diffstat (limited to 'google')
| -rw-r--r-- | google/BUILD.gn | 2 | ||||
| -rw-r--r-- | google/compression_utils.cc | 29 | ||||
| -rw-r--r-- | google/compression_utils.h | 2 | ||||
| -rw-r--r-- | google/compression_utils_portable.cc | 2 | ||||
| -rw-r--r-- | google/compression_utils_portable.h | 2 | ||||
| -rw-r--r-- | google/compression_utils_unittest.cc | 2 | ||||
| -rw-r--r-- | google/redact.h | 2 | ||||
| -rw-r--r-- | google/test/data/Mixed Paths.zip | bin | 9793 -> 12814 bytes | |||
| -rw-r--r-- | google/test/data/Windows Special Names.zip | bin | 565 -> 3985 bytes | |||
| -rwxr-xr-x | google/test/data/create_test_zip.sh | 2 | ||||
| -rw-r--r-- | google/zip.cc | 7 | ||||
| -rw-r--r-- | google/zip.h | 8 | ||||
| -rw-r--r-- | google/zip_internal.cc | 2 | ||||
| -rw-r--r-- | google/zip_internal.h | 2 | ||||
| -rw-r--r-- | google/zip_reader.cc | 192 | ||||
| -rw-r--r-- | google/zip_reader.h | 73 | ||||
| -rw-r--r-- | google/zip_reader_unittest.cc | 20 | ||||
| -rw-r--r-- | google/zip_unittest.cc | 257 | ||||
| -rw-r--r-- | google/zip_writer.cc | 2 | ||||
| -rw-r--r-- | google/zip_writer.h | 2 |
20 files changed, 457 insertions, 151 deletions
diff --git a/google/BUILD.gn b/google/BUILD.gn index e996b16..35ba1da 100644 --- a/google/BUILD.gn +++ b/google/BUILD.gn @@ -1,4 +1,4 @@ -# Copyright 2017 The Chromium Authors. All rights reserved. +# Copyright 2017 The Chromium Authors # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. diff --git a/google/compression_utils.cc b/google/compression_utils.cc index 781c805..279ea07 100644 --- a/google/compression_utils.cc +++ b/google/compression_utils.cc @@ -1,4 +1,4 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. +// Copyright 2014 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -24,8 +24,8 @@ bool GzipCompress(base::span<const char> input, // uLongf can be larger than size_t. uLongf compressed_size_long = static_cast<uLongf>(output_buffer_size); if (zlib_internal::GzipCompressHelper( - bit_cast<Bytef*>(output_buffer), &compressed_size_long, - bit_cast<const Bytef*>(input.data()), + base::bit_cast<Bytef*>(output_buffer), &compressed_size_long, + base::bit_cast<const Bytef*>(input.data()), static_cast<uLongf>(input.size()), malloc_fn, free_fn) != Z_OK) { return false; } @@ -53,9 +53,10 @@ bool GzipCompress(base::span<const uint8_t> input, std::string* output) { return false; } - if (zlib_internal::GzipCompressHelper(compressed_data, &compressed_data_size, - bit_cast<const Bytef*>(input.data()), - input_size, nullptr, nullptr) != Z_OK) { + if (zlib_internal::GzipCompressHelper( + compressed_data, &compressed_data_size, + base::bit_cast<const Bytef*>(input.data()), input_size, nullptr, + nullptr) != Z_OK) { free(compressed_data); return false; } @@ -76,13 +77,13 @@ bool GzipCompress(base::span<const uint8_t> input, std::string* output) { bool GzipUncompress(const std::string& input, std::string* output) { std::string uncompressed_output; uLongf uncompressed_size = static_cast<uLongf>(GetUncompressedSize(input)); - if (uncompressed_size > uncompressed_output.max_size()) + if (size_t{uncompressed_size} > uncompressed_output.max_size()) return false; uncompressed_output.resize(uncompressed_size); if (zlib_internal::GzipUncompressHelper( - bit_cast<Bytef*>(uncompressed_output.data()), &uncompressed_size, - bit_cast<const Bytef*>(input.data()), + base::bit_cast<Bytef*>(uncompressed_output.data()), + &uncompressed_size, base::bit_cast<const Bytef*>(input.data()), static_cast<uLongf>(input.length())) == Z_OK) { output->swap(uncompressed_output); return true; @@ -101,8 +102,8 @@ bool GzipUncompress(base::span<const uint8_t> input, if (uncompressed_size > output.size()) return false; return zlib_internal::GzipUncompressHelper( - bit_cast<Bytef*>(output.data()), &uncompressed_size, - bit_cast<const Bytef*>(input.data()), + base::bit_cast<Bytef*>(output.data()), &uncompressed_size, + base::bit_cast<const Bytef*>(input.data()), static_cast<uLongf>(input.size())) == Z_OK; } @@ -116,8 +117,8 @@ bool GzipUncompress(base::span<const uint8_t> input, std::string* output) { uLongf uncompressed_size = GetUncompressedSize(input); output->resize(uncompressed_size); return zlib_internal::GzipUncompressHelper( - bit_cast<Bytef*>(output->data()), &uncompressed_size, - bit_cast<const Bytef*>(input.data()), + base::bit_cast<Bytef*>(output->data()), &uncompressed_size, + base::bit_cast<const Bytef*>(input.data()), static_cast<uLongf>(input.size())) == Z_OK; } @@ -127,7 +128,7 @@ uint32_t GetUncompressedSize(base::span<const char> compressed_data) { uint32_t GetUncompressedSize(base::span<const uint8_t> compressed_data) { return zlib_internal::GetGzipUncompressedSize( - bit_cast<Bytef*>(compressed_data.data()), compressed_data.size()); + base::bit_cast<Bytef*>(compressed_data.data()), compressed_data.size()); } } // namespace compression diff --git a/google/compression_utils.h b/google/compression_utils.h index cca47be..ea39981 100644 --- a/google/compression_utils.h +++ b/google/compression_utils.h @@ -1,4 +1,4 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. +// Copyright 2014 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. diff --git a/google/compression_utils_portable.cc b/google/compression_utils_portable.cc index 331e41e..49d6bfe 100644 --- a/google/compression_utils_portable.cc +++ b/google/compression_utils_portable.cc @@ -1,6 +1,6 @@ /* compression_utils_portable.cc * - * Copyright 2019 The Chromium Authors. All rights reserved. + * Copyright 2019 The Chromium Authors * Use of this source code is governed by a BSD-style license that can be * found in the Chromium source repository LICENSE file. */ diff --git a/google/compression_utils_portable.h b/google/compression_utils_portable.h index c1f3775..92b033e 100644 --- a/google/compression_utils_portable.h +++ b/google/compression_utils_portable.h @@ -1,6 +1,6 @@ /* compression_utils_portable.h * - * Copyright 2019 The Chromium Authors. All rights reserved. + * Copyright 2019 The Chromium Authors * Use of this source code is governed by a BSD-style license that can be * found in the Chromium source repository LICENSE file. */ diff --git a/google/compression_utils_unittest.cc b/google/compression_utils_unittest.cc index 76572e5..8e24387 100644 --- a/google/compression_utils_unittest.cc +++ b/google/compression_utils_unittest.cc @@ -1,4 +1,4 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. +// Copyright 2014 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. diff --git a/google/redact.h b/google/redact.h index ea7da16..df6bcaf 100644 --- a/google/redact.h +++ b/google/redact.h @@ -1,4 +1,4 @@ -// Copyright (c) 2022 The Chromium Authors. All rights reserved. +// Copyright 2022 The Chromium Authors // 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_ diff --git a/google/test/data/Mixed Paths.zip b/google/test/data/Mixed Paths.zip Binary files differindex 2af418b..881f498 100644 --- a/google/test/data/Mixed Paths.zip +++ b/google/test/data/Mixed Paths.zip diff --git a/google/test/data/Windows Special Names.zip b/google/test/data/Windows Special Names.zip Binary files differindex 3b8a3ab..4fac907 100644 --- a/google/test/data/Windows Special Names.zip +++ b/google/test/data/Windows Special Names.zip diff --git a/google/test/data/create_test_zip.sh b/google/test/data/create_test_zip.sh index f4cc635..620e8f6 100755 --- a/google/test/data/create_test_zip.sh +++ b/google/test/data/create_test_zip.sh @@ -1,6 +1,6 @@ #!/bin/bash # -# Copyright (c) 2011 The Chromium Authors. All rights reserved. +# Copyright 2011 The Chromium Authors # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. # diff --git a/google/zip.cc b/google/zip.cc index 1a43196..490dcee 100644 --- a/google/zip.cc +++ b/google/zip.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright 2012 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -229,7 +229,10 @@ bool Unzip(const base::PlatformFile& src_file, // It's a file. std::unique_ptr<WriterDelegate> writer = writer_factory.Run(entry->path); - if (!writer || !reader.ExtractCurrentEntry(writer.get())) { + if (!writer || + (options.progress ? !reader.ExtractCurrentEntryWithListener( + writer.get(), options.progress) + : !reader.ExtractCurrentEntry(writer.get()))) { LOG(ERROR) << "Cannot extract file " << Redact(entry->path) << " from ZIP"; if (!options.continue_on_error) diff --git a/google/zip.h b/google/zip.h index 25ec655..e3036c8 100644 --- a/google/zip.h +++ b/google/zip.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright 2011 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -170,6 +170,9 @@ bool ZipFiles(const base::FilePath& src_dir, int dest_fd); #endif // defined(OS_POSIX) || defined(OS_FUCHSIA) +// Callback reporting the number of bytes written during Unzip. +using UnzipProgressCallback = base::RepeatingCallback<void(uint64_t bytes)>; + // Options of the Unzip function, with valid default values. struct UnzipOptions { // Encoding of entry paths in the ZIP archive. By default, paths are assumed @@ -180,6 +183,9 @@ struct UnzipOptions { // everything gets extracted. FilterCallback filter; + // Callback to report bytes extracted from the ZIP. + UnzipProgressCallback progress; + // Password to decrypt the encrypted files. std::string password; diff --git a/google/zip_internal.cc b/google/zip_internal.cc index 1adf2e6..e65d7ce 100644 --- a/google/zip_internal.cc +++ b/google/zip_internal.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright 2011 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. diff --git a/google/zip_internal.h b/google/zip_internal.h index 92833fa..f107d7f 100644 --- a/google/zip_internal.h +++ b/google/zip_internal.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright 2011 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. diff --git a/google/zip_reader.cc b/google/zip_reader.cc index b8143cd..e97027a 100644 --- a/google/zip_reader.cc +++ b/google/zip_reader.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Copyright 2012 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -59,6 +59,26 @@ std::ostream& operator<<(std::ostream& out, UnzipError error) { #undef SWITCH_ERR } +bool IsValidFileNameCharacterOnWindows(char16_t c) { + if (c < 32) + return false; + + switch (c) { + case '<': // Less than + case '>': // Greater than + case ':': // Colon + case '"': // Double quote + case '|': // Vertical bar or pipe + case '?': // Question mark + case '*': // Asterisk + case '/': // Forward slash + case '\\': // Backslash + return false; + } + + return true; +} + // A writer delegate that writes to a given string. class StringWriterDelegate : public WriterDelegate { public: @@ -210,21 +230,19 @@ bool ZipReader::OpenEntry() { return false; } - 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"/"); + // Normalize path. + Normalize(path_in_utf16); - // 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"/"); + entry_.original_size = info.uncompressed_size; // The file content of this entry is encrypted if flag bit 0 is set. entry_.is_encrypted = info.flag & 1; + if (entry_.is_encrypted) { + // Is the entry AES encrypted. + entry_.uses_aes_encryption = info.compression_method == 99; + } else { + entry_.uses_aes_encryption = false; + } // Construct the last modified time. The timezone info is not present in ZIP // archives, so we construct the time as UTC. @@ -249,7 +267,88 @@ bool ZipReader::OpenEntry() { return true; } +void ZipReader::Normalize(base::StringPiece16 in) { + entry_.is_unsafe = true; + + // Directory entries in ZIP have a path ending with "/". + entry_.is_directory = base::EndsWith(in, u"/"); + + std::u16string normalized_path; + if (base::StartsWith(in, u"/")) { + normalized_path = u"ROOT"; + entry_.is_unsafe = false; + } + + for (;;) { + // Consume initial path separators. + const base::StringPiece16::size_type i = in.find_first_not_of(u'/'); + if (i == base::StringPiece16::npos) + break; + + in.remove_prefix(i); + DCHECK(!in.empty()); + + // Isolate next path component. + const base::StringPiece16 part = in.substr(0, in.find_first_of(u'/')); + DCHECK(!part.empty()); + + in.remove_prefix(part.size()); + + if (!normalized_path.empty()) + normalized_path += u'/'; + + if (part == u".") { + normalized_path += u"DOT"; + entry_.is_unsafe = true; + continue; + } + + if (part == u"..") { + normalized_path += u"UP"; + entry_.is_unsafe = true; + continue; + } + + // Windows has more restrictions than other systems when it comes to valid + // file paths. Replace Windows-invalid characters on all systems for + // consistency. In particular, this prevents a path component containing + // colon and backslash from being misinterpreted as an absolute path on + // Windows. + for (const char16_t c : part) { + normalized_path += IsValidFileNameCharacterOnWindows(c) ? c : 0xFFFD; + } + + entry_.is_unsafe = false; + } + + // If the entry is a directory, add the final path separator to the entry + // path. + if (entry_.is_directory && !normalized_path.empty()) { + normalized_path += u'/'; + entry_.is_unsafe = false; + } + + entry_.path = base::FilePath::FromUTF16Unsafe(normalized_path); + + // By construction, we should always get a relative path. + DCHECK(!entry_.path.IsAbsolute()) << entry_.path; +} + +void ZipReader::ReportProgress(ListenerCallback listener_callback, + uint64_t bytes) const { + delta_bytes_read_ += bytes; + + const base::TimeTicks now = base::TimeTicks::Now(); + if (next_progress_report_time_ > now) + return; + + next_progress_report_time_ = now + progress_period_; + listener_callback.Run(delta_bytes_read_); + delta_bytes_read_ = 0; +} + bool ZipReader::ExtractCurrentEntry(WriterDelegate* delegate, + ListenerCallback listener_callback, uint64_t num_bytes_to_extract) const { DCHECK(zip_file_); DCHECK_LT(0, next_index_); @@ -290,6 +389,10 @@ bool ZipReader::ExtractCurrentEntry(WriterDelegate* delegate, break; } + if (listener_callback) { + ReportProgress(listener_callback, num_bytes_read); + } + DCHECK_LT(0, num_bytes_read); CHECK_LE(num_bytes_read, internal::kZipBufSize); @@ -325,9 +428,26 @@ bool ZipReader::ExtractCurrentEntry(WriterDelegate* delegate, delegate->OnError(); } + if (listener_callback) { + listener_callback.Run(delta_bytes_read_); + delta_bytes_read_ = 0; + } + return entire_file_extracted; } +bool ZipReader::ExtractCurrentEntry(WriterDelegate* delegate, + uint64_t num_bytes_to_extract) const { + return ExtractCurrentEntry(delegate, ListenerCallback(), + num_bytes_to_extract); +} + +bool ZipReader::ExtractCurrentEntryWithListener( + WriterDelegate* delegate, + ListenerCallback listener_callback) const { + return ExtractCurrentEntry(delegate, listener_callback); +} + void ZipReader::ExtractCurrentEntryToFilePathAsync( const base::FilePath& output_file_path, SuccessCallback success_callback, @@ -439,6 +559,7 @@ void ZipReader::Reset() { next_index_ = 0; reached_end_ = true; ok_ = false; + delta_bytes_read_ = 0; entry_ = {}; } @@ -504,12 +625,26 @@ FileWriterDelegate::~FileWriterDelegate() {} bool FileWriterDelegate::PrepareOutput() { DCHECK(file_); - const bool ok = file_->IsValid(); - if (ok) { - DCHECK_EQ(file_->GetLength(), 0) - << " The output file should be initially empty"; + + if (!file_->IsValid()) { + LOG(ERROR) << "File is not valid"; + return false; } - return ok; + + const int64_t length = file_->GetLength(); + if (length < 0) { + PLOG(ERROR) << "Cannot get length of file handle " + << file_->GetPlatformFile(); + return false; + } + + // Just log a warning if the file is not empty. + // See crbug.com/1309879 and crbug.com/774762. + LOG_IF(WARNING, length > 0) + << "File handle " << file_->GetPlatformFile() + << " is not empty: Its length is " << length << " bytes"; + + return true; } bool FileWriterDelegate::WriteBytes(const char* data, int num_bytes) { @@ -553,10 +688,25 @@ bool FilePathWriterDelegate::PrepareOutput() { 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(); + if (!owned_file_.IsValid()) { + PLOG(ERROR) << "Cannot create file " << Redact(output_file_path_) << ": " + << base::File::ErrorToString(owned_file_.error_details()); + return false; + } + + const int64_t length = owned_file_.GetLength(); + if (length < 0) { + PLOG(ERROR) << "Cannot get length of file " << Redact(output_file_path_); + return false; + } + + if (length > 0) { + LOG(ERROR) << "File " << Redact(output_file_path_) + << " is not empty: Its length is " << length << " bytes"; + return false; + } + + return true; } void FilePathWriterDelegate::OnError() { diff --git a/google/zip_reader.h b/google/zip_reader.h index df7452a..48244c8 100644 --- a/google/zip_reader.h +++ b/google/zip_reader.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright 2011 The Chromium Authors // 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_ZIP_READER_H_ @@ -85,6 +85,9 @@ class ZipReader { // A callback that is called periodically during the operation with the number // of bytes that have been processed so far. using ProgressCallback = base::RepeatingCallback<void(int64_t)>; + // A callback that is called periodically during the operation with the number + // of bytes that have been processed since the previous call (i.e. delta). + using ListenerCallback = base::RepeatingCallback<void(uint64_t)>; // Information of an entry (file or directory) in a ZIP archive. struct Entry { @@ -94,9 +97,14 @@ class ZipReader { // 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|. + // Path of the entry, converted to Unicode. This path is relative (eg + // "foo/bar.txt"). Absolute paths (eg "/foo/bar.txt") or paths containing + // ".." or "." components (eg "../foo/bar.txt") are converted to safe + // relative paths. Eg: + // (In ZIP) -> (Entry.path) + // /foo/bar -> ROOT/foo/bar + // ../a -> UP/a + // ./a -> DOT/a base::FilePath path; // Size of the original uncompressed file, or 0 if the entry is a directory. @@ -120,14 +128,17 @@ class ZipReader { // True if the entry is a directory. // False if the entry is a file. - bool is_directory; + bool is_directory = false; - // True if the entry path is considered unsafe, ie if it is absolute or if - // it contains "..". - bool is_unsafe; + // True if the entry path cannot be converted to a safe relative path. This + // happens if a file entry (not a directory) has a filename "." or "..". + bool is_unsafe = false; // True if the file content is encrypted. - bool is_encrypted; + bool is_encrypted = false; + + // True if the encryption scheme is AES. + bool uses_aes_encryption = false; // Entry POSIX permissions (POSIX systems only). int posix_mode; @@ -200,6 +211,17 @@ class ZipReader { uint64_t num_bytes_to_extract = std::numeric_limits<uint64_t>::max()) const; + // Extracts the current entry to |delegate|, starting from the beginning + // of the entry, calling |listener_callback| regularly with the number of + // bytes extracted. + // + // Returns true if the entire file was extracted without error. + // + // Precondition: Next() returned a non-null Entry. + bool ExtractCurrentEntryWithListener( + WriterDelegate* delegate, + ListenerCallback listener_callback) 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. @@ -257,6 +279,25 @@ class ZipReader { // reset automatically as needed. bool OpenEntry(); + // Normalizes the given path passed as UTF-16 string piece. Sets entry_.path, + // entry_.is_directory and entry_.is_unsafe. + void Normalize(base::StringPiece16 in); + + // Runs the ListenerCallback at a throttled rate. + void ReportProgress(ListenerCallback listener_callback, uint64_t bytes) const; + + // Extracts |num_bytes_to_extract| bytes of the current entry to |delegate|, + // starting from the beginning of the entry calling |listener_callback| if + // its supplied. + // + // Returns true if the entire file was extracted without error. + // + // Precondition: Next() returned a non-null Entry. + bool ExtractCurrentEntry(WriterDelegate* delegate, + ListenerCallback listener_callback, + uint64_t num_bytes_to_extract = + std::numeric_limits<uint64_t>::max()) const; + // 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, @@ -274,11 +315,21 @@ class ZipReader { bool ok_; Entry entry_; + // Next time to report progress. + mutable base::TimeTicks next_progress_report_time_ = base::TimeTicks::Now(); + + // Progress time delta. + // TODO(crbug.com/953256) Add this as parameter to the unzip options. + base::TimeDelta progress_period_ = base::Milliseconds(1000); + + // Number of bytes read since last progress report callback executed. + mutable uint64_t delta_bytes_read_ = 0; + base::WeakPtrFactory<ZipReader> weak_ptr_factory_{this}; }; -// A writer delegate that writes to a given File. This file is expected to be -// initially empty. +// A writer delegate that writes to a given File. It is recommended that this +// file be initially empty. class FileWriterDelegate : public WriterDelegate { public: // Constructs a FileWriterDelegate that manipulates |file|. The delegate will diff --git a/google/zip_reader_unittest.cc b/google/zip_reader_unittest.cc index fc80637..52dab20 100644 --- a/google/zip_reader_unittest.cc +++ b/google/zip_reader_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright 2011 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -308,19 +308,18 @@ TEST_F(ZipReaderTest, DotDotFile) { ZipReader reader; ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("evil.zip"))); base::FilePath target_path(FILE_PATH_LITERAL( - "../levilevilevilevilevilevilevilevilevilevilevilevil")); + "UP/levilevilevilevilevilevilevilevilevilevilevilevil")); 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(entry->is_unsafe); + EXPECT_FALSE(entry->is_unsafe); EXPECT_FALSE(entry->is_directory); } TEST_F(ZipReaderTest, InvalidUTF8File) { ZipReader reader; ASSERT_TRUE(reader.Open(data_dir_.AppendASCII("evil_via_invalid_utf8.zip"))); - base::FilePath target_path = base::FilePath::FromUTF8Unsafe(".�.\\evil.txt"); + 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); @@ -337,7 +336,7 @@ 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���_/SJIS_835C_��.txt"), base::FilePath::FromUTF8Unsafe( "�V�����t�H���_/�V�����e�L�X�g �h�L�������g.txt"))); } @@ -349,7 +348,7 @@ 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"), + "\u0090V‚µ‚¢ƒtƒHƒ‹ƒ_/SJIS_835C_ƒ�.txt"), base::FilePath::FromUTF8Unsafe( "\u0090V‚µ‚¢ƒtƒHƒ‹ƒ_/\u0090V‚µ‚¢ƒeƒLƒXƒg " "ƒhƒLƒ…ƒ\u0081ƒ“ƒg.txt"))); @@ -361,7 +360,7 @@ 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ГЛГ_/SJIS_835C_Г�.txt"), base::FilePath::FromUTF8Unsafe( "РVВ╡ВвГtГHГЛГ_/РVВ╡ВвГeГLГXГg ГhГLГЕГБГУГg.txt"))); } @@ -380,12 +379,11 @@ TEST_F(ZipReaderTest, AbsoluteFile) { ZipReader reader; ASSERT_TRUE( reader.Open(data_dir_.AppendASCII("evil_via_absolute_file_name.zip"))); - base::FilePath target_path(FILE_PATH_LITERAL("/evil.txt")); + base::FilePath target_path(FILE_PATH_LITERAL("ROOT/evil.txt")); 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(entry->is_unsafe); + EXPECT_FALSE(entry->is_unsafe); EXPECT_FALSE(entry->is_directory); } diff --git a/google/zip_unittest.cc b/google/zip_unittest.cc index 8fbec32..b639e8e 100644 --- a/google/zip_unittest.cc +++ b/google/zip_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright 2011 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -20,6 +20,7 @@ #include "base/files/scoped_temp_dir.h" #include "base/logging.h" #include "base/path_service.h" +#include "base/strings/strcat.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/test/bind.h" @@ -38,6 +39,7 @@ namespace { using testing::UnorderedElementsAre; +using testing::UnorderedElementsAreArray; std::vector<std::string> GetRelativePaths(const base::FilePath& dir, base::FileEnumerator::FileType type) { @@ -46,7 +48,7 @@ std::vector<std::string> GetRelativePaths(const base::FilePath& dir, 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()); + got_paths.push_back(relative.NormalizePathSeparatorsTo('/').AsUTF8Unsafe()); } EXPECT_EQ(base::File::FILE_OK, files.GetError()); @@ -401,11 +403,9 @@ TEST_F(ZipTest, UnzipEvil) { // won't create a persistent file outside test_dir_ in case of a // failure. base::FilePath output_dir = test_dir_.AppendASCII("out"); - ASSERT_FALSE(zip::Unzip(path, output_dir)); - base::FilePath evil_file = output_dir; - evil_file = evil_file.AppendASCII( - "../levilevilevilevilevilevilevilevilevilevilevilevil"); - ASSERT_FALSE(base::PathExists(evil_file)); + EXPECT_TRUE(zip::Unzip(path, output_dir)); + EXPECT_TRUE(base::PathExists(output_dir.AppendASCII( + "UP/levilevilevilevilevilevilevilevilevilevilevilevil"))); } TEST_F(ZipTest, UnzipEvil2) { @@ -416,7 +416,7 @@ TEST_F(ZipTest, UnzipEvil2) { base::FilePath output_dir = test_dir_.AppendASCII("out"); ASSERT_TRUE(zip::Unzip(path, output_dir)); ASSERT_TRUE(base::PathExists( - output_dir.Append(base::FilePath::FromUTF8Unsafe(".�.\\evil.txt")))); + output_dir.Append(base::FilePath::FromUTF8Unsafe(".�.�evil.txt")))); ASSERT_FALSE(base::PathExists(output_dir.AppendASCII("../evil.txt"))); } @@ -588,37 +588,84 @@ TEST_F(ZipTest, UnzipCannotCreateParentDir) { EXPECT_EQ("First file", contents); } +// TODO(crbug.com/1311140) Detect and rename reserved file names on Windows. TEST_F(ZipTest, UnzipWindowsSpecialNames) { - EXPECT_TRUE(zip::Unzip( - GetDataDirectory().AppendASCII("Windows Special Names.zip"), test_dir_)); + EXPECT_TRUE( + zip::Unzip(GetDataDirectory().AppendASCII("Windows Special Names.zip"), + test_dir_, {.continue_on_error = true})); - std::string contents; - EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII("Last"), &contents)); - EXPECT_EQ("Last file", contents); + std::unordered_set<std::string> want_paths = { + "First", + "Last", + "CLOCK$", + " NUL.txt", +#ifndef OS_WIN + "NUL", + "NUL ", + "NUL.", + "NUL .", + "NUL.txt", + "NUL.tar.gz", + "NUL..txt", + "NUL...txt", + "NUL .txt", + "NUL .txt", + "NUL ..txt", +#ifndef OS_MAC + "Nul.txt", +#endif + "nul.very long extension", + "a/NUL", + "CON", + "PRN", + "AUX", + "COM1", + "COM2", + "COM3", + "COM4", + "COM5", + "COM6", + "COM7", + "COM8", + "COM9", + "LPT1", + "LPT2", + "LPT3", + "LPT4", + "LPT5", + "LPT6", + "LPT7", + "LPT8", + "LPT9", +#endif + }; + + const std::vector<std::string> got_paths = + GetRelativePaths(test_dir_, base::FileEnumerator::FileType::FILES); + + for (const std::string& path : got_paths) { + const bool ok = want_paths.erase(path); #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")); + if (!ok) { + // See crbug.com/1313991: Different versions of Windows treat these + // filenames differently. No hard error here if there is an unexpected + // file. + LOG(WARNING) << "Found unexpected file: " << std::quoted(path); + continue; + } #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(ok) << "Found unexpected file: " << std::quoted(path); +#endif - EXPECT_TRUE( - base::ReadFileToString(test_dir_.AppendASCII("Nul.txt"), &contents)); - EXPECT_EQ("This is: Nul.txt", contents); + std::string contents; + EXPECT_TRUE(base::ReadFileToString(test_dir_.AppendASCII(path), &contents)); + EXPECT_EQ(base::StrCat({"This is: ", path}), contents); + } - EXPECT_TRUE(base::ReadFileToString( - test_dir_.AppendASCII("nul.very long extension"), &contents)); - EXPECT_EQ("This is: nul.very long extension", contents); -#endif + for (const std::string& path : want_paths) { + EXPECT_TRUE(false) << "Cannot find expected file: " << std::quoted(path); + } } TEST_F(ZipTest, UnzipDifferentCases) { @@ -695,45 +742,49 @@ TEST_F(ZipTest, UnzipMixedPaths) { std::unordered_set<std::string> want_paths = { #ifdef OS_WIN - "Dot", // - "Space→", // - "a\\b", // - "u\\v\\w\\x\\y\\z", // - "←Backslash2", // + "Dot", // + "Space→", // #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 + " ", // + "AUX", // Disappears on Windows + "COM1", // Disappears on Windows + "COM2", // Disappears on Windows + "COM3", // Disappears on Windows + "COM4", // Disappears on Windows + "COM5", // Disappears on Windows + "COM6", // Disappears on Windows + "COM7", // Disappears on Windows + "COM8", // Disappears on Windows + "COM9", // Disappears on Windows + "CON", // Disappears on Windows + "Dot .", // + "LPT1", // Disappears on Windows + "LPT2", // Disappears on Windows + "LPT3", // Disappears on Windows + "LPT4", // Disappears on Windows + "LPT5", // Disappears on Windows + "LPT6", // Disappears on Windows + "LPT7", // Disappears on Windows + "LPT8", // Disappears on Windows + "LPT9", // Disappears on Windows + "NUL ..txt", // Disappears on Windows + "NUL .txt", // Disappears on Windows + "NUL ", // Disappears on Windows + "NUL .", // Disappears on Windows "NUL .txt", // Disappears on Windows "NUL", // Disappears on Windows + "NUL.", // Disappears on Windows + "NUL...txt", // 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", // + "PRN", // Disappears on Windows + "Space→ ", // + "c/NUL", // Disappears on Windows + "nul.very long extension", // Disappears on Windows #ifndef OS_MAC - "CASE", // - "Case", // + "CASE", // Conflicts with "Case" + "case", // Conflicts with "Case" #endif #endif " NUL.txt", // @@ -741,46 +792,82 @@ TEST_F(ZipTest, UnzipMixedPaths) { "$HOME", // "%TMP", // "-", // - "...Tree", // + "...Three", // "..Two", // ".One", // "Ampersand &", // + "Angle ��", // "At @", // - "Backslash3→\\←Backslash4", // + "Backslash1→�", // + "Backslash3→�←Backslash4", // + "Backspace �", // "Backtick `", // + "Bell �", // + "CLOCK$", // "Caret ^", // + "Carriage Return �", // + "Case", // + "Colon �", // "Comma ,", // "Curly {}", // + "C�", // + "C��", // + "C��Temp", // + "C��Temp�", // + "C��Temp�File", // "Dash -", // "Delete \x7F", // "Dollar $", // + "Double quote �", // "Equal =", // + "Escape �", // "Euro €", // "Exclamation !", // "FileOrDir", // "First", // "Hash #", // "Last", // + "Line Feed �", // "Percent %", // + "Pipe �", // "Plus +", // + "Question �", // "Quote '", // + "ROOT/At The Top", // + "ROOT/UP/Over The Top", // + "ROOT/dev/null", // "Round ()", // "Semicolon ;", // "Smile \U0001F642", // "Square []", // + "Star �", // "String Terminator \u009C", // + "Tab �", // "Tilde ~", // + "UP/One Level Up", // + "UP/UP/Two Levels Up", // "Underscore _", // - "case", // + "a/DOT/b", // + "a/UP/b", // + "u/v/w/x/y/z", // "~", // + "�←Backslash2", // + "��server�share�file", // }; 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); + const bool ok = want_paths.erase(path); +#ifdef OS_WIN + // See crbug.com/1313991: Different versions of Windows treat reserved + // Windows filenames differently. No hard error here if there is an + // unexpected file. + LOG_IF(WARNING, !ok) << "Found unexpected file: " << std::quoted(path); +#else + EXPECT_TRUE(ok) << "Found unexpected file: " << std::quoted(path); +#endif } for (const std::string& path : want_paths) { @@ -789,14 +876,24 @@ TEST_F(ZipTest, UnzipMixedPaths) { 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 - )); + UnorderedElementsAreArray({ + "Empty", + "ROOT", + "ROOT/Empty", + "ROOT/UP", + "ROOT/dev", + "UP", + "UP/UP", + "a", + "a/DOT", + "a/UP", + "c", + "u", + "u/v", + "u/v/w", + "u/v/w/x", + "u/v/w/x/y", + })); } TEST_F(ZipTest, UnzipWithDelegates) { @@ -897,7 +994,7 @@ TEST_F(ZipTest, UnzipSjisAsUtf8) { std::string contents; ASSERT_TRUE(base::ReadFileToString( - dir.Append(base::FilePath::FromUTF8Unsafe("SJIS_835C_�\\.txt")), + dir.Append(base::FilePath::FromUTF8Unsafe("SJIS_835C_��.txt")), &contents)); EXPECT_EQ( "This file's name contains 0x5c (backslash) as the 2nd byte of Japanese " @@ -1261,10 +1358,10 @@ TEST_F(ZipTest, NestedZip) { // 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). +// test (crbug.com/1299736, crbug.com/1300448, crbug.com/1369958). #if defined(THREAD_SANITIZER) || BUILDFLAG(IS_FUCHSIA) || \ BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) || \ - BUILDFLAG(IS_CHROMEOS_LACROS) || !defined(NDEBUG) + BUILDFLAG(IS_CHROMEOS) || !defined(NDEBUG) TEST_F(ZipTest, DISABLED_BigFile) { #else TEST_F(ZipTest, BigFile) { diff --git a/google/zip_writer.cc b/google/zip_writer.cc index e3f677f..31161ae 100644 --- a/google/zip_writer.cc +++ b/google/zip_writer.cc @@ -1,4 +1,4 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. +// Copyright 2017 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. diff --git a/google/zip_writer.h b/google/zip_writer.h index aa3c965..dd10929 100644 --- a/google/zip_writer.h +++ b/google/zip_writer.h @@ -1,4 +1,4 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. +// Copyright 2017 The Chromium Authors // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. |