From dec7fd3f01e94d2de4285a178fab23c41dd9b6c0 Mon Sep 17 00:00:00 2001 From: Plamen Totev Date: Sat, 3 Sep 2022 12:07:31 +0300 Subject: [PATCH] Fix UnArchiver#isOverwrite not working as expected 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 --- .../plexus/archiver/AbstractUnArchiver.java | 37 +++++++------------ .../archiver/AbstractUnArchiverTest.java | 20 ++++++---- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java index ab78577fa..2fa207240 100644 --- a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java @@ -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; @@ -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; } } diff --git a/src/test/java/org/codehaus/plexus/archiver/AbstractUnArchiverTest.java b/src/test/java/org/codehaus/plexus/archiver/AbstractUnArchiverTest.java index 41b4fdebf..b5bfef2f1 100644 --- a/src/test/java/org/codehaus/plexus/archiver/AbstractUnArchiverTest.java +++ b/src/test/java/org/codehaus/plexus/archiver/AbstractUnArchiverTest.java @@ -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" ); @@ -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 @@ -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 ); } @@ -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 @@ -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 ); }