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.

Sign up for GitHub

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

Fix UnArchiver#isOverwrite not working as expected #229

Merged
merged 1 commit into from
Sep 6, 2022
Merged

Fix UnArchiver#isOverwrite not working as expected #229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 13 additions & 24 deletions src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,16 +401,18 @@ else if ( isDirectory )
protected boolean shouldExtractEntry( File targetDirectory, File targetFileName, String entryName, Date entryDate ) throws IOException
{
// entryname | entrydate | filename | filedate | behavior
// (1) readme.txt | 1970 | readme.txt | 2020 | never overwrite
// (2) readme.txt | 2020 | readme.txt | 1970 | only overwrite when isOverWrite()
// (3) README.txt | 1970 | readme.txt | 2020 | case-insensitive filesystem: warn + never overwrite
// (1) readme.txt | 1970 | - | - | always extract if the file does not exist
// (2) readme.txt | 1970 | readme.txt | 2020 | do not overwrite unless isOverwrite() is true
// (3) readme.txt | 2020 | readme.txt | 1970 | always override when the file is older than the archive entry
// (4) README.txt | 1970 | readme.txt | 2020 | case-insensitive filesystem: warn + do not overwrite unless isOverwrite()
// case-sensitive filesystem: extract without warning
// (4) README.txt | 2020 | readme.txt | 1970 | case-insensitive filesystem: warn + only overwrite when isOverWrite()
// (5) README.txt | 2020 | readme.txt | 1970 | case-insensitive filesystem: warn + overwrite because entry is newer
// case-sensitive filesystem: extract without warning

// The canonical file name follows the name of the archive entry, but takes into account the case-
// sensitivity of the filesystem. So on a case-sensitive file system, file.exists() returns false for
// scenario (3) and (4).
// scenario (4) and (5).
// No matter the case sensitivity of the file system, file.exists() returns false when there is no file with the same name (1).
if ( !targetFileName.exists() )
{
return true;
Expand All @@ -423,33 +425,20 @@ protected boolean shouldExtractEntry( File targetDirectory, File targetFileName,
targetDirectory.getCanonicalPath() + File.separatorChar,
"" )
+ suffix;
boolean fileOnDiskIsNewerThanEntry = targetFileName.lastModified() >= entryDate.getTime();
boolean fileOnDiskIsOlderThanEntry = targetFileName.lastModified() < entryDate.getTime();
plamentotev marked this conversation as resolved.
Show resolved Hide resolved
boolean differentCasing = !entryName.equals( relativeCanonicalDestPath );

String casingMessage = String.format( Locale.ENGLISH, "Archive entry '%s' and existing file '%s' names differ only by case."
+ " This may lead to an unexpected outcome on case-insensitive filesystems.", entryName, canonicalDestPath );

// (1)
if ( fileOnDiskIsNewerThanEntry )
{
// (3)
if ( differentCasing )
{
getLogger().warn( casingMessage );
casingMessageEmitted.incrementAndGet();
}
return false;
}

// (4)
// Warn for case (4) and (5) if the file system is case-insensitive
if ( differentCasing )
{
String casingMessage = String.format( Locale.ENGLISH, "Archive entry '%s' and existing file '%s' names differ only by case."
+ " This may lead to an unexpected outcome on case-insensitive filesystems.", entryName, canonicalDestPath );
getLogger().warn( casingMessage );
casingMessageEmitted.incrementAndGet();
}

// (2)
return isOverwrite();
// Override the existing file if isOverwrite() is true or if the file on disk is older than the one in the archive
return isOverwrite() || fileOnDiskIsOlderThanEntry;
}

}
20 changes: 12 additions & 8 deletions src/test/java/org/codehaus/plexus/archiver/AbstractUnArchiverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,14 @@ public void shouldExtractWhenFileOnDiskDoesNotExist( @TempDir File temporaryFol
Date entryDate = new Date();

// when & then
// always extract the file if it does not exist
assertTrue( abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
abstractUnArchiver.setOverwrite( false );
assertTrue( abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
}

@Test
public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive( @TempDir File temporaryFolder ) throws IOException
public void shouldExtractWhenFileOnDiskIsNewerThanEntryInArchive( @TempDir File temporaryFolder ) throws IOException
{
// given
File file = new File( temporaryFolder, "whatever.txt" );
Expand All @@ -105,11 +108,14 @@ public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive( @TempDir Fi
Date entryDate = new Date( 0 );

// when & then
// if the file is newer than archive entry, extract only if overwrite is true (default)
assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
abstractUnArchiver.setOverwrite( false );
assertFalse( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
}

@Test
public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive_andWarnAboutDifferentCasing( @TempDir File temporaryFolder )
public void shouldExtractWhenFileOnDiskIsNewerThanEntryInArchive_andWarnAboutDifferentCasing( @TempDir File temporaryFolder )
throws IOException
{
// given
Expand All @@ -120,7 +126,7 @@ public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive_andWarnAbout
Date entryDate = new Date( 0 );

// when & then
assertFalse( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
assertTrue( this.abstractUnArchiver.casingMessageEmitted.get() > 0 );
}

Expand All @@ -135,12 +141,10 @@ public void shouldExtractWhenEntryInArchiveIsNewerThanFileOnDisk( @TempDir File
Date entryDate = new Date( System.currentTimeMillis() );

// when & then
this.abstractUnArchiver.setOverwrite( true );
// always extract the file if the archive entry is newer than the file on disk
assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );

// when & then
this.abstractUnArchiver.setOverwrite( false );
assertFalse( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
}

@Test
Expand All @@ -158,7 +162,7 @@ public void shouldExtractWhenEntryInArchiveIsNewerThanFileOnDiskAndWarnAboutDiff
this.abstractUnArchiver.setOverwrite( true );
assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
this.abstractUnArchiver.setOverwrite( false );
assertFalse( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
assertTrue( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder, file, entryname, entryDate ) );
assertTrue( this.abstractUnArchiver.casingMessageEmitted.get() > 0 );
}

Expand Down