Replace char* with ZipEntryName in ziparchive API.
It's important because entry names can be encoded in UTF-8 and can have \0
character in the middle.
Use vector instead of char* for prefix in IterationHandle.
Bug: 16162465
Change-Id: Ie34c8d7c6231cc258530c22bdde5542895213649
diff --git a/include/ziparchive/zip_archive.h b/include/ziparchive/zip_archive.h
index 27a9796..7da6e84 100644
--- a/include/ziparchive/zip_archive.h
+++ b/include/ziparchive/zip_archive.h
@@ -21,6 +21,7 @@
#define LIBZIPARCHIVE_ZIPARCHIVE_H_
#include <stdint.h>
+#include <string.h>
#include <sys/types.h>
#include <utils/Compat.h>
@@ -33,8 +34,16 @@
};
struct ZipEntryName {
- const char* name;
+ const uint8_t* name;
uint16_t name_length;
+
+ ZipEntryName() {}
+
+ /*
+ * entry_name has to be an c-style string with only ASCII characters.
+ */
+ explicit ZipEntryName(const char* entry_name)
+ : name(reinterpret_cast<const uint8_t*>(entry_name)), name_length(strlen(entry_name)) {}
};
/*
@@ -124,7 +133,7 @@
* and length, a call to VerifyCrcAndLengths must be made after entry data
* has been processed.
*/
-int32_t FindEntry(const ZipArchiveHandle handle, const char* entryName,
+int32_t FindEntry(const ZipArchiveHandle handle, const ZipEntryName& entryName,
ZipEntry* data);
/*
@@ -136,12 +145,12 @@
* EndIteration to free any allocated memory.
*
* This method also accepts an optional prefix to restrict iteration to
- * entry names that start with |prefix|.
+ * entry names that start with |optional_prefix|.
*
* Returns 0 on success and negative values on failure.
*/
int32_t StartIteration(ZipArchiveHandle handle, void** cookie_ptr,
- const char* prefix);
+ const ZipEntryName* optional_prefix);
/*
* Advance to the next element in the zipfile in iteration order.
diff --git a/libziparchive/Android.mk b/libziparchive/Android.mk
index d96bc63..684c635 100644
--- a/libziparchive/Android.mk
+++ b/libziparchive/Android.mk
@@ -31,6 +31,7 @@
LOCAL_C_INCLUDES += ${includes}
LOCAL_CFLAGS := -Werror
+include external/libcxx/libcxx.mk
include $(BUILD_STATIC_LIBRARY)
include $(CLEAR_VARS)
@@ -43,6 +44,7 @@
LOCAL_MODULE:= libziparchive-host
LOCAL_CFLAGS := -Werror
LOCAL_MULTILIB := both
+include external/libcxx/libcxx.mk
include $(BUILD_HOST_STATIC_LIBRARY)
include $(CLEAR_VARS)
diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index a29eb79..cbe1b14 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -29,6 +29,7 @@
#include <unistd.h>
#include <utils/Compat.h>
#include <utils/FileMap.h>
+#include <vector>
#include <zlib.h>
#include <JNIHelp.h> // TEMP_FAILURE_RETRY may or may not be in unistd
@@ -385,8 +386,10 @@
return val;
}
-static uint32_t ComputeHash(const char* str, uint16_t len) {
+static uint32_t ComputeHash(const ZipEntryName& name) {
uint32_t hash = 0;
+ uint16_t len = name.name_length;
+ const uint8_t* str = name.name;
while (len--) {
hash = hash * 31 + *str++;
@@ -401,21 +404,21 @@
*/
static int64_t EntryToIndex(const ZipEntryName* hash_table,
const uint32_t hash_table_size,
- const char* name, uint16_t length) {
- const uint32_t hash = ComputeHash(name, length);
+ const ZipEntryName& name) {
+ const uint32_t hash = ComputeHash(name);
// NOTE: (hash_table_size - 1) is guaranteed to be non-negative.
uint32_t ent = hash & (hash_table_size - 1);
while (hash_table[ent].name != NULL) {
- if (hash_table[ent].name_length == length &&
- memcmp(hash_table[ent].name, name, length) == 0) {
+ if (hash_table[ent].name_length == name.name_length &&
+ memcmp(hash_table[ent].name, name.name, name.name_length) == 0) {
return ent;
}
ent = (ent + 1) & (hash_table_size - 1);
}
- ALOGV("Zip: Unable to find entry %.*s", length, name);
+ ALOGV("Zip: Unable to find entry %.*s", name.name_length, name.name);
return kEntryNotFound;
}
@@ -423,8 +426,8 @@
* Add a new entry to the hash table.
*/
static int32_t AddToHash(ZipEntryName *hash_table, const uint64_t hash_table_size,
- const char* name, uint16_t length) {
- const uint64_t hash = ComputeHash(name, length);
+ const ZipEntryName& name) {
+ const uint64_t hash = ComputeHash(name);
uint32_t ent = hash & (hash_table_size - 1);
/*
@@ -432,17 +435,17 @@
* Further, we guarantee that the hashtable size is not 0.
*/
while (hash_table[ent].name != NULL) {
- if (hash_table[ent].name_length == length &&
- memcmp(hash_table[ent].name, name, length) == 0) {
+ if (hash_table[ent].name_length == name.name_length &&
+ memcmp(hash_table[ent].name, name.name, name.name_length) == 0) {
// We've found a duplicate entry. We don't accept it
- ALOGW("Zip: Found duplicate entry %.*s", length, name);
+ ALOGW("Zip: Found duplicate entry %.*s", name.name_length, name.name);
return kDuplicateEntry;
}
ent = (ent + 1) & (hash_table_size - 1);
}
- hash_table[ent].name = name;
- hash_table[ent].name_length = length;
+ hash_table[ent].name = name.name;
+ hash_table[ent].name_length = name.name_length;
return 0;
}
@@ -640,9 +643,12 @@
const uint16_t comment_length = cdr->comment_length;
/* add the CDE filename to the hash table */
- const char* file_name = reinterpret_cast<const char *>(ptr + sizeof(CentralDirectoryRecord));
+ const uint8_t* file_name = ptr + sizeof(CentralDirectoryRecord);
+ ZipEntryName entry_name;
+ entry_name.name = file_name;
+ entry_name.name_length = file_name_length;
const int add_result = AddToHash(archive->hash_table,
- archive->hash_table_size, file_name, file_name_length);
+ archive->hash_table_size, entry_name);
if (add_result) {
ALOGW("Zip: Error adding entry to hash table %d", add_result);
result = add_result;
@@ -751,12 +757,11 @@
static int32_t FindEntry(const ZipArchive* archive, const int ent,
ZipEntry* data) {
const uint16_t nameLen = archive->hash_table[ent].name_length;
- const char* name = archive->hash_table[ent].name;
// Recover the start of the central directory entry from the filename
// pointer. The filename is the first entry past the fixed-size data,
// so we can just subtract back from that.
- const uint8_t* ptr = reinterpret_cast<const uint8_t*>(name);
+ const uint8_t* ptr = archive->hash_table[ent].name;
ptr -= sizeof(CentralDirectoryRecord);
// This is the base of our mmapped region, we have to sanity check that
@@ -847,7 +852,7 @@
return kIoError;
}
- if (memcmp(name, name_buf, nameLen)) {
+ if (memcmp(archive->hash_table[ent].name, name_buf, nameLen)) {
free(name_buf);
return kInconsistentInformation;
}
@@ -884,12 +889,12 @@
struct IterationHandle {
uint32_t position;
- const char* prefix;
- uint16_t prefix_len;
+ std::vector<uint8_t> prefix;
ZipArchive* archive;
};
-int32_t StartIteration(ZipArchiveHandle handle, void** cookie_ptr, const char* prefix) {
+int32_t StartIteration(ZipArchiveHandle handle, void** cookie_ptr,
+ const ZipEntryName* optional_prefix) {
ZipArchive* archive = (ZipArchive *) handle;
if (archive == NULL || archive->hash_table == NULL) {
@@ -897,14 +902,13 @@
return kInvalidHandle;
}
- IterationHandle* cookie = (IterationHandle*) malloc(sizeof(IterationHandle));
+ IterationHandle* cookie = new IterationHandle();
cookie->position = 0;
cookie->archive = archive;
- if (prefix != NULL) {
- cookie->prefix = strdup(prefix);
- cookie->prefix_len = strlen(prefix);
- } else {
- cookie->prefix = NULL;
+ if (optional_prefix != NULL) {
+ cookie->prefix.insert(cookie->prefix.begin(),
+ optional_prefix->name,
+ optional_prefix->name + optional_prefix->name_length);
}
*cookie_ptr = cookie ;
@@ -912,29 +916,22 @@
}
void EndIteration(void* cookie) {
- if (cookie != NULL) {
- IterationHandle* handle = reinterpret_cast<IterationHandle*>(cookie);
- if (handle->prefix != NULL) {
- free(const_cast<char*>(handle->prefix));
- }
- free(cookie);
- }
+ delete reinterpret_cast<IterationHandle*>(cookie);
}
-int32_t FindEntry(const ZipArchiveHandle handle, const char* entryName,
+int32_t FindEntry(const ZipArchiveHandle handle, const ZipEntryName& entryName,
ZipEntry* data) {
const ZipArchive* archive = (ZipArchive*) handle;
- const int nameLen = strlen(entryName);
- if (nameLen == 0 || nameLen > 65535) {
- ALOGW("Zip: Invalid filename %s", entryName);
+ if (entryName.name_length == 0) {
+ ALOGW("Zip: Invalid filename %.*s", entryName.name_length, entryName.name);
return kInvalidEntryName;
}
const int64_t ent = EntryToIndex(archive->hash_table,
- archive->hash_table_size, entryName, nameLen);
+ archive->hash_table_size, entryName);
if (ent < 0) {
- ALOGV("Zip: Could not find entry %.*s", nameLen, entryName);
+ ALOGV("Zip: Could not find entry %.*s", entryName.name_length, entryName.name);
return ent;
}
@@ -959,8 +956,8 @@
for (uint32_t i = currentOffset; i < hash_table_length; ++i) {
if (hash_table[i].name != NULL &&
- (handle->prefix == NULL ||
- (memcmp(handle->prefix, hash_table[i].name, handle->prefix_len) == 0))) {
+ (handle->prefix.empty() ||
+ (memcmp(&(handle->prefix[0]), hash_table[i].name, handle->prefix.size()) == 0))) {
handle->position = (i + 1);
const int error = FindEntry(archive, i, data);
if (!error) {
diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc
index 813a87f..4775de0 100644
--- a/libziparchive/zip_archive_test.cc
+++ b/libziparchive/zip_archive_test.cc
@@ -40,6 +40,27 @@
'\n'
};
+static const uint16_t kATxtNameLength = 5;
+static const uint16_t kBTxtNameLength = 5;
+static const uint16_t kNonexistentTxtNameLength = 15;
+static const uint16_t kEmptyTxtNameLength = 9;
+
+static const uint8_t kATxtName[kATxtNameLength] = {
+ 'a', '.', 't', 'x', 't'
+};
+
+static const uint8_t kBTxtName[kBTxtNameLength] = {
+ 'b', '.', 't', 'x', 't'
+};
+
+static const uint8_t kNonexistentTxtName[kNonexistentTxtNameLength] = {
+ 'n', 'o', 'n', 'e', 'x', 'i', 's', 't', 'e', 'n', 't', '.', 't', 'x' ,'t'
+};
+
+static const uint8_t kEmptyTxtName[kEmptyTxtNameLength] = {
+ 'e', 'm', 'p', 't', 'y', '.', 't', 'x', 't'
+};
+
static int32_t OpenArchiveWrapper(const std::string& name,
ZipArchiveHandle* handle) {
const std::string abs_path = test_data_dir + "/" + name;
@@ -108,7 +129,10 @@
ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));
ZipEntry data;
- ASSERT_EQ(0, FindEntry(handle, "a.txt", &data));
+ ZipEntryName name;
+ name.name = kATxtName;
+ name.name_length = kATxtNameLength;
+ ASSERT_EQ(0, FindEntry(handle, name, &data));
// Known facts about a.txt, from zipinfo -v.
ASSERT_EQ(63, data.offset);
@@ -118,7 +142,10 @@
ASSERT_EQ(0x950821c5, data.crc32);
// An entry that doesn't exist. Should be a negative return code.
- ASSERT_LT(FindEntry(handle, "nonexistent.txt", &data), 0);
+ ZipEntryName absent_name;
+ absent_name.name = kNonexistentTxtName;
+ absent_name.name_length = kNonexistentTxtNameLength;
+ ASSERT_LT(FindEntry(handle, absent_name, &data), 0);
CloseArchive(handle);
}
@@ -129,7 +156,10 @@
// An entry that's deflated.
ZipEntry data;
- ASSERT_EQ(0, FindEntry(handle, "a.txt", &data));
+ ZipEntryName a_name;
+ a_name.name = kATxtName;
+ a_name.name_length = kATxtNameLength;
+ ASSERT_EQ(0, FindEntry(handle, a_name, &data));
const uint32_t a_size = data.uncompressed_length;
ASSERT_EQ(a_size, sizeof(kATxtContents));
uint8_t* buffer = new uint8_t[a_size];
@@ -138,7 +168,10 @@
delete[] buffer;
// An entry that's stored.
- ASSERT_EQ(0, FindEntry(handle, "b.txt", &data));
+ ZipEntryName b_name;
+ b_name.name = kBTxtName;
+ b_name.name_length = kBTxtNameLength;
+ ASSERT_EQ(0, FindEntry(handle, b_name, &data));
const uint32_t b_size = data.uncompressed_length;
ASSERT_EQ(b_size, sizeof(kBTxtContents));
buffer = new uint8_t[b_size];
@@ -184,7 +217,10 @@
ASSERT_EQ(0, OpenArchiveFd(fd, "EmptyEntriesTest", &handle));
ZipEntry entry;
- ASSERT_EQ(0, FindEntry(handle, "empty.txt", &entry));
+ ZipEntryName empty_name;
+ empty_name.name = kEmptyTxtName;
+ empty_name.name_length = kEmptyTxtNameLength;
+ ASSERT_EQ(0, FindEntry(handle, empty_name, &entry));
ASSERT_EQ(static_cast<uint32_t>(0), entry.uncompressed_length);
uint8_t buffer[1];
ASSERT_EQ(0, ExtractToMemory(handle, &entry, buffer, 1));
@@ -231,7 +267,10 @@
ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));
ZipEntry entry;
- ASSERT_EQ(0, FindEntry(handle, "a.txt", &entry));
+ ZipEntryName name;
+ name.name = kATxtName;
+ name.name_length = kATxtNameLength;
+ ASSERT_EQ(0, FindEntry(handle, name, &entry));
ASSERT_EQ(0, ExtractEntryToFile(handle, &entry, fd));