Fix ExtractEntryToFile.
We would always write uncompressed data at offset 0 instead
of the current filedescriptor offset.
Also adds a unit-test & a clarifying comment on the API.
Change-Id: If44757e96dde504ce63d81b4dec7115fc6f6d5fb
diff --git a/include/ziparchive/zip_archive.h b/include/ziparchive/zip_archive.h
index ac3d792..1877494 100644
--- a/include/ziparchive/zip_archive.h
+++ b/include/ziparchive/zip_archive.h
@@ -152,7 +152,10 @@
int32_t Next(void* cookie, ZipEntry* data, ZipEntryName *name);
/*
- * Uncompress and write an entry to a file descriptor.
+ * Uncompress and write an entry to an open file identified by |fd|.
+ * |entry->uncompressed_length| bytes will be written to the file at
+ * its current offset, and the file will be truncated at the end of
+ * the uncompressed data.
*
* Returns 0 on success and negative values on failure.
*/
diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index dddff1f..2b827b3 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -1007,13 +1007,21 @@
ZipEntry* entry, int fd) {
const int32_t declared_length = entry->uncompressed_length;
- int result = TEMP_FAILURE_RETRY(ftruncate(fd, declared_length));
- if (result == -1) {
- ALOGW("Zip: unable to truncate file to %ud", declared_length);
+ const off64_t current_offset = lseek64(fd, 0, SEEK_CUR);
+ if (current_offset == -1) {
+ ALOGW("Zip: unable to seek to current location on fd %d: %s", fd,
+ strerror(errno));
return kIoError;
}
- android::FileMap* map = MapFileSegment(fd, 0, declared_length,
+ int result = TEMP_FAILURE_RETRY(ftruncate(fd, declared_length + current_offset));
+ if (result == -1) {
+ ALOGW("Zip: unable to truncate file to %lld: %s", declared_length + current_offset,
+ strerror(errno));
+ return kIoError;
+ }
+
+ android::FileMap* map = MapFileSegment(fd, current_offset, declared_length,
false, kTempMappingFileName);
if (map == NULL) {
return kMmapFailed;
diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc
index 55cb755..022be5b 100644
--- a/libziparchive/zip_archive_test.cc
+++ b/libziparchive/zip_archive_test.cc
@@ -16,8 +16,12 @@
#include "ziparchive/zip_archive.h"
-#include "getopt.h"
+#include <errno.h>
+#include <getopt.h>
#include <stdio.h>
+#include <unistd.h>
+#include <vector>
+
#include <gtest/gtest.h>
static std::string test_data_dir;
@@ -136,6 +140,43 @@
CloseArchive(handle);
}
+TEST(ziparchive, ExtractToFile) {
+ char kTempFilePattern[] = "zip_archive_test_XXXXXX";
+ int fd = mkstemp(kTempFilePattern);
+ ASSERT_NE(-1, fd);
+ const uint8_t data[8] = { '1', '2', '3', '4', '5', '6', '7', '8' };
+ const ssize_t data_size = sizeof(data);
+
+ ASSERT_EQ(data_size, TEMP_FAILURE_RETRY(write(fd, data, data_size)));
+
+ ZipArchiveHandle handle;
+ ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));
+
+ ZipEntry entry;
+ ASSERT_EQ(0, FindEntry(handle, "a.txt", &entry));
+ ASSERT_EQ(0, ExtractEntryToFile(handle, &entry, fd));
+
+
+ // Assert that the first 8 bytes of the file haven't been clobbered.
+ uint8_t read_buffer[data_size];
+ ASSERT_EQ(0, lseek64(fd, 0, SEEK_SET));
+ ASSERT_EQ(data_size, TEMP_FAILURE_RETRY(read(fd, read_buffer, data_size)));
+ ASSERT_EQ(0, memcmp(read_buffer, data, data_size));
+
+ // Assert that the remainder of the file contains the incompressed data.
+ std::vector<uint8_t> uncompressed_data(entry.uncompressed_length);
+ ASSERT_EQ(static_cast<ssize_t>(entry.uncompressed_length),
+ TEMP_FAILURE_RETRY(
+ read(fd, &uncompressed_data[0], entry.uncompressed_length)));
+ ASSERT_EQ(0, memcmp(&uncompressed_data[0], kATxtContents,
+ sizeof(kATxtContents)));
+
+ // Assert that the total length of the file is sane
+ ASSERT_EQ(data_size + sizeof(kATxtContents), lseek64(fd, 0, SEEK_END));
+
+ close(fd);
+}
+
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);