Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 964b95cf authored by Andreas Gampe's avatar Andreas Gampe
Browse files

Ziparchive: Enable -Wconversion 

Enable -Wconversion (but not -Wsign-conversion). Fix up code. Handle
some actual error cases:

* too many files
* files too large

Bug: 130039052
Test: atest ziparchive-tests
Change-Id: I632af317b9e65dbc728851efefd0d11a2b5c29b9
parent 3a6a01bd

libziparchive/Android.bp

+2 −0
Original line number Diff line number Diff line
@@ -26,6 +26,8 @@ cc_defaults { 
        // Incorrectly warns when C++11 empty brace {} initializer is used.

        // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61489

        "-Wno-missing-field-initializers",

        "-Wconversion",

        "-Wno-sign-conversion",

    ],



    // Enable -Wold-style-cast only for non-Windows targets.  _islower_l,

libziparchive/entry_name_utils-inl.h

+9 −2
Original line number Diff line number Diff line
@@ -20,9 +20,15 @@ 
#include <stddef.h>

#include <stdint.h>



#include <limits>



// Check if |length| bytes at |entry_name| constitute a valid entry name.

// Entry names must be valid UTF-8 and must not contain '0'.

// Entry names must be valid UTF-8 and must not contain '0'. They also must

// fit into the central directory record.

