Rezip tool used to modify the APK.

build/android/rezip/rezip.cc

Index: build/android/rezip/rezip.cc
diff --git a/build/android/rezip/rezip.cc b/build/android/rezip/rezip.cc
new file mode 100644
index 0000000000000000000000000000000000000000..49d5895da9aec86a489a3f325d0df0e6d9b69300
--- /dev/null
+++ b/build/android/rezip/rezip.cc
@@ -0,0 +1,492 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// rezip is a tool which is used to modify zip files. It reads in a zip
+// file and outputs a new zip file applying various transforms. The tool

[agl, 2014/06/11 17:52:40]

"after" before "applying".

[Anton, 2014/06/12 09:31:19]

Done.

+// is used in the Android Chromium build process to modify and APK file

[agl, 2014/06/11 17:52:41]

s/and/an/

[Anton, 2014/06/12 09:31:20]

Done.

+// (which are zip files). The main application of this is to modify the
+// APK so that the shared library is no longer compressed. Ironically,
+// this saves both transmission and device drive space. It saves
+// transmission space because uncompressed libraries make much smaller
+// deltas with previous versions. It saves device drive space because
+// it is no longer necessary to have both a compressed and uncompressed
+// shared library on the device. To achieve this the uncompressed library
+// is opened directly from within the APK using the "crazy" linker.
+
+#include <assert.h>
+#include <string.h>
+
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "third_party/zlib/contrib/minizip/unzip.h"
+#include "third_party/zlib/contrib/minizip/zip.h"
+
+const int kMaxFilenameInZip = 256;

[agl, 2014/06/11 17:52:40]

static for these?

[Anton, 2014/06/12 09:31:19]

Why? compile time constants are better.

[agl, 2014/06/17 17:40:37]

static doesn't affect whether they are constant or not, but it prevents the
symbols from being exported from the compilation unit, which I think you want.

[Anton, 2014/06/18 09:44:27]

And so does "const". It is in section 7.1.5.1.2 of the spec.

