srikanth-lingala / zip4j

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

ZipOutputStream.putNextEntry no longer sets last modified file time on file entries #473

Closed mintern closed 2 years ago

mintern commented 2 years ago

As of 2.11.0, file entries added via putNextEntry have a last modified date of 1980 when ZipParameters are passed with the default lastModifiedFileTime == 0 value. https://github.com/srikanth-lingala/zip4j/commit/0f9901076d10f4fd4319d3ee3eb1d00f136f581e removed some conditional logic from FileHeaderFactory.generateFileHeader:

-    if (zipParameters.getLastModifiedFileTime() > 0) {
-      fileHeader.setLastModifiedTime(Zip4jUtil.epochToExtendedDosTime(zipParameters.getLastModifiedFileTime()));
-    } else {
-      fileHeader.setLastModifiedTime(Zip4jUtil.epochToExtendedDosTime(System.currentTimeMillis()));
-    }
+    fileHeader.setLastModifiedTime(Zip4jUtil.epochToExtendedDosTime(zipParameters.getLastModifiedFileTime()));

This logic was added to putNextEntry instead, but only for directory entries:

     if (isZipEntryDirectory(zipParameters.getFileNameInZip())) {
       clonedZipParameters.setWriteExtendedLocalFileHeader(false);
       clonedZipParameters.setCompressionMethod(CompressionMethod.STORE);
       clonedZipParameters.setEncryptFiles(false);
       clonedZipParameters.setEntrySize(0);
+
+      if (zipParameters.getLastModifiedFileTime() <= 0) {
+        clonedZipParameters.setLastModifiedFileTime(System.currentTimeMillis());
+      }
     }

As a result, the following test, which passes for 2.10.0, fails for 2.11.0 and 2.11.1:

    @Test
    public void recentLastModifiedTime() throws IOException {
        long currentTime = System.currentTimeMillis();
        ByteArrayOutputStream zip = new ByteArrayOutputStream();
        try (ZipOutputStream zos = new ZipOutputStream(zip)) {
            ZipParameters params = new ZipParameters();
            params.setFileNameInZip("test");
            zos.putNextEntry(params);
            zos.write(new byte[0]);
            zos.closeEntry();
        }
        try (ZipInputStream zis = new ZipInputStream(new ByteArrayInputStream(zip.toByteArray()))) {
            ZipEntry e = zis.getNextEntry();
            long zipTime = e.getTime();
            // ZIP only has 2 second precision, so adjust for potential rounding.
            assertTrue(currentTime < zipTime + 2000,
                    String.format("ZIP entry modified time (%d) should be near current time (%d)",
                            zipTime, currentTime));
        }
    }

Was this change intended as part of the fix to #434, or was this an unintended side effect?

The documentation for ZipParameters.setLastModifiedTime says:

Set the last modified time recorded in the ZIP file for the added files. If less than 0, the last modified time is cleared and the current time is used

but the current time is not being used for file entries when zipParams.getLastModifiedFileTime() <= 0 like it used to.

(I noticed a separate potential bug where ZipParameters.setLastModifiedTime just returns when lastModifiedFileTime <= 0. It doesn't actually clear the current value as the documentation indicates.)

srikanth-lingala commented 2 years ago

I fixed both the issues you mentioned (including the one where ZipParameters.setLastModifiedTime doesn't clear the time). I modified your test case slightly and used it in the project. Thanks for the detailed issue report, I appreciate that.

mintern commented 2 years ago

Thank you for the quick fix! I'm glad my report and sample test were useful.

I'm not seeing any ZipParameters.setLastModifiedFileTime change (perhaps it wasn't git added?), but that was just an aside on my part. You fixed the issue that I was running into.

Thanks again!

srikanth-lingala commented 2 years ago

Thanks for pointing out. I fixed it.

srikanth-lingala commented 2 years ago

Fix included in v2.11.2 released today