Skip to content

Fix AbstractZipUnArchiver handling of zip entries with unspecified modification time#393

Merged
slachiewicz merged 1 commit intomasterfrom
copilot/fix-cc0aca1e-e9a0-4df7-90c3-f0c41afe4807
Oct 8, 2025
Merged

Fix AbstractZipUnArchiver handling of zip entries with unspecified modification time#393
slachiewicz merged 1 commit intomasterfrom
copilot/fix-cc0aca1e-e9a0-4df7-90c3-f0c41afe4807

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 5, 2025 •
edited
Loading

Problem

According to the ZIP specification, ZipEntry.getTime() can legally return -1 to indicate that the modification time is not specified. However, AbstractZipUnArchiver was passing this value directly to new Date(-1) and eventually to File.setLastModified(-1), which throws IllegalArgumentException: Negative time.

This caused extraction failures when attempting to unzip JAR files with entries that do not have a modification time set:

java.lang.IllegalArgumentException: Negative time
    at java.io.File.setLastModified (File.java:1441)
    at org.codehaus.plexus.archiver.AbstractUnArchiver.extractFile (AbstractUnArchiver.java:374)
    at org.codehaus.plexus.archiver.zip.AbstractZipUnArchiver.execute (AbstractZipUnArchiver.java:221)

Solution

Added checks for -1 modification time (in addition to the existing check for 0) in two locations:

  1. ZipEntryFileInfo.getLastModified(): Now returns PlexusIoResource.UNKNOWN_MODIFICATION_DATE (which is 0) when zipEntry.getTime() returns either -1 or 0

  2. AbstractZipUnArchiver.execute(): Sanitizes the modification time before creating the Date object by checking if it's -1 or 0, and replacing it with UNKNOWN_MODIFICATION_DATE

This ensures that File.setLastModified() is never called with a negative value, while properly handling zip entries with unspecified modification times according to the ZIP specification.

Testing

Added testZipWithNegativeModificationTime() which:

  • Creates a zip file with an entry having -1 modification time using Apache Commons Compress
  • Extracts it using ZipUnArchiver
  • Verifies the extraction succeeds and file content is correct

All existing tests continue to pass.

Fixes #262

Original prompt

This section details on the original issue you should resolve

<issue_title>AbstractZipUnArchiver does not check for unspecified modification time</issue_title>
<issue_description>Attempting to unzip a jar with entries which do not have a modification time fails. This is readily reproducible with this patch: https://git.opendaylight.org/gerrit/c/controller/+/95334 which ends up hitting this codepath via maven-dependency-plugin:

Caused by: java.lang.IllegalArgumentException: Negative time
    at java.io.File.setLastModified (File.java:1441)
    at org.codehaus.plexus.archiver.AbstractUnArchiver.extractFile (AbstractUnArchiver.java:374)
    at org.codehaus.plexus.archiver.zip.AbstractZipUnArchiver.execute (AbstractZipUnArchiver.java:221)
    at org.codehaus.plexus.archiver.zip.AbstractZipUnArchiver.execute (AbstractZipUnArchiver.java:180)
    at org.codehaus.plexus.archiver.AbstractUnArchiver.extract (AbstractUnArchiver.java:149)
    at org.apache.maven.plugins.dependency.AbstractDependencyMojo.unpack (AbstractDependencyMojo.java:313)
    at org.apache.maven.plugins.dependency.AbstractDependencyMojo.unpack (AbstractDependencyMojo.java:224)
    at org.apache.maven.plugins.dependency.fromDependencies.UnpackDependenciesMojo.doExecute (UnpackDependenciesMojo.java:107)
    at org.apache.maven.plugins.dependency.AbstractDependencyMojo.execute (AbstractDependencyMojo.java:144)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:137)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:208)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:154)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:146)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:81)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:56)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:128)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:305)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:956)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:290)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:194)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:566)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:289)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:229)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:415)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:356)

The problem seems to be here:
https://github.com/codehaus-plexus/plexus-archiver/blob/master/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java#L209

ZipEntry.getTime() can legally return -1 to indicate 'modification time is not specifed'. This is not checked and passed directly to Date(long), wfrom whence it is extracted verbatim and passed to File.setLastModified() -- where negative values are explicitly forbidden.</issue_description>

Comments on the Issue (you are @copilot in this section)

@plamentotev Do you know which file caused the issue? While `ZipEntry.getTime()` is allowed to return `-1` to indicate that no modification time was specified, I did a quick check and it should be specified (the value is always set). Did you debug why it is not set (or set to this value)? Maybe there is some other issue causing the value to not be set.
Fixes #170

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] AbstractZipUnArchiver does not check for unspecified modification time Fix AbstractZipUnArchiver handling of zip entries with unspecified modification time Oct 5, 2025
Copilot AI requested a review from slachiewicz October 5, 2025 14:05
@slachiewicz slachiewicz marked this pull request as ready for review October 5, 2025 14:09
@slachiewicz slachiewicz requested a review from hboutemy October 5, 2025 19:19
slawekjaranowski
slawekjaranowski reviewed Oct 6, 2025
View reviewed changes
Comment thread .gitignore Outdated
gnodet
gnodet reviewed Oct 8, 2025
View reviewed changes
Comment thread src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java Outdated
gnodet
gnodet reviewed Oct 8, 2025
View reviewed changes
Comment thread src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java Outdated
gnodet
gnodet reviewed Oct 8, 2025
View reviewed changes
Comment thread .gitignore Outdated
gnodet
gnodet requested changes Oct 8, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@gnodet gnodet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think checking for <= 0 is easier.

@gnodet gnodet mentioned this pull request Oct 8, 2025
Closed
@slachiewicz slachiewicz force-pushed the copilot/fix-cc0aca1e-e9a0-4df7-90c3-f0c41afe4807 branch from 7d46ae4 to 1008193 Compare October 8, 2025 09:29
@slachiewicz slachiewicz merged commit 6a62131 into master Oct 8, 2025
15 checks passed
@slachiewicz slachiewicz deleted the copilot/fix-cc0aca1e-e9a0-4df7-90c3-f0c41afe4807 branch October 8, 2025 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slawekjaranowski slawekjaranowski slawekjaranowski left review comments

@gnodet gnodet gnodet approved these changes

@hboutemy hboutemy Awaiting requested review from hboutemy

+1 more reviewer

@slachiewicz slachiewicz slachiewicz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@slachiewicz slachiewicz

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AbstractZipUnArchiver does not check for unspecified modification time

4 participants

@gnodet @slawekjaranowski @slachiewicz