srikanth-lingala / zip4j

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

Unzipping deletes files that are not writable #501

Open creswick opened 1 year ago

creswick commented 1 year ago

Apologies for the lack of a canned minimal test case. It's also possible this has been fixed in more recent versions, but the control flow is complex enough that I wouldn't be surprised if this hasn't come up much (it only happens if you extract the same archive twice, more or less). I observed the problem with v2.9.

Reproduction:

  1. Create a zip that contains a file with permissions that don't include write. (e.g. chmod a-w <file> then zip it up).
  2. Unzip that zip with zip4j (e.g. new ZipFile(file).extractAll(someDestiation))
  3. Unzip that zip again, to the same location.

The file that was not writable will be removed, and the second unzip operation will fail.

This seems to be because of the way exceptions are handled here: https://github.com/srikanth-lingala/zip4j/blob/master/src/main/java/net/lingala/zip4j/tasks/AbstractExtractFileTask.java#L116

  private void unzipFile(ZipInputStream inputStream, File outputFile, ProgressMonitor progressMonitor, byte[] buff)
      throws IOException {

    int readLength;
    try (OutputStream outputStream = new FileOutputStream(outputFile)) {
      while ((readLength = inputStream.read(buff)) != -1) {
        outputStream.write(buff, 0, readLength);
        progressMonitor.updateWorkCompleted(readLength);
        verifyIfTaskIsCancelled();
      }
    } catch (Exception e) {
      if (outputFile.exists()) {
        outputFile.delete(); 
      }
      throw  e;  /// <------ This exception does not appear to be caught in the call stack, so the unzip operation on the file is never retried.
    }
  }

There are probably a few opinions about what this should do, but I think the current behavior where it deletes the evidence and then throws an exception is not ideal.

The exception could probably be caught higher in the stack, and the unzipFile call for that file retried, and I think this would do what the unzip program does.