srikanth-lingala / zip4j

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

Add an option to use an existing file when creating new zip archive, or provide a utility method for generating Temp archives #476

Closed mdindoffer closed 1 year ago

mdindoffer commented 2 years ago

A very common usecase is to zip some files into a temporary archive. The most straightforward way to do this would be to create a temp file and then feed it to zip4j:

Path tmpFilePath = Files.createTempFile("tmpFilePrefix", ".zip");
ZipFile zipFile = new ZipFile(tmpTestDownload.toFile());

This however fails with:

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)

because the API expects the passed file to be a valid ZIP archive. This is really a pitfall of trying to create opaque API that magically tries to either open a zip file or create one based on hidden variables.

Nevertheless, there are two possible ways to solve this problem:

  1. Add a flag to the ZipFile constructor to allow reuse of existing files by means of overwriting them
  2. Adding another constructor, or a factory method, to create a ZipFile backed by a new temporary file, e.g. ZipFile.createTempArchive(String prefix)
tangyk commented 1 year ago

My solution for avoiding this problem

    // create temp file and then delete it 
    File tempZipFile = File.createTempFile("xx", ".zip");
    String path = tempZipFile.toString();
    tempZipFile.delete();

    ZipFile zipFile = new ZipFile(path);

It's a little ridiculous, but works for me

srikanth-lingala commented 1 year ago

because the API expects the passed file to be a valid ZIP archive

if the zip file does not exist, zip4j will not try to read or open it. If the zip file exists, it has to be in a valid zip format, otherwise zip4j cannot add more content to the zip file. My guess is that your temp file already exists, and is not a valid zip file, and you get this error. Try using the suggestion above: delete the temp file (if your use case permits) before adding content to it. But if the file exists, it has to a valid zip file.

mdindoffer commented 1 year ago

Umm, what? This is not a question, I was reporting a UX bug. The behaviour you describe is exactly the problem here. It should be fixed, like I describe in the OP. The API is badly designed, because the assumption of using only a nonexistent file to facilitate writing is just flawed.

If you delete a file before writing to it - why even create it in the first place? Those are not atomic operations, someone might take up the filename in the meantime, while you fiddle with deleting and recreating the file.

Once again, there are 2 backwards-compatible solutions to this:

  1. Add a flag to the ZipFile constructor to allow reuse of existing files by means of overwriting them
  2. Add another constructor, or a factory method, to create a ZipFile backed by a new temporary file, e.g. ZipFile.createTempArchive(String prefix)
srikanth-lingala commented 1 year ago

Fixed in v2.11.5 released today. Zip4j now allows overriding empty files and doesn't throw that exception as explained here

@mdindoffer It really helps if people are polite around here. I am trying to do my best to help people in my extremely limited spare time. If you have any problem using this library, feel free to use a different one or you can contribute with a pull request.