zipper::Unzipper:extract silently fails

[Mimickal]

Issue

If a zip archive fails to extract to a directory using zipper::Unzipper::extract, the failure is not signaled to the caller in any way.

Example

The following code attempts to extract an archive to a directory the program does not have write permissions in. I would expect this to throw an exception, or at least return false to signify that the extraction failed. Instead, it always returns true.

zipper::Unzipper zip("my/real/archive.zip");
zip.extract("/dev/something"); // I don't have write permissions here

Where it happens

zipper::Unzipper::extract calls zipper::Unzipper::Impl::extractAll and passes the return value on to the caller. zipper::Unzipper::Impl::extractAll calls zipper::Unzipper::Impl::extractCurrentEntryToFile, which also returns a boolean indicating success.

The problem here is that zipper::Unzipper::Impl::extractAll does not use the boolean value returned from zipper::Unzipper::Impl::extractCurrentEntryToFile. It instead always returns true (see here).

How to fix it

We can't just return false as soon as we get a false value from extractCurrentEntryToFile, because that can happen part-way through extracting an archive. This would tell the caller we failed to extract the archive, while also leaving a partially-extracted archive on disk, which is probably not the desired outcome.

Is this a known issue? If not, what are people's thoughts on the desired functionality here?

[Lecrapouille]

Sorry for the long delay. I'll give a try to look at

[Lecrapouille]

Fixed ! Reopen the ticket if the fix is incomplete

[panosk]

Hello,

Unfortunately this change breaks completely the extractAll() method.

[Lecrapouille]

@panosk Can you provide a basic example ?

[panosk]

Sure, here it is:

zipper::Unzipper unzipper("my_zipfile","my_password");
if (!unzipper.extract("destination_dir")) // <-- This always returns false

I didn't dig much into Zipper's code, but it seems this is happening because the block

if (!extractCurrentEntryToFile(*it, alternativeName))
                          return false;

in extractAll always returns false.

[Lecrapouille]

@panosk Hmm ! Maybe this is related to https://github.com/sebastiandev/zipper/issues/102 in where for some architectures the password is working fine while on others it is not working. 1/ Can retry without password. 2/ Can you simply check with an external tool to open your password'ed zip file and read your files. I guess for 1/ you'll success and for 2/ the external tool will fail too. Could be cool to know which version and what exact lib your cmake is including. If my hypothesis is correct, I think I'll close this ticket, and we will follow the investigation directly in 102.

The extractAll() method seems fine to me. It's working with Debian:

echo "../Test1.txt" > "../Test1.txt"
echo "../Test2.txt" > "../Test2.txt"

And the code (main.cpp):

#include <zipper/zipper.h>
#include <zipper/unzipper.h>

using namespace zipper;

#include <fstream>

int main()
{
    try
    {
        std::ifstream input1("../Test1.txt");
        if (input1.fail())
        {
            throw std::runtime_error("Cannot open input file 1");
        }
        std::ifstream input2("../Test2.txt");
        if (input2.fail())
        {
            throw std::runtime_error("Cannot open input file 2");
        }

        //
        std::cout << "Zipper example" << std::endl;
        Zipper zipper("ziptest2.zip", "123456", Zipper::Overwrite);
        std::cout << "Input1" << std::endl;
        zipper.add(input1, "Test1", Zipper::Medium);
        std::cout << "Input2" << std::endl;
        zipper.add(input2, "Test2");
        std::cout << "Zipper Finished" << std::endl;
        zipper.close();

        //
        Unzipper unzipper("ziptest2.zip", "123456");
        if (!unzipper.extract("foo"))
        {
            throw std::runtime_error("Failed extracting");
        }
        unzipper.close();
        std::cout << "Unzipper Finished" << std::endl;
    }
    catch (std::exception &exception)
    {
        std::cerr << exception.what() << std::endl;
    }
}

Compilation:

 g++ -W -Wall main.cpp -o prog `pkg-config zipper --cflags --libs`
./prog

Zipper example
Input1
Input2
Zipper Finished
Unzipper Finished

 ls foo
Test1  Test2

cat foo/Test1 foo/Test2
../Test1.txt
../Test2.txt

[panosk]

@Lecrapouille , thanks for testing. I'm using zip files created with external tools (7zip), so I can extract them just fine with the tools. I also tried without a password but I still get the same problem. I'm using the master branch of Zipper (04-05-2021) and it links to zlib 1.2.11 that I compiled and installed on a Windows 10 machine.

[Lecrapouille]

Closed since implemented in the v2.x branch. Reopen it if needed