Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impossible to write into temporary file using zip4j #493

Closed
mgarin opened this issue Feb 20, 2023 · 3 comments
Closed

Impossible to write into temporary file using zip4j #493

mgarin opened this issue Feb 20, 2023 · 3 comments

Comments

@mgarin
Copy link

mgarin commented Feb 20, 2023

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
Copy link
Owner

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.

@srikanth-lingala srikanth-lingala self-assigned this Feb 20, 2023
@srikanth-lingala srikanth-lingala added bug Something isn't working resolved improvement and removed bug Something isn't working labels Feb 20, 2023
@mgarin
Copy link
Author

mgarin commented Feb 21, 2023

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

@srikanth-lingala
Copy link
Owner

Fixed in v2.11.5 released today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants