diff options
-rw-r--r-- | google/zip.cc | 4 | ||||
-rw-r--r-- | google/zip_reader.cc | 4 | ||||
-rw-r--r-- | google/zip_reader.h | 13 | ||||
-rw-r--r-- | google/zip_reader_unittest.cc | 50 |
4 files changed, 43 insertions, 28 deletions
diff --git a/google/zip.cc b/google/zip.cc index 25e4fcb..88842c0 100644 --- a/google/zip.cc +++ b/google/zip.cc @@ -4,7 +4,6 @@ #include "third_party/zlib/google/zip.h" -#include <limits> #include <string> #include <vector> @@ -216,8 +215,7 @@ 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(), std::numeric_limits<uint64_t>::max())) { + if (!writer || !reader.ExtractCurrentEntry(writer.get())) { DLOG(WARNING) << "Cannot extract " << entry->path; return false; } diff --git a/google/zip_reader.cc b/google/zip_reader.cc index b134cde..dcda7da 100644 --- a/google/zip_reader.cc +++ b/google/zip_reader.cc @@ -415,8 +415,8 @@ bool ZipReader::ExtractCurrentEntryToString(uint64_t max_read_bytes, // 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(static_cast<size_t>(std::min( - base::checked_cast<int64_t>(max_read_bytes), entry_.original_size))); + 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); if (!ExtractCurrentEntry(&writer, max_read_bytes)) { diff --git a/google/zip_reader.h b/google/zip_reader.h index 1f309bd..0d81c21 100644 --- a/google/zip_reader.h +++ b/google/zip_reader.h @@ -7,6 +7,7 @@ #include <stddef.h> #include <stdint.h> +#include <limits> #include <memory> #include <string> @@ -15,6 +16,7 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/memory/weak_ptr.h" +#include "base/numerics/safe_conversions.h" #include "base/time/time.h" #if defined(USE_SYSTEM_MINIZIP) @@ -60,8 +62,7 @@ class WriterDelegate { // // while (const ZipReader::entry* entry = reader.Next()) { // auto writer = CreateFilePathWriterDelegate(extract_dir, entry->path); -// if (!reader.ExtractCurrentEntry( -// writer, std::numeric_limits<uint64_t>::max())) { +// if (!reader.ExtractCurrentEntry(writer)) { // // Cannot extract // return; // } @@ -192,7 +193,8 @@ class ZipReader { // // 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 @@ -230,6 +232,11 @@ class ZipReader { bool ExtractCurrentEntryToString(uint64_t max_read_bytes, std::string* output) const; + 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 archive. // // Precondition: one of the Open() methods returned true. diff --git a/google/zip_reader_unittest.cc b/google/zip_reader_unittest.cc index 0680e5c..5cd878d 100644 --- a/google/zip_reader_unittest.cc +++ b/google/zip_reader_unittest.cc @@ -115,8 +115,7 @@ class MockWriterDelegate : public zip::WriterDelegate { 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); } const zip::ZipReader::Entry* LocateAndOpenEntry( @@ -426,7 +425,7 @@ TEST_F(ZipReaderTest, EncryptedFile_WrongPassword) { EXPECT_FALSE(entry->is_directory); EXPECT_FALSE(entry->is_encrypted); std::string contents = "dummy"; - EXPECT_TRUE(reader.ExtractCurrentEntryToString(1000, &contents)); + EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents)); EXPECT_EQ("This is not encrypted.\n", contents); } @@ -442,7 +441,7 @@ TEST_F(ZipReaderTest, EncryptedFile_WrongPassword) { EXPECT_FALSE(entry->is_directory); EXPECT_TRUE(entry->is_encrypted); std::string contents = "dummy"; - EXPECT_FALSE(reader.ExtractCurrentEntryToString(1000, &contents)); + EXPECT_FALSE(reader.ExtractCurrentEntryToString(&contents)); EXPECT_EQ("", contents); } @@ -462,7 +461,7 @@ TEST_F(ZipReaderTest, EncryptedFile_RightPassword) { EXPECT_FALSE(entry->is_directory); EXPECT_FALSE(entry->is_encrypted); std::string contents = "dummy"; - EXPECT_TRUE(reader.ExtractCurrentEntryToString(1000, &contents)); + EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents)); EXPECT_EQ("This is not encrypted.\n", contents); } @@ -478,7 +477,7 @@ TEST_F(ZipReaderTest, EncryptedFile_RightPassword) { EXPECT_FALSE(entry->is_directory); EXPECT_TRUE(entry->is_encrypted); std::string contents = "dummy"; - EXPECT_FALSE(reader.ExtractCurrentEntryToString(1000, &contents)); + EXPECT_FALSE(reader.ExtractCurrentEntryToString(&contents)); EXPECT_EQ("", contents); } @@ -490,7 +489,7 @@ TEST_F(ZipReaderTest, EncryptedFile_RightPassword) { EXPECT_FALSE(entry->is_directory); EXPECT_TRUE(entry->is_encrypted); std::string contents = "dummy"; - EXPECT_TRUE(reader.ExtractCurrentEntryToString(1000, &contents)); + EXPECT_TRUE(reader.ExtractCurrentEntryToString(&contents)); EXPECT_EQ("This is encrypted with ZipCrypto.\n", contents); } @@ -664,10 +663,7 @@ TEST_F(ZipReaderTest, ExtractToFileAsync_WrongCrc) { ASSERT_TRUE(base::ReadFileToString(target_path, &contents)); EXPECT_EQ("This file has been changed after its CRC was computed.\n", contents); - - int64_t file_size = 0; - ASSERT_TRUE(base::GetFileSize(target_path, &file_size)); - EXPECT_EQ(file_size, listener.current_progress()); + EXPECT_EQ(contents.size(), listener.current_progress()); } // Verifies that the asynchronous extraction to a file works. @@ -729,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(); @@ -787,7 +783,7 @@ TEST_F(ZipReaderTest, ExtractPosixPermissions) { 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, 10000)); + ASSERT_TRUE(reader.ExtractCurrentEntry(&delegate)); } reader.Close(); @@ -830,8 +826,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntryPrepareFailure) { 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 @@ -847,8 +842,7 @@ TEST_F(ZipReaderTest, ExtractCurrentEntryWriteBytesFailure) { 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. @@ -865,8 +859,7 @@ 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) { @@ -876,9 +869,26 @@ TEST_F(ZipReaderTest, WrongCrc) { const ZipReader::Entry* const entry = LocateAndOpenEntry(&reader, base::FilePath::FromASCII("Corrupted.txt")); ASSERT_TRUE(entry); + std::string contents = "dummy"; - EXPECT_FALSE(reader.ExtractCurrentEntryToString(1000, &contents)); + EXPECT_FALSE(reader.ExtractCurrentEntryToString(&contents)); EXPECT_EQ("", contents); + + contents = "dummy"; + EXPECT_FALSE( + reader.ExtractCurrentEntryToString(entry->original_size + 1, &contents)); + EXPECT_EQ("", 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 { |