Ensure that if entryName contains / char they are replaced by "File.separator" #180
Conversation
| public void shouldExtractWhenCasingDifferOnlyInEntryNamePath() throws IOException | ||
| { | ||
| // given | ||
| File file = new File( temporaryFolder.getRoot() + File.separator + "folder", "whatever.txt" ); // does not create the file! |
The current code assumes that if canonical path (relative to the destination directory) is not the same as entry name, then the only cause of the difference could be that the files are with different cases. Which does not seems to be always true. One improvement that we can do is do add additional check - if the canonical path (relative to the destination directory) is not the same as entry name, compare them but this time case insensitive. This will eliminate the false positives in case we have missed some other normalization that may cause the two names to be different. But I wonder if it will introduce additional false negatives. @cedmail just to be sure my understanding is correct. Even with the current version the file is extracted properly, just a warning is printed, right? Or there is issue with the extracted file. And I guess the warning is showed then the file system separator is not the same as the one user in the zip file, right? |
|
And interestingly enough the windows tests cases are failing. |
@plamentotev Yes the issue is that when unarchive a file of type zip (WAR) into a Windows system, it works smoothly it target directory is empty, but if you repeat the operation you get a warning for every file in archive (thousands of them) |
|
@plamentotev Do we want to settle this for the next version?` |
I didn't have to look at it. I'll try to look into it this or next week. |
|
This really spams the log a lot. Any chance to fix this? |
| @@ -409,7 +409,7 @@ protected boolean shouldExtractEntry( File targetDirectory, File targetFileName, | |||
| "" ) | |||
| + suffix; | |||
| boolean fileOnDiskIsNewerThanEntry = targetFileName.lastModified() >= entryDate.getTime(); | |||
| boolean differentCasing = !entryName.equals( relativeCanonicalDestPath ); | |||
| boolean differentCasing = !entryName.replace("/", File.separator).equals( relativeCanonicalDestPath ); | |||
There was a problem hiding this comment.
This change is wrong. Both strings would need to have "/" be replaced by File.separator if you want to look for different casing.
Since updating to plexus archiver 4.2.5 (from version 2) we have seen those warning:
[WARNING] Archive entry 'META-INF/MANIFEST.MF' and existing file 'C:\Dev\rt\master\tomcat\webapps\jahia\META-INF\MANIFEST.MF' names differ only by case. This may lead to an unexpected outcome on case-insensitive filesystems.
Issue here is that the AbtractUnArchiver assumes entryName is just a file but in a ZipArchive entryName can be a path and so when evaluating if we should extract, we should replace "/" char (Zip always use "/" by spec) by the File OS separator