Merge "Replace char* with ZipEntryName in ziparchive API."

This commit is contained in:
Piotr Jastrzebski 2014-08-18 08:26:44 +00:00 committed by Gerrit Code Review
commit c0879ee9d3
4 changed files with 99 additions and 52 deletions

View file

@ -21,6 +21,7 @@
#define LIBZIPARCHIVE_ZIPARCHIVE_H_ #define LIBZIPARCHIVE_ZIPARCHIVE_H_
#include <stdint.h> #include <stdint.h>
#include <string.h>
#include <sys/types.h> #include <sys/types.h>
#include <utils/Compat.h> #include <utils/Compat.h>
@ -33,8 +34,16 @@ enum {
}; };
struct ZipEntryName { struct ZipEntryName {
const char* name; const uint8_t* name;
uint16_t name_length; 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 @@ void CloseArchive(ZipArchiveHandle handle);
* and length, a call to VerifyCrcAndLengths must be made after entry data * and length, a call to VerifyCrcAndLengths must be made after entry data
* has been processed. * has been processed.
*/ */
int32_t FindEntry(const ZipArchiveHandle handle, const char* entryName, int32_t FindEntry(const ZipArchiveHandle handle, const ZipEntryName& entryName,
ZipEntry* data); ZipEntry* data);
/* /*
@ -136,12 +145,12 @@ int32_t FindEntry(const ZipArchiveHandle handle, const char* entryName,
* EndIteration to free any allocated memory. * EndIteration to free any allocated memory.
* *
* This method also accepts an optional prefix to restrict iteration to * 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. * Returns 0 on success and negative values on failure.
*/ */
int32_t StartIteration(ZipArchiveHandle handle, void** cookie_ptr, 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. * Advance to the next element in the zipfile in iteration order.

View file

@ -31,6 +31,7 @@ LOCAL_MODULE:= libziparchive
LOCAL_C_INCLUDES += ${includes} LOCAL_C_INCLUDES += ${includes}
LOCAL_CFLAGS := -Werror LOCAL_CFLAGS := -Werror
include external/libcxx/libcxx.mk
include $(BUILD_STATIC_LIBRARY) include $(BUILD_STATIC_LIBRARY)
include $(CLEAR_VARS) include $(CLEAR_VARS)
@ -43,6 +44,7 @@ LOCAL_STATIC_LIBRARIES := libz libutils
LOCAL_MODULE:= libziparchive-host LOCAL_MODULE:= libziparchive-host
LOCAL_CFLAGS := -Werror LOCAL_CFLAGS := -Werror
LOCAL_MULTILIB := both LOCAL_MULTILIB := both
include external/libcxx/libcxx.mk
include $(BUILD_HOST_STATIC_LIBRARY) include $(BUILD_HOST_STATIC_LIBRARY)
include $(CLEAR_VARS) include $(CLEAR_VARS)

View file

@ -29,6 +29,7 @@
#include <unistd.h> #include <unistd.h>
#include <utils/Compat.h> #include <utils/Compat.h>
#include <utils/FileMap.h> #include <utils/FileMap.h>
#include <vector>
#include <zlib.h> #include <zlib.h>
#include <JNIHelp.h> // TEMP_FAILURE_RETRY may or may not be in unistd #include <JNIHelp.h> // TEMP_FAILURE_RETRY may or may not be in unistd
@ -385,8 +386,10 @@ static uint32_t RoundUpPower2(uint32_t val) {
return val; return val;
} }
static uint32_t ComputeHash(const char* str, uint16_t len) { static uint32_t ComputeHash(const ZipEntryName& name) {
uint32_t hash = 0; uint32_t hash = 0;
uint16_t len = name.name_length;
const uint8_t* str = name.name;
while (len--) { while (len--) {
hash = hash * 31 + *str++; hash = hash * 31 + *str++;
@ -401,21 +404,21 @@ static uint32_t ComputeHash(const char* str, uint16_t len) {
*/ */
static int64_t EntryToIndex(const ZipEntryName* hash_table, static int64_t EntryToIndex(const ZipEntryName* hash_table,
const uint32_t hash_table_size, const uint32_t hash_table_size,
const char* name, uint16_t length) { const ZipEntryName& name) {
const uint32_t hash = ComputeHash(name, length); const uint32_t hash = ComputeHash(name);
// NOTE: (hash_table_size - 1) is guaranteed to be non-negative. // NOTE: (hash_table_size - 1) is guaranteed to be non-negative.
uint32_t ent = hash & (hash_table_size - 1); uint32_t ent = hash & (hash_table_size - 1);
while (hash_table[ent].name != NULL) { while (hash_table[ent].name != NULL) {
if (hash_table[ent].name_length == length && if (hash_table[ent].name_length == name.name_length &&
memcmp(hash_table[ent].name, name, length) == 0) { memcmp(hash_table[ent].name, name.name, name.name_length) == 0) {
return ent; return ent;
} }
ent = (ent + 1) & (hash_table_size - 1); 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; return kEntryNotFound;
} }
@ -423,8 +426,8 @@ static int64_t EntryToIndex(const ZipEntryName* hash_table,
* Add a new entry to the hash table. * Add a new entry to the hash table.
*/ */
static int32_t AddToHash(ZipEntryName *hash_table, const uint64_t hash_table_size, static int32_t AddToHash(ZipEntryName *hash_table, const uint64_t hash_table_size,
const char* name, uint16_t length) { const ZipEntryName& name) {
const uint64_t hash = ComputeHash(name, length); const uint64_t hash = ComputeHash(name);
uint32_t ent = hash & (hash_table_size - 1); uint32_t ent = hash & (hash_table_size - 1);
/* /*
@ -432,17 +435,17 @@ static int32_t AddToHash(ZipEntryName *hash_table, const uint64_t hash_table_siz
* Further, we guarantee that the hashtable size is not 0. * Further, we guarantee that the hashtable size is not 0.
*/ */
while (hash_table[ent].name != NULL) { while (hash_table[ent].name != NULL) {
if (hash_table[ent].name_length == length && if (hash_table[ent].name_length == name.name_length &&
memcmp(hash_table[ent].name, name, length) == 0) { memcmp(hash_table[ent].name, name.name, name.name_length) == 0) {
// We've found a duplicate entry. We don't accept it // 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; return kDuplicateEntry;
} }
ent = (ent + 1) & (hash_table_size - 1); ent = (ent + 1) & (hash_table_size - 1);
} }
hash_table[ent].name = name; hash_table[ent].name = name.name;
hash_table[ent].name_length = length; hash_table[ent].name_length = name.name_length;
return 0; return 0;
} }
@ -640,9 +643,12 @@ static int32_t ParseZipArchive(ZipArchive* archive) {
const uint16_t comment_length = cdr->comment_length; const uint16_t comment_length = cdr->comment_length;
/* add the CDE filename to the hash table */ /* 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, 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) { if (add_result) {
ALOGW("Zip: Error adding entry to hash table %d", add_result); ALOGW("Zip: Error adding entry to hash table %d", add_result);
result = add_result; result = add_result;
@ -751,12 +757,11 @@ static inline ssize_t ReadAtOffset(int fd, uint8_t* buf, size_t len,
static int32_t FindEntry(const ZipArchive* archive, const int ent, static int32_t FindEntry(const ZipArchive* archive, const int ent,
ZipEntry* data) { ZipEntry* data) {
const uint16_t nameLen = archive->hash_table[ent].name_length; 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 // Recover the start of the central directory entry from the filename
// pointer. The filename is the first entry past the fixed-size data, // pointer. The filename is the first entry past the fixed-size data,
// so we can just subtract back from that. // 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); ptr -= sizeof(CentralDirectoryRecord);
// This is the base of our mmapped region, we have to sanity check that // This is the base of our mmapped region, we have to sanity check that
@ -847,7 +852,7 @@ static int32_t FindEntry(const ZipArchive* archive, const int ent,
return kIoError; return kIoError;
} }
if (memcmp(name, name_buf, nameLen)) { if (memcmp(archive->hash_table[ent].name, name_buf, nameLen)) {
free(name_buf); free(name_buf);
return kInconsistentInformation; return kInconsistentInformation;
} }
@ -884,12 +889,12 @@ static int32_t FindEntry(const ZipArchive* archive, const int ent,
struct IterationHandle { struct IterationHandle {
uint32_t position; uint32_t position;
const char* prefix; std::vector<uint8_t> prefix;
uint16_t prefix_len;
ZipArchive* archive; 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; ZipArchive* archive = (ZipArchive *) handle;
if (archive == NULL || archive->hash_table == NULL) { if (archive == NULL || archive->hash_table == NULL) {
@ -897,14 +902,13 @@ int32_t StartIteration(ZipArchiveHandle handle, void** cookie_ptr, const char* p
return kInvalidHandle; return kInvalidHandle;
} }
IterationHandle* cookie = (IterationHandle*) malloc(sizeof(IterationHandle)); IterationHandle* cookie = new IterationHandle();
cookie->position = 0; cookie->position = 0;
cookie->archive = archive; cookie->archive = archive;
if (prefix != NULL) { if (optional_prefix != NULL) {
cookie->prefix = strdup(prefix); cookie->prefix.insert(cookie->prefix.begin(),
cookie->prefix_len = strlen(prefix); optional_prefix->name,
} else { optional_prefix->name + optional_prefix->name_length);
cookie->prefix = NULL;
} }
*cookie_ptr = cookie ; *cookie_ptr = cookie ;
@ -912,29 +916,22 @@ int32_t StartIteration(ZipArchiveHandle handle, void** cookie_ptr, const char* p
} }
void EndIteration(void* cookie) { void EndIteration(void* cookie) {
if (cookie != NULL) { delete reinterpret_cast<IterationHandle*>(cookie);
IterationHandle* handle = reinterpret_cast<IterationHandle*>(cookie);
if (handle->prefix != NULL) {
free(const_cast<char*>(handle->prefix));
}
free(cookie);
}
} }
int32_t FindEntry(const ZipArchiveHandle handle, const char* entryName, int32_t FindEntry(const ZipArchiveHandle handle, const ZipEntryName& entryName,
ZipEntry* data) { ZipEntry* data) {
const ZipArchive* archive = (ZipArchive*) handle; const ZipArchive* archive = (ZipArchive*) handle;
const int nameLen = strlen(entryName); if (entryName.name_length == 0) {
if (nameLen == 0 || nameLen > 65535) { ALOGW("Zip: Invalid filename %.*s", entryName.name_length, entryName.name);
ALOGW("Zip: Invalid filename %s", entryName);
return kInvalidEntryName; return kInvalidEntryName;
} }
const int64_t ent = EntryToIndex(archive->hash_table, const int64_t ent = EntryToIndex(archive->hash_table,
archive->hash_table_size, entryName, nameLen); archive->hash_table_size, entryName);
if (ent < 0) { 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; return ent;
} }
@ -959,8 +956,8 @@ int32_t Next(void* cookie, ZipEntry* data, ZipEntryName* name) {
for (uint32_t i = currentOffset; i < hash_table_length; ++i) { for (uint32_t i = currentOffset; i < hash_table_length; ++i) {
if (hash_table[i].name != NULL && if (hash_table[i].name != NULL &&
(handle->prefix == NULL || (handle->prefix.empty() ||
(memcmp(handle->prefix, hash_table[i].name, handle->prefix_len) == 0))) { (memcmp(&(handle->prefix[0]), hash_table[i].name, handle->prefix.size()) == 0))) {
handle->position = (i + 1); handle->position = (i + 1);
const int error = FindEntry(archive, i, data); const int error = FindEntry(archive, i, data);
if (!error) { if (!error) {

View file

@ -40,6 +40,27 @@ static const uint8_t kBTxtContents[] = {
'\n' '\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, static int32_t OpenArchiveWrapper(const std::string& name,
ZipArchiveHandle* handle) { ZipArchiveHandle* handle) {
const std::string abs_path = test_data_dir + "/" + name; const std::string abs_path = test_data_dir + "/" + name;
@ -108,7 +129,10 @@ TEST(ziparchive, FindEntry) {
ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle)); ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));
ZipEntry data; 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. // Known facts about a.txt, from zipinfo -v.
ASSERT_EQ(63, data.offset); ASSERT_EQ(63, data.offset);
@ -118,7 +142,10 @@ TEST(ziparchive, FindEntry) {
ASSERT_EQ(0x950821c5, data.crc32); ASSERT_EQ(0x950821c5, data.crc32);
// An entry that doesn't exist. Should be a negative return code. // 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); CloseArchive(handle);
} }
@ -129,7 +156,10 @@ TEST(ziparchive, ExtractToMemory) {
// An entry that's deflated. // An entry that's deflated.
ZipEntry data; 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; const uint32_t a_size = data.uncompressed_length;
ASSERT_EQ(a_size, sizeof(kATxtContents)); ASSERT_EQ(a_size, sizeof(kATxtContents));
uint8_t* buffer = new uint8_t[a_size]; uint8_t* buffer = new uint8_t[a_size];
@ -138,7 +168,10 @@ TEST(ziparchive, ExtractToMemory) {
delete[] buffer; delete[] buffer;
// An entry that's stored. // 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; const uint32_t b_size = data.uncompressed_length;
ASSERT_EQ(b_size, sizeof(kBTxtContents)); ASSERT_EQ(b_size, sizeof(kBTxtContents));
buffer = new uint8_t[b_size]; buffer = new uint8_t[b_size];
@ -184,7 +217,10 @@ TEST(ziparchive, EmptyEntries) {
ASSERT_EQ(0, OpenArchiveFd(fd, "EmptyEntriesTest", &handle)); ASSERT_EQ(0, OpenArchiveFd(fd, "EmptyEntriesTest", &handle));
ZipEntry entry; 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); ASSERT_EQ(static_cast<uint32_t>(0), entry.uncompressed_length);
uint8_t buffer[1]; uint8_t buffer[1];
ASSERT_EQ(0, ExtractToMemory(handle, &entry, buffer, 1)); ASSERT_EQ(0, ExtractToMemory(handle, &entry, buffer, 1));
@ -231,7 +267,10 @@ TEST(ziparchive, ExtractToFile) {
ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle)); ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));
ZipEntry entry; 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)); ASSERT_EQ(0, ExtractEntryToFile(handle, &entry, fd));