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

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

Closed
mintern opened this issue Sep 6, 2022 · 4 comments
Assignees
Labels
bug Something isn't working resolved

Comments

@mintern
Copy link

mintern commented Sep 6, 2022

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. 0f99010 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
Copy link
Owner

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

mintern commented Sep 7, 2022

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

Thanks for pointing out. I fixed it.

@srikanth-lingala
Copy link
Owner

Fix included in v2.11.2 released today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working resolved
Projects
None yet
Development

No branches or pull requests

2 participants