[MDEV-651] - fail when detecting duplicate file names #112
Conversation
(cherry picked from commit 412e800cb93c9f739cfa29e4c0b0b55ad8a73e22)
There was a problem hiding this comment.
Hi,
Thanks for the contribution. As a user using Windows (so case insensitive file system) I would agree that would be nice to have a log message saying that an archive content was not properly extracted because the file system is case insensitive and the archive contains files that differ only in their case. But I would rather not fail the build. The behavior of Plexus Archiver is platform dependent(for example the unit test are failing on Windows). Some file system are case-insensitive, other does not have permissions, symbolic links, etc. Plexus Archiver does best effort to extract the content correctly but does not grantee it. Of course one may argue that in this case file is lost and the problem is more serious but even with that in mind I'm would be careful to fail the build for all users by default and without a flag to control it.
As for the purposed changes - I don't think they work. Please take a look at my comment in-line. Also would be nice to have a test cases that test your code.
| @@ -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() ) | |||
There was a problem hiding this comment.
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() ) ) |
There was a problem hiding this comment.
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.
|
Superseeded by #128 |
(cherry picked from commit 412e800cb93c9f739cfa29e4c0b0b55ad8a73e22)