Skip to content

[MDEV-651] - fail when detecting duplicate file names #112

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

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java
Original file line number Diff line number Diff line change
Expand Up @@ -343,9 +343,17 @@ protected void extractFile( final File srcF, final File dir, final InputStream c

try
{
if ( !isOverwrite() && f.exists() && ( f.lastModified() >= entryDate.getTime() ) )
if ( !isOverwrite() && f.exists() )
Copy link
Member

Choose a reason for hiding this comment

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

The overwrite flag meaning is to indicate if "files are overwritten, even if they are newer than the corresponding entry in the archive.". The primary use case is when you're extracting the archive over existing directory - one created from previous build or from generated content in the current build. I don't see the logic to put the check here. As a user I would like to have the warning regardless if the file on the disk is overwritten or not - the warning is about that the content of the archive is not extracted properly (no matter if the file is overwritten or not the content of the destination folder will not be what I would expect) due to the file system limitation.

{
return;
final File entryFile = FileUtils.getFile( entryName );
if ( !StringUtils.equals( canonicalDestPath, entryFile.getAbsolutePath() ) )
Copy link
Member

@plamentotev plamentotev Apr 30, 2019

Choose a reason for hiding this comment

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

This check is not correct. First of all if you don't extract in the current directory it will always be true. For example when I ran one of the test cases canonicalDestPath is C:\git\plexus-archiver\target\zip-unarchiver-tests\resources\artifactId and entryFile.getAbsolutePath() is C:\git\plexus-archiver\resources\artifactId. Also I've made some quick tests on Windows and canonicalDestPath and entryFile.getAbsolutePath() are always with the same case. I do believe that is the case for all OSes as it is represent path not existing file (I could be wrong as the canonical path is system dependent) so it won't work even if the paths were resolved properly.

What is more the canonical and the absolute path could be different for a couple of reasons - including resolving symbolic links - just comparing the two strings is not reliable way to determine if they represent two files with the same name with different case of the letters.

{
throw new ArchiverException( "Duplicate files (" + canonicalDestPath + "," + entryFile.getAbsolutePath() + ")" );
}
else if ( f.lastModified() >= entryDate.getTime() )
{
return;
}
}

// create intermediary directories - sometimes zip don't add them
Expand Down