libzip zip_source_t buffer is freed even tho I told libzip not to

Question

I face an issue with libzip and I don't know if I did something wrong of if it's a bug in the library.

First I create an archive with zip_source_buffer_create() from a zip file on disk (don't ask why I don't open the file directly).

I set the freep flag to 0 which is supposed to mean I will handle the buffer freeing myself and it's not lipzip's job.

It works if I don't add a file to the archive.

If I add a file to the archive, when I delete the buffer libzip has already free'd it and my application crashes. Is that a bug in the library ?

I wrote this self-explanatory mcve. With AUTO_FREE_BUF=0 and ADD_FILE=1, the application crashes. Every other (3) cases work.

libzip version: 1.7.3

#include <zip.h>
#include <string>
#include <filesystem>
#include <fstream>
#include <iostream>

#define AUTO_FREE_BUF 0 //1-0
#define ADD_FILE 1 //1-0

int main(int argc, char** argv) {

    const std::string add_file = "file.txt";
    const std::string zip_file = "zipfile.zip";

    size_t sz = std::filesystem::file_size(zip_file);
    char* buf = new char[sz];
    {
        std::ifstream ifs{ zip_file, std::ios::binary };
        ifs.read(buf, sz);
        ifs.close();

        zip_error_t ze;
        zip_error_init(&ze);
        zip_source_t* zs = zip_source_buffer_create(buf, sz, AUTO_FREE_BUF, &ze);
        if (zs == nullptr) {
            zip_error_fini(&ze);
            return -1;
        }

        zip_error_init(&ze);
        zip_t* z = zip_open_from_source(zs, NULL, &ze);
        if (z == nullptr) {
            zip_error_fini(&ze);
            return -1;
        }
#if ADD_FILE == 1
        //add file
        {
            zip_source_t* file = zip_source_file(z, add_file.c_str(), 0, -1);
            if (file == nullptr)
                return -1;

            zip_int64_t add = zip_file_add(z, add_file.c_str(), file, ZIP_FL_ENC_GUESS);
            if (add == -1)
                return -1;
        }
#endif
        zip_error_fini(&ze);
        zip_source_keep(zs);
        int close = zip_close(z);
        if (close == -1)
            return -1;

        //write back archive to disk
        //..

#if AUTO_FREE_BUF == 1
        zip_source_free(zs);
#else
        zip_source_free(zs); //<-is supposed to NOT free the buffer (with freep=0) but sometimes does
        delete[] buf;
#endif
    }

    return 0;
}

Answer 1

Don't handle zip archives created with WinRar with libzip............... (create them with libzip)

Answer 2

I've installed libzip from sources (as described in the install.md) and reworked your source. I've added a textfile (with "Hello world") and a zip file with one text file inside.

I'm not getting errors. (However, nothing is added to the zip file, so that's also not right)

(ignore the exception parameters commented out :P )

#include <zip.h>

#include <exception>
#include <filesystem>
#include <fstream>
#include <iostream>
#include <string>

static constexpr auto add_file{"file.txt"};
static constexpr auto zip_file{"zipfile.zip"};
static constexpr auto AUTO_FREE_BUF{false};  //1-0
static constexpr auto ADD_FILE{true};        //1-0

class MsgException : public std::exception {
   public:
    MsgException(std::string message) : _message(message) {}
    char const *what() const noexcept override { return _message.c_str(); }

   private:
    std::string const _message;
};

int main() {
    std::cout << "Started main\n";
    auto const sz = std::filesystem::file_size(zip_file);
    auto *const buf = new char[sz];
    {
        std::ifstream ifs{zip_file, std::ios::binary};
        ifs.read(buf, sz);
    }
    std::cout << "Zipfile read\n";

    zip_error_t ze;
    try {
        zip_error_init(&ze);
        auto *zs = zip_source_buffer_create(buf, sz, AUTO_FREE_BUF ? 1 : 0, &ze);
        if (zs == nullptr) throw MsgException("Can't create source: %s." /*, zip_error_strerror(&ze)*/);

        //zip_error_init(&ze);
        auto *z = zip_open_from_source(zs, 0, &ze);
        if (z == nullptr) throw MsgException("Can't open zip from source: %s." /*, zip_error_strerror(&ze)*/);

        if (ADD_FILE) {
            //add file
            auto *file = zip_source_file(z, add_file, 0, -1);
            if (file == nullptr) throw MsgException("Can't create data source from file '%s': %s." /*, add_file, zip_strerror(z)*/);

            if (zip_file_add(z, add_file, file, ZIP_FL_ENC_GUESS) < 0) {
                //? zip_source_free(file);
                throw MsgException("Can't file to zip archive: %s." /*, zip_strerror(z)*/);
            }
        }

        zip_source_keep(zs);

        if (zip_close(z) < 0) throw MsgException("Can't close zip archive: %s." /*, zip_strerror(z)*/);

        //write back archive to disk
        //..

        zip_source_free(zs);  //<-is supposed to NOT free the buffer (with freep=0) but sometimes does
    } catch (std::exception &e) {
        std::cerr << "Program aborted with error: " << e.what() << '\n';
    }
    zip_error_fini(&ze);
    if (!AUTO_FREE_BUF) delete[] buf;

    std::cout << "Finished main\n";
    return 0;
}

