diff --git a/libziparchive/include/ziparchive/zip_archive.h b/libziparchive/include/ziparchive/zip_archive.h index 6e837a430..7eead7e43 100644 --- a/libziparchive/include/ziparchive/zip_archive.h +++ b/libziparchive/include/ziparchive/zip_archive.h @@ -177,7 +177,10 @@ int32_t FindEntry(const ZipArchiveHandle archive, const std::string_view entryNa * * Returns 0 on success and negative values on failure. */ -// TODO: switch these ZipStrings to std::string_view. +int32_t StartIteration(ZipArchiveHandle archive, void** cookie_ptr, + const std::string_view optional_prefix = "", + const std::string_view optional_suffix = ""); +// TODO: remove this. int32_t StartIteration(ZipArchiveHandle archive, void** cookie_ptr, const ZipString* optional_prefix, const ZipString* optional_suffix); diff --git a/libziparchive/unzip.cpp b/libziparchive/unzip.cpp index c6def7329..3a3a694b9 100644 --- a/libziparchive/unzip.cpp +++ b/libziparchive/unzip.cpp @@ -249,7 +249,7 @@ static void ProcessAll(ZipArchiveHandle zah) { // libziparchive iteration order doesn't match the central directory. // We could sort, but that would cost extra and wouldn't match either. void* cookie; - int err = StartIteration(zah, &cookie, nullptr, nullptr); + int err = StartIteration(zah, &cookie); if (err != 0) { error(1, 0, "couldn't iterate %s: %s", archive_name, ErrorCodeString(err)); } diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc index 2bd8fb92a..ac3e23633 100644 --- a/libziparchive/zip_archive.cc +++ b/libziparchive/zip_archive.cc @@ -689,54 +689,59 @@ static int32_t FindEntry(const ZipArchive* archive, const int32_t ent, ZipEntry* } struct IterationHandle { - uint32_t position; - // TODO: switch these to std::string now that Windows uses libc++ too. - ZipString prefix; - ZipString suffix; ZipArchive* archive; - IterationHandle(const ZipString* in_prefix, const ZipString* in_suffix) { - if (in_prefix) { - uint8_t* name_copy = new uint8_t[in_prefix->name_length]; - memcpy(name_copy, in_prefix->name, in_prefix->name_length); - prefix.name = name_copy; - prefix.name_length = in_prefix->name_length; - } else { - prefix.name = NULL; - prefix.name_length = 0; - } - if (in_suffix) { - uint8_t* name_copy = new uint8_t[in_suffix->name_length]; - memcpy(name_copy, in_suffix->name, in_suffix->name_length); - suffix.name = name_copy; - suffix.name_length = in_suffix->name_length; - } else { - suffix.name = NULL; - suffix.name_length = 0; - } - } + std::string prefix_holder; + ZipString prefix; - ~IterationHandle() { - delete[] prefix.name; - delete[] suffix.name; - } + std::string suffix_holder; + ZipString suffix; + + uint32_t position = 0; + + IterationHandle(ZipArchive* archive, const std::string_view in_prefix, + const std::string_view in_suffix) + : archive(archive), + prefix_holder(in_prefix), + prefix(prefix_holder), + suffix_holder(in_suffix), + suffix(suffix_holder) {} }; int32_t StartIteration(ZipArchiveHandle archive, void** cookie_ptr, - const ZipString* optional_prefix, const ZipString* optional_suffix) { + const std::string_view optional_prefix, + const std::string_view optional_suffix) { if (archive == NULL || archive->hash_table == NULL) { ALOGW("Zip: Invalid ZipArchiveHandle"); return kInvalidHandle; } - IterationHandle* cookie = new IterationHandle(optional_prefix, optional_suffix); - cookie->position = 0; - cookie->archive = archive; + if (optional_prefix.size() > static_cast(UINT16_MAX) || + optional_suffix.size() > static_cast(UINT16_MAX)) { + ALOGW("Zip: prefix/suffix too long"); + return kInvalidEntryName; + } - *cookie_ptr = cookie; + *cookie_ptr = new IterationHandle(archive, optional_prefix, optional_suffix); return 0; } +// TODO: remove this. +int32_t StartIteration(ZipArchiveHandle archive, void** cookie_ptr, + const ZipString* optional_prefix, const ZipString* optional_suffix) { + std::string prefix; + if (optional_prefix) { + prefix = std::string(reinterpret_cast(optional_prefix->name), + optional_prefix->name_length); + } + std::string suffix; + if (optional_suffix) { + suffix = std::string(reinterpret_cast(optional_suffix->name), + optional_suffix->name_length); + } + return StartIteration(archive, cookie_ptr, prefix.c_str(), suffix.c_str()); +} + void EndIteration(void* cookie) { delete reinterpret_cast(cookie); } diff --git a/libziparchive/zip_archive_benchmark.cpp b/libziparchive/zip_archive_benchmark.cpp index 52166a479..434f2e1ef 100644 --- a/libziparchive/zip_archive_benchmark.cpp +++ b/libziparchive/zip_archive_benchmark.cpp @@ -75,7 +75,7 @@ static void Iterate_all_files(benchmark::State& state) { while (state.KeepRunning()) { OpenArchive(temp_file->path, &handle); - StartIteration(handle, &iteration_cookie, nullptr, nullptr); + StartIteration(handle, &iteration_cookie); while (Next(iteration_cookie, &data, &name) == 0) { } EndIteration(iteration_cookie); diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc index cfbce1cb7..993c975ec 100644 --- a/libziparchive/zip_archive_test.cc +++ b/libziparchive/zip_archive_test.cc @@ -107,7 +107,7 @@ TEST(ziparchive, OpenDoNotAssumeFdOwnership) { close(fd); } -static void AssertIterationOrder(const ZipString* prefix, const ZipString* suffix, +static void AssertIterationOrder(const std::string_view prefix, const std::string_view suffix, const std::vector& expected_names_sorted) { ZipArchiveHandle handle; ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle)); @@ -137,30 +137,26 @@ TEST(ziparchive, Iteration) { static const std::vector kExpectedMatchesSorted = {"a.txt", "b.txt", "b/", "b/c.txt", "b/d.txt"}; - AssertIterationOrder(nullptr, nullptr, kExpectedMatchesSorted); + AssertIterationOrder("", "", kExpectedMatchesSorted); } TEST(ziparchive, IterationWithPrefix) { - ZipString prefix("b/"); static const std::vector kExpectedMatchesSorted = {"b/", "b/c.txt", "b/d.txt"}; - AssertIterationOrder(&prefix, nullptr, kExpectedMatchesSorted); + AssertIterationOrder("b/", "", kExpectedMatchesSorted); } TEST(ziparchive, IterationWithSuffix) { - ZipString suffix(".txt"); static const std::vector kExpectedMatchesSorted = {"a.txt", "b.txt", "b/c.txt", "b/d.txt"}; - AssertIterationOrder(nullptr, &suffix, kExpectedMatchesSorted); + AssertIterationOrder("", ".txt", kExpectedMatchesSorted); } TEST(ziparchive, IterationWithPrefixAndSuffix) { - ZipString prefix("b"); - ZipString suffix(".txt"); static const std::vector kExpectedMatchesSorted = {"b.txt", "b/c.txt", "b/d.txt"}; - AssertIterationOrder(&prefix, &suffix, kExpectedMatchesSorted); + AssertIterationOrder("b", ".txt", kExpectedMatchesSorted); } TEST(ziparchive, IterationWithBadPrefixAndSuffix) { @@ -168,9 +164,7 @@ TEST(ziparchive, IterationWithBadPrefixAndSuffix) { ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle)); void* iteration_cookie; - ZipString prefix("x"); - ZipString suffix("y"); - ASSERT_EQ(0, StartIteration(handle, &iteration_cookie, &prefix, &suffix)); + ASSERT_EQ(0, StartIteration(handle, &iteration_cookie, "x", "y")); ZipEntry data; ZipString name; @@ -228,7 +222,7 @@ TEST(ziparchive, TestInvalidDeclaredLength) { ASSERT_EQ(0, OpenArchiveWrapper("declaredlength.zip", &handle)); void* iteration_cookie; - ASSERT_EQ(0, StartIteration(handle, &iteration_cookie, nullptr, nullptr)); + ASSERT_EQ(0, StartIteration(handle, &iteration_cookie)); ZipString name; ZipEntry data;