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

3 files changed, 57 insertions, 5 deletions

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 StartIteration(ZipArchiveHandle handle, void** cookie_ptr,
 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 @@ int32_t ExtractEntryToFile(ZipArchiveHandle handle,
                            ZipEntry* entry, int fd) {
   const int32_t declared_length = entry->uncompressed_length;
 
-  int result = TEMP_FAILURE_RETRY(ftruncate(fd, 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;
+  }
+
+  int result = TEMP_FAILURE_RETRY(ftruncate(fd, declared_length + current_offset));
   if (result == -1) {
-    ALOGW("Zip: unable to truncate file to %ud", declared_length);
+    ALOGW("Zip: unable to truncate file to %lld: %s", declared_length + current_offset,
+          strerror(errno));
     return kIoError;
   }
 
-  android::FileMap* map  = MapFileSegment(fd, 0, declared_length,
+  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 @@ TEST(ziparchive, ExtractToMemory) {
   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);