Skip to content

Commit

Permalink
Fix UnArchiver#isOverwrite not working as expected
Browse files Browse the repository at this point in the history 
Regression in efd980d changed the way
UnArchiver#isOverwrite flag work.
It is indented to indicate that UnArchiver should
always override the existing entries.
efd980d changed it to indicate whether existing file
should be overridden if the entry is newer,
while in this case the file should be always overridden.

This commit returns the old behavior: if the entry
is newer, override the existing file; if the entry
is older, override the existing file only if isOverwrite is true.

Fixes: #228
  • Loading branch information
@plamentotev
plamentotev committed Sep 6, 2022
1 parent 57d924b commit 8656ee0
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 32 deletions.
37 changes: 13 additions & 24 deletions src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java
View file
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();
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
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

0 comments on commit 8656ee0

Please sign in to comment.