build using

g++ -std=c++17 main.cpp -lzip -o main

Edit: Added some RAII

#include <zip.h>

#include <exception>
#include <filesystem>
#include <fstream>
#include <iostream>
#include <string>
#include <vector>

static constexpr auto add_file{ "file.txt" };
static constexpr auto zip_file{ "zipfile.zip" };
static constexpr auto AUTO_FREE_BUF{ false };  //1-0
static constexpr auto ADD_FILE{ true };        //1-0

class MsgException : public std::exception {
public:
    MsgException(std::string message) : _message(message) {}
    char const* what() const noexcept override { return _message.c_str(); }

private:
    std::string const _message;
};

//RAII
class ZipError {
public:
    ZipError() { zip_error_init(&ze); }
    ~ZipError() { zip_error_fini(&ze); }
    zip_error* operator&() { return &ze; }
    // actually probably need to delete copy/move constructor/assignment etc...
private:
    zip_error_t ze;
};

int main() {
    std::cout << "Started main\n";
    std::vector<char> buf(std::filesystem::file_size(zip_file)); // RAII
    {
        std::ifstream ifs{ zip_file, std::ios::binary };
        ifs.read(buf.data(), buf.size());
    }
    std::cout << "Zipfile read\n";

    
    try {
        ZipError ze;
        auto* zs = zip_source_buffer_create(buf.data(), buf.size(), AUTO_FREE_BUF ? 1 : 0, &ze);
        if (zs == nullptr) throw MsgException("Can't create source: %s." /*, zip_error_strerror(&ze)*/);

        //zip_error_init(&ze);
        auto* z = zip_open_from_source(zs, 0, &ze);
        if (z == nullptr) throw MsgException("Can't open zip from source: %s." /*, zip_error_strerror(&ze)*/);

        if (ADD_FILE) {
            //add file
            auto* file = zip_source_file(z, add_file, 0, -1);
            if (file == nullptr) throw MsgException("Can't create data source from file '%s': %s." /*, add_file, zip_strerror(z)*/);

            if (zip_file_add(z, add_file, file, ZIP_FL_ENC_GUESS) < 0) {
                //? zip_source_free(file);
                throw MsgException("Can't file to zip archive: %s." /*, zip_strerror(z)*/);
            }
        }

        zip_source_keep(zs);

        if (zip_close(z) < 0) throw MsgException("Can't close zip archive: %s." /*, zip_strerror(z)*/);

        //write back archive to disk
        //..

        zip_source_free(zs);  //<-is supposed to NOT free the buffer (with freep=0) but sometimes does
    }
    catch (std::exception& e) {
        std::cerr << "Program aborted with error: " << e.what() << '\n';
    }
    
    std::cout << "Finished main\n";
    return 0;
}

Answer 3

After (hours now) of investigation I think I understand the rationale behind this and I think libzip doesn't behave as it should.

zip_source_free(src) decrements the refcount of src and if it reaches 0 the src object is free'd. Additionnaly if src was created with freep=1 the associated data buffer is also free'd. If freep=0 then the data buffer should not be freed.

But in this case and in some occasions the buffer IS free'd and that's probably unintended.

In the mcve above, adding a file to the archive makes libzip free the data buffer even tho the source was created with freep=0. If no file is added to the archive then the buffer is not free'd as expected and I can safely delete[] it.

This is not a solution to my question tho. I noticed src.zip_err=18 [Invalid argument] in the mcve after adding the file so there might be something wrong with how the file is added - I haven't found what't wrong with it.