diff options
-rw-r--r-- | google/zip.cc | 4 | ||||
-rw-r--r-- | google/zip.h | 4 | ||||
-rw-r--r-- | google/zip_reader.cc | 4 | ||||
-rw-r--r-- | google/zip_reader.h | 9 | ||||
-rw-r--r-- | google/zip_unittest.cc | 23 |
5 files changed, 28 insertions, 16 deletions
diff --git a/google/zip.cc b/google/zip.cc index de9cdce..7f9af5b 100644 --- a/google/zip.cc +++ b/google/zip.cc @@ -10,6 +10,7 @@ #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" @@ -180,6 +181,9 @@ bool Unzip(const base::FilePath& src_file, return false; } + 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), diff --git a/google/zip.h b/google/zip.h index 6980d03..621f0d9 100644 --- a/google/zip.h +++ b/google/zip.h @@ -198,7 +198,9 @@ bool Unzip(const base::PlatformFile& zip_file, UnzipOptions options = {}); // Unzips the contents of |zip_file| into |dest_dir|. -// WARNING: This can overwrite existing files in |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 = {}); diff --git a/google/zip_reader.cc b/google/zip_reader.cc index d2f86a4..93fe134 100644 --- a/google/zip_reader.cc +++ b/google/zip_reader.cc @@ -550,8 +550,8 @@ bool FilePathWriterDelegate::PrepareOutput() { return false; } - owned_file_.Initialize(output_file_path_, base::File::FLAG_CREATE_ALWAYS | - base::File::FLAG_WRITE); + 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()); diff --git a/google/zip_reader.h b/google/zip_reader.h index 6ca9cd9..edf548b 100644 --- a/google/zip_reader.h +++ b/google/zip_reader.h @@ -326,7 +326,8 @@ class FileWriterDelegate : public WriterDelegate { int64_t file_length_ = 0; }; -// A writer delegate that writes a file at a given path. +// 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(base::FilePath output_file_path); @@ -336,10 +337,12 @@ class FilePathWriterDelegate : public FileWriterDelegate { ~FilePathWriterDelegate() override; - // Creates the output file and any necessary intermediate directories. + // 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; - // Deletes the file. + // Deletes the output file. void OnError() override; private: diff --git a/google/zip_unittest.cc b/google/zip_unittest.cc index 935fddd..cf54991 100644 --- a/google/zip_unittest.cc +++ b/google/zip_unittest.cc @@ -517,7 +517,7 @@ TEST_F(ZipTest, UnzipRepeatedDirName) { } TEST_F(ZipTest, UnzipRepeatedFileName) { - EXPECT_TRUE(zip::Unzip( + EXPECT_FALSE(zip::Unzip( GetDataDirectory().AppendASCII("Repeated File Name.zip"), test_dir_)); EXPECT_THAT( @@ -527,7 +527,7 @@ TEST_F(ZipTest, UnzipRepeatedFileName) { std::string contents; EXPECT_TRUE( base::ReadFileToString(test_dir_.AppendASCII("repeated"), &contents)); - EXPECT_EQ("Second file", contents); + EXPECT_EQ("First file", contents); } TEST_F(ZipTest, UnzipCannotCreateEmptyDir) { @@ -602,22 +602,25 @@ TEST_F(ZipTest, UnzipWindowsSpecialNames) { } TEST_F(ZipTest, UnzipDifferentCases) { - EXPECT_TRUE(zip::Unzip(GetDataDirectory().AppendASCII( - "Repeated File Name With Different Cases.zip"), - test_dir_)); - #if defined(OS_WIN) || defined(OS_MAC) - // There is only one file, with the name of the first file but the contents of - // the last file. + // 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("Upper case 3", contents); + EXPECT_EQ("Mixed case 111", contents); #else - // All the files are correctly extracted. + // 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")); |