Extracting data from compressed file returns random data

Question

I'm using libzip to extract the content of each file in a zip into my own data structure, a C++ immutable POD.

The problem is that every time I extract the content of a file, I get some random data with tacked on to the end. Here's my code:

void Parser::populateFileMetadata() {
int error = 0;
zip *zip = zip_open(this->file_path.c_str(), 0, &error);
if (zip == nullptr) {
  LOG(DEBUG)<< "Could not open zip file.";
  return;
}

const zip_int64_t n_entries = zip_get_num_entries(zip, ZIP_FL_UNCHANGED);
for (zip_int64_t i = 0; i < n_entries; i++) {
  const char *file_name = zip_get_name(zip, i, ZIP_FL_ENC_GUESS);
  struct zip_stat st;
  zip_stat_init(&st);
  zip_stat(zip, file_name, (ZIP_FL_NOCASE|ZIP_FL_UNCHANGED), &st);
  char *content = new char[st.size];
  zip_file *file = zip_fopen(zip, file_name, \
                             (ZIP_FL_NOCASE|ZIP_FL_UNCHANGED));
  const zip_int64_t did_read = zip_fread(file, content, st.size);
  if (did_read <= 0) {
    LOG(WARNING)<< "Could not read contents of " << file_name << ".";
    continue;
  }
  const FileMetadata metadata(string(file_name), -1, string(content));
  this->file_metadata.push_back(metadata);

  zip_fclose(file);
  delete[] content;
}
zip_close(zip);
}

Answer 1

zip_fread seems to increase the size of content, so I just truncate content: content[st.size] = '\0';

Answer 2

You're constructing a std::string from content without telling the constructor how long it is, so the constructor is going to read from the start of the buffer until it finds a terminating NUL. But there's no guarantee that the file contains one, and so the constructor reads past the end of your buffer until it happens to find a NUL.

Fix: use the two-argument std::string constructor (string(const char* s, size_t size)) and pass it the data length.

Answer 3

@ruipacheco solution did not work for me. Doing content[st.size] = '\0'; fixed the problem but caused the error "double free or corruption..." when calling zip_fclose() and/or delete[] content So I did the below and it seems to work

void ReadZip(std::string &data){

....
....
data.resize(st.size);
for(uint i = 0; i<st.size; ++i)
    data[i] = std::move(content[i]);
}