srikanth-lingala / zip4j

A Java library for zip files and streams
Apache License 2.0
2.06k stars 310 forks source link

Impossible to write into temporary file using zip4j #493

Closed mgarin closed 1 year ago

mgarin commented 1 year ago

Here is a small SSCCE that I tried to run on latest zip4j version (2.11.4):

package com.test;

import net.lingala.zip4j.ZipFile;
import net.lingala.zip4j.model.ZipParameters;

import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

public class Zip4jExample
{
    public static void main ( String[] args ) throws IOException, NoSuchMethodException, InvocationTargetException, IllegalAccessException
    {
        final Path tempFile = Files.createTempFile ( "bundle", ".zip" );

        try ( final ZipFile zipFile = new ZipFile ( tempFile.toFile () ) )
        {
            try ( final InputStream inputStream = Files.newInputStream ( Paths.get ( "C:\\file.txt" ) ) )
            {
                final ZipParameters parameters = new ZipParameters ();
                parameters.setFileNameInZip ( "file.txt" );
                zipFile.addStream (
                        inputStream,
                        parameters
                );
            }
        }

        System.out.println ( tempFile );
    }
}

This results in exception:

Exception in thread "main" net.lingala.zip4j.exception.ZipException: Zip file size less than minimum expected zip file size. Probably not a zip file or a corrupted zip file
    at net.lingala.zip4j.headers.HeaderReader.readAllHeaders(HeaderReader.java:70)
    at net.lingala.zip4j.ZipFile.readZipInfo(ZipFile.java:1135)
    at net.lingala.zip4j.ZipFile.addStream(ZipFile.java:418)
    at com.test.Zip4jExample.main(Zip4jExample.java:25)

I've looked into the zip4j source code and it seems that whenever file physically exists on the disk - zip4j will always try to read information about zip archive (more specifically - it's headers via HeaderReader) and fail with an exception if it is unable to read it.

The issue is that I see no valid way to use zip4j to write into/overwrite existing file. It is especially problematic with temporary files as standard Java API automatically creates those files on the disk - both older File.createTempFile(...) and newer Files.createTempFile(...). To make it work - I would have to manually delete that temporary file from disk while keeping its path to reuse later in zip4j's ZipFile. This creates unnecessary overhead every time I need to create an archive in temporary folder.

From what I can see - it is possible to "hack" around the issue by preemptively creating new empty model via reflection:

public class Zip4jExample
{
    public static void main ( String[] args ) throws IOException, NoSuchMethodException, InvocationTargetException, IllegalAccessException
    {
        final Path tempFile = Files.createTempFile ( "bundle", ".zip" );

        try ( final ZipFile zipFile = new ZipFile ( tempFile.toFile () ) )
        {
            final Method createNewZipModel = zipFile.getClass ().getDeclaredMethod ( "createNewZipModel" );
            createNewZipModel.setAccessible ( true );
            createNewZipModel.invoke ( zipFile );

            try ( final InputStream inputStream = Files.newInputStream ( Paths.get ( "C:\\file.txt" ) ) )
            {
                final ZipParameters parameters = new ZipParameters ();
                parameters.setFileNameInZip ( "file.txt" );
                zipFile.addStream (
                        inputStream,
                        parameters
                );
            }
        }

        System.out.println ( tempFile );
    }
}

I can see why it is safer to assume that if file exists - it may already be a zip archive and may contain some files. But having some flag/setting in ZipFile - probably in a separate constructor - to force new model creation for effectively "overwrite" mode could be very handy for situations like the one I've mentioned above.

But maybe I'm missing something and there is already a way to do that in the current version, although I didn't find one even after looking through the source code of ZipFile and related classes.

srikanth-lingala commented 1 year ago

I can see why it is safer to assume that if file exists - it may already be a zip archive and may contain some files.

That is right. To add content to an existing file it has to be a zip file, and therefore the check.

However, I fixed this by allowing content to be added to empty files. I will include this fix in the next release.

mgarin commented 1 year ago

Thank you for the quick changes! That should definitely do the trick for most common use-cases.

srikanth-lingala commented 1 year ago

Fixed in v2.11.5 released today.