inline bool IsValidEntryName(const uint8_t* entry_name, const size_t length) {

  if (length > std::numeric_limits<uint16_t>::max()) {

    return false;

  }

  for (size_t i = 0; i < length; ++i) {

    const uint8_t byte = entry_name[i];

    if (byte == 0) {
@@ -35,7 +41,8 @@ inline bool IsValidEntryName(const uint8_t* entry_name, const size_t length) { 
      return false;

    } else {

      // 2-5 byte sequences.

      for (uint8_t first = (byte & 0x7f) << 1; first & 0x80; first = (first & 0x7f) << 1) {

      for (uint8_t first = static_cast<uint8_t>((byte & 0x7f) << 1); first & 0x80;

           first = static_cast<uint8_t>((first & 0x7f) << 1)) {

        ++i;



        // Missing continuation byte..

libziparchive/include/ziparchive/zip_writer.h

+3 −3
Original line number Diff line number Diff line
@@ -76,7 +76,7 @@ class ZipWriter { 
    uint32_t uncompressed_size;

    uint16_t last_mod_time;

    uint16_t last_mod_date;

    uint32_t padding_length;

    uint16_t padding_length;

    off64_t local_file_header_offset;

  };
@@ -161,8 +161,8 @@ class ZipWriter { 


  int32_t HandleError(int32_t error_code);

  int32_t PrepareDeflate();

  int32_t StoreBytes(FileEntry* file, const void* data, size_t len);

  int32_t CompressBytes(FileEntry* file, const void* data, size_t len);

  int32_t StoreBytes(FileEntry* file, const void* data, uint32_t len);

  int32_t CompressBytes(FileEntry* file, const void* data, uint32_t len);

  int32_t FlushCompressedBytes(FileEntry* file);



  enum class State {

libziparchive/unzip.cpp

+1 −1
Original line number Diff line number Diff line
@@ -72,7 +72,7 @@ static bool MakeDirectoryHierarchy(const std::string& path) { 


static int CompressionRatio(int64_t uncompressed, int64_t compressed) {

  if (uncompressed == 0) return 0;

  return (100LL * (uncompressed - compressed)) / uncompressed;

  return static_cast<int>((100LL * (uncompressed - compressed)) / uncompressed);

}



static void MaybeShowHeader() {

libziparchive/zip_archive.cc

+20 −18
Original line number Diff line number Diff line
@@ -147,7 +147,7 @@ static int64_t EntryToIndex(const ZipStringOffset* hash_table, const uint32_t ha 
/*

 * Add a new entry to the hash table.

 */

static int32_t AddToHash(ZipStringOffset* hash_table, const uint64_t hash_table_size,

static int32_t AddToHash(ZipStringOffset* hash_table, const uint32_t hash_table_size,

                         const ZipString& name, const uint8_t* start) {

  const uint64_t hash = ComputeHash(name);

  uint32_t ent = hash & (hash_table_size - 1);
@@ -215,7 +215,7 @@ ZipArchive::~ZipArchive() { 
}



static int32_t MapCentralDirectory0(const char* debug_file_name, ZipArchive* archive,

                                    off64_t file_length, off64_t read_amount,

                                    off64_t file_length, uint32_t read_amount,

                                    uint8_t* scan_buffer) {

  const off64_t search_start = file_length - read_amount;
@@ -231,7 +231,8 @@ static int32_t MapCentralDirectory0(const char* debug_file_name, ZipArchive* arc 
   * doing an initial minimal read; if we don't find it, retry with a

   * second read as above.)

   */

  int i = read_amount - sizeof(EocdRecord);

  CHECK_LE(read_amount, std::numeric_limits<int32_t>::max());

  int32_t i = read_amount - sizeof(EocdRecord);

  for (; i >= 0; i--) {

    if (scan_buffer[i] == 0x50) {

      uint32_t* sig_addr = reinterpret_cast<uint32_t*>(&scan_buffer[i]);
@@ -334,9 +335,9 @@ static int32_t MapCentralDirectory(const char* debug_file_name, ZipArchive* arch 
   *

   * We start by pulling in the last part of the file.

   */

  off64_t read_amount = kMaxEOCDSearch;

  uint32_t read_amount = kMaxEOCDSearch;

  if (file_length < read_amount) {

    read_amount = file_length;

    read_amount = static_cast<uint32_t>(file_length);

  }



  std::vector<uint8_t> scan_buffer(read_amount);
@@ -533,7 +534,7 @@ static int32_t ValidateDataDescriptor(MappedZipFile& mapped_zip, ZipEntry* entry 
  return 0;

}



static int32_t FindEntry(const ZipArchive* archive, const int ent, ZipEntry* data) {

static int32_t FindEntry(const ZipArchive* archive, const int32_t ent, ZipEntry* data) {

  const uint16_t nameLen = archive->hash_table[ent].name_length;



  // Recover the start of the central directory entry from the filename
@@ -752,9 +753,10 @@ int32_t FindEntry(const ZipArchiveHandle archive, const ZipString& entryName, Zi 
                                   archive->central_directory.GetBasePtr());

  if (ent < 0) {

    ALOGV("Zip: Could not find entry %.*s", entryName.name_length, entryName.name);

    return ent;

    return static_cast<int32_t>(ent);  // kEntryNotFound is safe to truncate.

  }

  return FindEntry(archive, ent, data);

  // We know there are at most hast_table_size entries, safe to truncate.

  return FindEntry(archive, static_cast<uint32_t>(ent), data);

}



int32_t Next(void* cookie, ZipEntry* data, ZipString* name) {
@@ -837,7 +839,6 @@ class FileWriter : public zip_archive::Writer { 
      return FileWriter{};

    }



    int result = 0;

#if defined(__linux__)

    if (declared_length > 0) {

      // Make sure we have enough space on the volume to extract the compressed
@@ -849,7 +850,7 @@ class FileWriter : public zip_archive::Writer { 
      // EOPNOTSUPP error when issued in other filesystems.

      // Hence, check for the return error code before concluding that the

      // disk does not have enough space.

      result = TEMP_FAILURE_RETRY(fallocate(fd, 0, current_offset, declared_length));

      long result = TEMP_FAILURE_RETRY(fallocate(fd, 0, current_offset, declared_length));

      if (result == -1 && errno == ENOSPC) {

        ALOGW("Zip: unable to allocate %" PRId64 " bytes at offset %" PRId64 ": %s",

              static_cast<int64_t>(declared_length), static_cast<int64_t>(current_offset),
@@ -867,7 +868,7 @@ class FileWriter : public zip_archive::Writer { 


    // Block device doesn't support ftruncate(2).

    if (!S_ISBLK(sb.st_mode)) {

      result = TEMP_FAILURE_RETRY(ftruncate(fd, declared_length + current_offset));

      long result = TEMP_FAILURE_RETRY(ftruncate(fd, declared_length + current_offset));

      if (result == -1) {

        ALOGW("Zip: unable to truncate file to %" PRId64 ": %s",

              static_cast<int64_t>(declared_length + current_offset), strerror(errno));
@@ -986,16 +987,16 @@ int32_t Inflate(const Reader& reader, const uint32_t compressed_length, 
  std::unique_ptr<z_stream, decltype(zstream_deleter)> zstream_guard(&zstream, zstream_deleter);



  const bool compute_crc = (crc_out != nullptr);

  uint64_t crc = 0;

  uLong crc = 0;

  uint32_t remaining_bytes = compressed_length;

  do {

    /* read as much as we can */

    if (zstream.avail_in == 0) {

      const size_t read_size = (remaining_bytes > kBufSize) ? kBufSize : remaining_bytes;

      const uint32_t read_size = (remaining_bytes > kBufSize) ? kBufSize : remaining_bytes;

      const uint32_t offset = (compressed_length - remaining_bytes);

      // Make sure to read at offset to ensure concurrent access to the fd.

      if (!reader.ReadAtOffset(read_buf.data(), read_size, offset)) {

        ALOGW("Zip: inflate read failed, getSize = %zu: %s", read_size, strerror(errno));

        ALOGW("Zip: inflate read failed, getSize = %u: %s", read_size, strerror(errno));

        return kIoError;

      }
@@ -1019,7 +1020,8 @@ int32_t Inflate(const Reader& reader, const uint32_t compressed_length, 
      if (!writer->Append(&write_buf[0], write_size)) {

        return kIoError;

      } else if (compute_crc) {

        crc = crc32(crc, &write_buf[0], write_size);

        DCHECK_LE(write_size, kBufSize);

        crc = crc32(crc, &write_buf[0], static_cast<uint32_t>(write_size));

      }



      zstream.next_out = &write_buf[0];
@@ -1064,17 +1066,17 @@ static int32_t CopyEntryToWriter(MappedZipFile& mapped_zip, const ZipEntry* entr 


  const uint32_t length = entry->uncompressed_length;

  uint32_t count = 0;

  uint64_t crc = 0;

  uLong crc = 0;

  while (count < length) {

    uint32_t remaining = length - count;

    off64_t offset = entry->offset + count;



    // Safe conversion because kBufSize is narrow enough for a 32 bit signed value.

    const size_t block_size = (remaining > kBufSize) ? kBufSize : remaining;

    const uint32_t block_size = (remaining > kBufSize) ? kBufSize : remaining;



    // Make sure to read at offset to ensure concurrent access to the fd.

    if (!mapped_zip.ReadAtOffset(buf.data(), block_size, offset)) {

      ALOGW("CopyFileToFile: copy read failed, block_size = %zu, offset = %" PRId64 ": %s",

      ALOGW("CopyFileToFile: copy read failed, block_size = %u, offset = %" PRId64 ": %s",

            block_size, static_cast<int64_t>(offset), strerror(errno));

      return kIoError;

    }