+const int kMaxExtraFieldInZip = 8192;
+const int kBufferSize = 4096;
+const int kPageSize = 4096;
+
+// This is done to avoid having to make a dependency on all of base.
+class LogStream {
+ public:
+  ~LogStream() {
+    stream_.flush();
+    std::cerr << stream_.str() << std::endl;
+  }
+  std::ostream& stream() {
+    return stream_;
+  }
+ private:
+  std::ostringstream stream_;
+};
+
+#define LOG(tag) (LogStream().stream() << #tag << ":")
+
+// Copy the data from the currently opened file in the zipfile we are unzipping
+// into the currently opened file of the zipfile we are zipping.
+static bool CopySubfile(unzFile in_file,
+                        zipFile out_file,
+                        const char* in_zip_filename,
+                        const char* out_zip_filename,
+                        const char* in_filename,
+                        const char* out_filename) {
+  char buf[kBufferSize];
+
+  int bytes = 0;
+  do {
+    bytes = unzReadCurrentFile(in_file, buf, sizeof(buf));
+    if (bytes == -1) {

[agl, 2014/06/11 17:52:40]

unzReadCurrentFile returns negative numbers for error, but not always -1. (E.g.
UNZ_CRCERROR is -105.)

[Anton, 2014/06/12 09:31:20]

Done.

+      LOG(ERROR) << "failed to read from " << in_filename << " in zipfile "
+                 << in_zip_filename;
+      return false;
+    }
+
+    if (bytes == 0) {
+      break;
+    }
+
+    if (ZIP_OK != zipWriteInFileInZip(out_file, buf, bytes)) {
+      LOG(ERROR) << "failed to write from " << out_filename << " in zipfile "
+                 << out_zip_filename;
+      return false;
+    }
+  } while (bytes != 0);

[agl, 2014/06/11 17:52:41]

it looks like this do { } while loop should be a for(;;) loop because bytes != 0
is checked in the middle.

[Anton, 2014/06/12 09:31:20]

Changed to while (true)

+
+  return true;
+}
+
+static zip_fileinfo BuildOutInfo(const unz_file_info& in_info) {
+  zip_fileinfo out_info;
+  out_info.tmz_date.tm_sec = in_info.tmu_date.tm_sec;

[agl, 2014/06/11 17:52:41]

does assigning the whole struct as one not work?

[Anton, 2014/06/12 09:31:20]

No, they are different types. tm_zip v. tm_unz

+  out_info.tmz_date.tm_min = in_info.tmu_date.tm_min;
+  out_info.tmz_date.tm_hour = in_info.tmu_date.tm_hour;
+  out_info.tmz_date.tm_mday = in_info.tmu_date.tm_mday;
+  out_info.tmz_date.tm_mon = in_info.tmu_date.tm_mon;
+  out_info.tmz_date.tm_year = in_info.tmu_date.tm_year;
+
+  out_info.dosDate = in_info.dosDate;
+  out_info.internal_fa = in_info.internal_fa;
+  out_info.external_fa = in_info.external_fa;
+  return out_info;
+}
+
+// RAII pattern for closing the unzip file.
+class UnzipCloser {
+ public:
+  UnzipCloser(unzFile z_file, const char* z_filename)
+      : z_file_(z_file), z_filename_(z_filename) {}
+
+  ~UnzipCloser() {
+    if (unzClose(z_file_) != UNZ_OK) {
+      LOG(ERROR) << "failed to close input zipfile " << z_filename_;
+      exit(1);
+    }
+  }
+
+ private:
+  const char* z_filename_;
+  unzFile z_file_;
+};
+
+// RAII pattern for closing the out zip file.
+class ZipCloser {
+ public:
+  ZipCloser(zipFile z_file, const char* z_filename)
+      : z_file_(z_file), z_filename_(z_filename) {}
+
+  ~ZipCloser() {
+    if (zipClose(z_file_, NULL) != ZIP_OK) {
+      LOG(ERROR) << "failed to close output zipfile" << z_filename_;
+      exit(1);
+    }
+  }
+
+ private:
+  const char* z_filename_;
+  zipFile z_file_;
+};
+
+typedef std::string (*RenameFun)(const char* in_filename);
+typedef int (*AlignFun)(const char* in_filename,
+                        unzFile in_file,
+                        char* extra_buffer,
+                        int size);
+typedef bool (*InflateFun)(const char* filename);
+
+static bool IsPrefixLibraryFilename(const char* filename,
+                                    const char* base_prefix) {
+  // We are basically matching "lib/[^/]*/<base_prefix>lib.*[.]so".
+  // However, we don't have C++11 regex, so we just handroll the test.
+  std::string filename_str = filename;

[agl, 2014/06/11 17:52:40]

const for these?

[Anton, 2014/06/12 09:31:20]

Done.

+  std::string prefix = "lib/";
+  std::string suffix = ".so";
+
+  if (filename_str.length() < suffix.length() + prefix.length()) {
+    // too short
+    return false;
+  }
+
+  if (filename_str.substr(0, prefix.size()) != prefix) {

[agl, 2014/06/11 17:52:41]

this test uses strsub and !=, but the next uses just compare. They should
probably both be one or the other.

[Anton, 2014/06/12 09:31:20]

Changed to use compare

+    // does not start with "lib/"
+    return false;
+  }
+
+  if (filename_str.compare(filename_str.length() - suffix.length(),
+                           suffix.length(),
+                           suffix) != 0) {
+    // does not end with ".so"
+    return false;
+  }
+
+  size_t last_slash = filename_str.find_last_of('/');

[agl, 2014/06/11 17:52:41]

these, and below, could be const if you like - not important.

[Anton, 2014/06/12 09:31:20]

Done.

+  if (last_slash < prefix.length()) {
+    // Only one slash
+    return false;
+  }
+
+  size_t second_slash = filename_str.find_first_of('/', prefix.length());
+  if (second_slash != last_slash) {
+    // filename_str contains more than two slashes.
+    return false;
+  }
+
+  std::string libprefix = std::string(base_prefix) + "lib";
+  if (filename_str.compare(last_slash + 1, libprefix.length(), libprefix) !=

[agl, 2014/06/11 17:52:41]

I think you need to check that libprefix isn't out of range for filename_str.

[Anton, 2014/06/12 09:31:19]

compare goes until libprefix.length() or the end of the string, so you can't go
out of range. last_slash + 1 could be out of range except we already checked
that the string ends in ".so".

+      0) {
+    // basename piece does not start with <base_prefix>"lib"
+    return false;
+  }
+
+  std::string linker = "libchromium_android_linker.so";

[agl, 2014/06/11 17:52:41]

this isn't mentioned in the comment at the top of the function.

Also, this could be out of range for filename_str. Lastly, the second clause
should be first because it's much cheaper.

[Anton, 2014/06/12 09:31:20]

Done
Same as in the previous case.
Done

+  if (filename_str.compare(last_slash + 1, linker.length(), linker) == 0 &&
+      last_slash + 1 + linker.length() == filename_str.length()) {
+    // Do not match the linker.
+    return false;
+  }
+  return true;
+}
+
+static bool IsLibraryFilename(const char* filename) {
+  return IsPrefixLibraryFilename(filename, "");
+}
+
+static bool IsCrazyLibraryFilename(const char* filename) {
+  return IsPrefixLibraryFilename(filename, "crazy.");
+}
+
+static std::string RenameLibrary(const char* in_filename) {
+  if (!IsLibraryFilename(in_filename)) {
+    // Don't rename
+    return in_filename;
+  }
+
+  std::string filename_str = in_filename;
+  size_t last_slash = filename_str.find_last_of('/');
+  if (last_slash == std::string::npos &&

[agl, 2014/06/11 17:52:41]

if last_slash equals npos, I'm not sure that the second clause of this
conjunction can be false. 

[Anton, 2014/06/12 09:31:20]

This is really just a fail safe, as IsLibraryFilename() already guarantees that
slash is in a safe location. Anyway you are right it should be an ||.

+      last_slash != filename_str.length() - 1) {
+    return in_filename;
+  }
+
+  // We rename the library, so that the Android Package Manager
+  // no longer extracts the library.
+  std::string basename_prefix = "crazy.";
+  return filename_str.substr(0, last_slash + 1) + basename_prefix +
+         filename_str.substr(last_slash + 1);
+}
+
+static int PageAlignCrazyLibrary(const char* in_filename,
+                                 unzFile in_file,
+                                 char* extra_buffer,
+                                 int size) {
+  if (!IsCrazyLibraryFilename(in_filename)) {
+    return size;
+  }
+  ZPOS64_T pos = unzGetCurrentFileZStreamPos64(in_file);
+  int padding = kPageSize - (pos % kPageSize);
+  if (padding == 0) {

[agl, 2014/06/11 17:52:41]

Can padding ever be zero? You might want to be checking for padding ==
kPageSize.

[Anton, 2014/06/12 09:31:20]

Done.

+    return size;
+  }

[agl, 2014/06/11 17:52:41]

maybe

if (INT_MAX - size < padding) {
  abort();
}

In case the file size gets near 2GB?

[Anton, 2014/06/12 09:31:20]

Changed to:
assert(extra_size < kMaxExtraFieldInZip - padding);

+
+  assert(padding + size < kMaxExtraFieldInZip);
+  memset(extra_buffer + size, 0, padding);
+  return size + padding;
+}
+
+// As only the read side API provides offsets, we check that we added the
+// correct amount of padding by reading the zip file we just generated.
+static bool CheckPageAlign(const char* out_zip_filename) {
+  unzFile in_file = unzOpen(out_zip_filename);
+  if (in_file == NULL) {
+    LOG(ERROR) << "failed to open zipfile " << out_zip_filename;
+    return false;
+  }
+  UnzipCloser unzipCloser(in_file, out_zip_filename);
+
+  int err = 0;
+  bool checked = false;
+  do {
+    char in_filename[kMaxFilenameInZip + 1];

[agl, 2014/06/11 17:52:41]

you add one to the size as if you were trying to ensure that it was always NUL
terminated, but you don't set the last byte to NUL.

[Anton, 2014/06/12 09:31:20]

This is based on the code in unzip.c which appears to have the same problem.

+    // Get info and extra field for current file.
+    unz_file_info in_info;
+    err = unzGetCurrentFileInfo(in_file,
+                                &in_info,
+                                in_filename,
+                                sizeof(in_filename) - 1,
+                                NULL,
+                                0,
+                                NULL,
+                                0);
+    if (err != UNZ_OK) {
+      LOG(ERROR) << "failed to get filename" << out_zip_filename;
+      return false;
+    }
+
+    if (IsCrazyLibraryFilename(in_filename)) {
+      err = unzOpenCurrentFile(in_file);
+      if (err != UNZ_OK) {
+        LOG(ERROR) << "failed to open subfile" << out_zip_filename << " "
+                   << in_filename;
+        return false;
+      }
+
+      ZPOS64_T pos = unzGetCurrentFileZStreamPos64(in_file);
+      int alignment = pos % kPageSize;
+      checked = (alignment == 0);
+      if (!checked) {
+        LOG(ERROR) << "Failed to page align library " << in_filename
+                   << ", position " << pos << " alignment " << alignment;
+      }
+
+      err = unzCloseCurrentFile(in_file);
+      if (err != UNZ_OK) {
+        LOG(ERROR) << "failed to close subfile" << out_zip_filename << " "
+                   << in_filename;
+        return false;
+      }
+    }
+
+    int next = unzGoToNextFile(in_file);
+    if (next == UNZ_END_OF_LIST_OF_FILE) {
+      break;
+    }
+    if (next != UNZ_OK) {
+      LOG(ERROR) << "failed to go to next file" << out_zip_filename;
+      return false;
+    }
+  } while (true);

[agl, 2014/06/11 17:52:41]

use for(;;) rather than do { } while (true).

[Anton, 2014/06/12 09:31:19]

Changed to while (true)

+  return checked;
+}
+
+// Copy files from one archive to another applying alignment, rename and
+// inflate transformations if given.
+static bool Rezip(const char* in_zip_filename,
+                  const char* out_zip_filename,
+                  AlignFun align_fun,
+                  RenameFun rename_fun,
+                  InflateFun inflate_fun) {
+  unzFile in_file = unzOpen(in_zip_filename);
+  if (in_file == NULL) {
+    LOG(ERROR) << "failed to open zipfile " << in_zip_filename;
+    return false;
+  }
+  UnzipCloser unzipCloser(in_file, in_zip_filename);
+
+  zipFile out_file = zipOpen(out_zip_filename, APPEND_STATUS_CREATE);
+  if (unzGoToFirstFile(in_file) != UNZ_OK) {
+    LOG(ERROR) << "failed to go to first file in " << in_zip_filename;
+    return false;
+  }
+  ZipCloser zipCloser(out_file, out_zip_filename);
+
+  int err = 0;
+  do {
+    char in_filename[kMaxFilenameInZip + 1];

[agl, 2014/06/11 17:52:40]

ditto about the NUL termination.

[Anton, 2014/06/12 09:31:20]

Done.

+    // Get info and extra field for current file.
+    char extra_buffer[kMaxExtraFieldInZip];
+    unz_file_info in_info;
+    err = unzGetCurrentFileInfo(in_file,
+                                &in_info,
+                                in_filename,
+                                sizeof(in_filename) - 1,
+                                &extra_buffer,
+                                sizeof(extra_buffer),
+                                NULL,
+                                0);
+    if (err != UNZ_OK) {
+      LOG(ERROR) << "failed to get filename " << in_zip_filename;
+      return false;
+    }
+
+    std::string out_filename = in_filename;
+    if (rename_fun != NULL) {
+      out_filename = rename_fun(in_filename);
+    } else {
+      out_filename = in_filename;

[agl, 2014/06/11 17:52:41]

this else can be removed.

[Anton, 2014/06/12 09:31:20]

Done.

+    }
+
+    bool inflate = false;
+    if (inflate_fun != NULL) {
+      inflate = inflate_fun(in_filename);
+    }
+
+    // Open the current file.
+    int method = 0;
+    int level = 0;
+    int raw = !inflate;
+    err = unzOpenCurrentFile2(in_file, &method, &level, raw);
+    if (inflate) {
+      method = Z_NO_COMPRESSION;
+      level = 0;
+    }
+
+    if (err != UNZ_OK) {
+      LOG(ERROR) << "failed to open subfile " << in_zip_filename << " "
+                 << in_filename;
+      return false;
+    }
+
+    // Get the extra field from the local header.
+    char local_extra_buffer[kMaxExtraFieldInZip];
+    int local_extra_size = unzGetLocalExtrafield(
+        in_file, &local_extra_buffer, sizeof(local_extra_buffer));
+
+    if (align_fun != NULL) {
+      local_extra_size =
+          align_fun(in_filename, in_file, local_extra_buffer, local_extra_size);
+    }
+
+    const char* local_extra = local_extra_size > 0 ? local_extra_buffer : NULL;
+    const char* extra = in_info.size_file_extra > 0 ? extra_buffer : NULL;
+
+    // Build the output info structure from the input info structure.
+    zip_fileinfo out_info = BuildOutInfo(in_info);
+
+    int ret = zipOpenNewFileInZip4(out_file,
+                                   out_filename.c_str(),
+                                   &out_info,
+                                   local_extra,
+                                   local_extra_size,
+                                   extra,
+                                   in_info.size_file_extra,
+                                   /* comment */ NULL,
+                                   method,
+                                   level,
+                                   /* raw */ 1,
+                                   /* windowBits */ 0,
+                                   /* memLevel */ 0,
+                                   /* strategy */ 0,
+                                   /* password */ NULL,
+                                   /* crcForCrypting */ 0,
+                                   in_info.version,
+                                   /*flagBase */ 0);
+
+    if (ZIP_OK != ret) {
+      LOG(ERROR) << "failed to open subfile " << out_zip_filename << " "
+                 << out_filename;
+      return false;
+    }
+
+    if (!CopySubfile(in_file,
+                     out_file,
+                     in_zip_filename,
+                     out_zip_filename,
+                     in_filename,
+                     out_filename.c_str())) {
+      return false;
+    }
+
+    if (ZIP_OK != zipCloseFileInZipRaw(
+                      out_file, in_info.uncompressed_size, in_info.crc)) {
+      LOG(ERROR) << "failed to close subfile " << out_zip_filename << " "
+                 << out_filename;
+      return false;
+    }
+
+    err = unzCloseCurrentFile(in_file);
+    if (err != UNZ_OK) {
+      LOG(ERROR) << "failed to close subfile " << in_zip_filename << " "
+                 << in_filename;
+      return false;
+    }
+    int next = unzGoToNextFile(in_file);
+    if (next == UNZ_END_OF_LIST_OF_FILE) {
+      break;
+    }
+    if (next != UNZ_OK) {
+      LOG(ERROR) << "failed to go to next file" << in_zip_filename;
+      return false;
+    }
+  } while (true);

[agl, 2014/06/11 17:52:41]

use for(;;) rather than do { } while(true).

[Anton, 2014/06/12 09:31:19]

Changed to while (true)

+
+  return true;
+}
+
+int main(int argc, const char* argv[]) {
+  if (argc != 4) {
+    LOG(ERROR) << "Usage: <action> <in_zipfile> <out_zipfile>";
+    LOG(ERROR) << " <action> is 'inflatealign', 'dropdescriptors' or 'rename'";
+    exit(1);
+  }
+
+  const char* action = argv[1];
+  const char* in_zip_filename = argv[2];
+  const char* out_zip_filename = argv[3];
+
+  InflateFun inflate_fun = NULL;
+  AlignFun align_fun = NULL;
+  RenameFun rename_fun = NULL;
+  bool checkPageAlign = false;
+  if (strcmp("inflatealign", action) == 0) {
+    inflate_fun = &IsCrazyLibraryFilename;
+    align_fun = &PageAlignCrazyLibrary;
+    checkPageAlign = true;
+  } else if (strcmp("rename", action) == 0) {
+    rename_fun = &RenameLibrary;
+  } else if (strcmp("dropdescriptors", action) == 0) {
+    // Minizip does not know about data descriptors, so the default
+    // copying action will drop the descriptors. This should be fine
+    // as data descriptors are complete redundant information.

[agl, 2014/06/11 17:52:40]

s/complete//

[Anton, 2014/06/12 09:31:20]

Done.

+    // Note we need to explicitly drop the descriptors before trying to
+    // do alignment otherwise we will miscalculate the position because
+    // we don't know about the data descriptors.
+  } else {
+    LOG(ERROR) << "Usage: <action> should be 'inflatealign', "
+                  "'dropdescriptors' or 'rename'";
+    exit(1);
+  }
+
+  if (!Rezip(in_zip_filename,
+             out_zip_filename,
+             align_fun,
+             rename_fun,
+             inflate_fun)) {
+    exit(1);
+  }
+  if (checkPageAlign && !CheckPageAlign(out_zip_filename)) {
+    exit(1);
+  }
+  return 0;
+}