[MDEP-651] Warn on duplicate archive entries

src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java

@@ -343,7 +343,7 @@ protected void extractFile( final File srcF, final File dir, final InputStream c
 
         try
         {
-            if ( !isOverwrite() && f.exists() && ( f.lastModified() >= entryDate.getTime() ) )
+            if ( !shouldExtractEntry( f, entryName, entryDate ) )
             {
                 return;
             }
@@ -384,4 +384,39 @@ else if ( isDirectory )
         }
     }
 
+    // Visible for testing
+    protected boolean shouldExtractEntry( File file, String entryName, Date entryDate ) throws IOException
+    {
+        String canonicalDestPath = file.getCanonicalPath();
+        boolean fileOnDiskIsNewerThanEntry = ( file.lastModified() >= entryDate.getTime() );
+        boolean differentCasing = !StringUtils.equalsIgnoreCase( entryName, canonicalDestPath );
+
+        String casingMessage = String.format( "Archive entry '%s' and existing file '%s' names differ only by case."
+                + " This may cause issues on case-insensitive filesystems.", entryName, canonicalDestPath );
+
+        // Optimise for situation where we need no checks
+        if ( !file.exists() )
+        {
+            return true;
+        }
+
+        // (1)
+        if ( fileOnDiskIsNewerThanEntry )
+        {
+            // (3)
+            if ( differentCasing )
+            {
+                getLogger().warn( casingMessage );
+            }
+            return false;
+        }
+
+        // (4)
+        if ( differentCasing )
+        {
+            getLogger().warn( casingMessage );
+        }
+        return isOverwrite();
+    }
+
 }

src/test/java/org/codehaus/plexus/archiver/AbstractUnArchiverTest.java

@@ -19,13 +19,23 @@
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
+import java.util.Date;
 
 import org.codehaus.plexus.components.io.filemappers.FileMapper;
+import org.hamcrest.BaseMatcher;
+import org.hamcrest.CoreMatchers;
+import org.hamcrest.Description;
+import org.hamcrest.core.StringContains;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
+
+import static org.hamcrest.CoreMatchers.hasItem;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
 
 /**
  * Unit test for {@link AbstractUnArchiver}
@@ -37,7 +47,11 @@ public class AbstractUnArchiverTest
     @Rule
     public ExpectedException thrown = ExpectedException.none();
 
+    @Rule
+    public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
     private AbstractUnArchiver abstractUnArchiver;
+    private CapturingLog log = new CapturingLog( 0 /*debug*/, "AbstractUnArchiver" );
 
     @Before
     public void setUp()
@@ -58,6 +72,7 @@ protected void execute()
                 // unused
             }
         };
+        this.abstractUnArchiver.enableLogging( log );
     }
 
     @After
@@ -72,7 +87,7 @@ public void shouldThrowExceptionBecauseRewrittenPathIsOutOfDirectory()
     {
         // given
         this.thrown.expectMessage( "Entry is outside of the target directory (../PREFIX/ENTRYNAME.SUFFIX)" );
-        final File targetFolder = Files.createTempDirectory( null ).toFile();
+        final File targetFolder = temporaryFolder.newFolder();
         final FileMapper[] fileMappers = new FileMapper[] { new FileMapper()
         {
             @Override
@@ -97,4 +112,93 @@ public String getMappedFileName( String pName )
         // ArchiverException is thrown providing the rewritten path
     }
 
+    @Test
+    public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive() throws IOException
+    {
+        // given
+        File file = temporaryFolder.newFile( "readme.txt" );
+        file.setLastModified( System.currentTimeMillis() );
+        String entryname = "readme.txt";
+        Date entryDate = new Date( 0 );
+
+        // when & then
+        assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is ( false ) );
+    }
+
+    @Test
+    public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive_andWarnAboutDifferentCasing() throws IOException
+    {
+        // given
+        File file = temporaryFolder.newFile( "readme.txt" );
+        file.setLastModified( System.currentTimeMillis() );
+        String entryname = "README.txt";
+        Date entryDate = new Date( 0 );
+
+        // when & then
+        assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is ( false ) );
+        assertThat( this.log.getWarns(), hasItem( new LogMessageMatcher( "names differ only by case" ) ) );
+    }
+
+    @Test
+    public void shouldExtractWhenEntryInArchiveIsNewerThanFileOnDisk() throws IOException
+    {
+        // given
+        File file = temporaryFolder.newFile( "readme.txt" );
+        file.setLastModified( 0 );
+        String entryname = "readme.txt";
+        Date entryDate = new Date( System.currentTimeMillis() );
+
+        // when & then
+        this.abstractUnArchiver.setOverwrite( true );
+        assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( true ) );
+
+        // when & then
+        this.abstractUnArchiver.setOverwrite( false );
+        assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( false ) );
+    }
+
+    @Test
+    public void shouldExtractWhenEntryInArchiveIsNewerThanFileOnDiskAndWarnAboutDifferentCasing() throws IOException
+    {
+        // given
+        File file = temporaryFolder.newFile( "readme.txt" );
+        file.setLastModified( 0 );
+        String entryname = "README.txt";
+        Date entryDate = new Date( System.currentTimeMillis() );
+
+        // when & then
+        this.abstractUnArchiver.setOverwrite( true );
+        assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( true ) );
+        this.abstractUnArchiver.setOverwrite( false );
+        assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( false ) );
+        assertThat( this.log.getWarns(), hasItem( new LogMessageMatcher( "names differ only by case" ) ) );
+    }
+
+    static class LogMessageMatcher extends BaseMatcher<CapturingLog.Message> {
+        private final StringContains delegateMatcher;
+
+        LogMessageMatcher( String needle )
+        {
+            this.delegateMatcher = new StringContains( needle );
+        }
+
+        @Override
+        public void describeTo( Description description )
+        {
+            description.appendText( "a log message with " );
+            delegateMatcher.describeTo( description );
+        }
+
+        @Override
+        public boolean matches( Object item )
+        {
+            if ( item instanceof CapturingLog.Message )
+            {
+                CapturingLog.Message message = (CapturingLog.Message) item;
+                String haystack = message.message;
+                return delegateMatcher.matches( haystack );
+            }
+            return false;
+        }
+    }
 }

src/test/java/org/codehaus/plexus/archiver/CapturingLog.java

@@ -0,0 +1,99 @@
+package org.codehaus.plexus.archiver;
+
+import org.codehaus.plexus.logging.AbstractLogger;
+import org.codehaus.plexus.logging.Logger;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class CapturingLog extends AbstractLogger
+{
+    public CapturingLog( int threshold, String name )
+    {
+        super( threshold, name );
+    }
+
+    static class Message {
+        public final String message;
+        public final Throwable throwable;
+
+        public Message( String message, Throwable throwable )
+        {
+            this.message = message;
+            this.throwable = throwable;
+        }
+
+        @Override
+        public String toString()
+        {
+            return "Message{" + "message='" + message + '\'' + ", throwable=" + throwable + '}';
+        }
+    }
+
+    private final List<Message> debugs = new ArrayList<>();
+    @Override
+    public void debug( String s, Throwable throwable )
+    {
+        debugs.add( new Message( s, throwable ) );
+    }
+
+    public List<Message> getDebugs()
+    {
+        return debugs;
+    }
+
+
+    private final List<Message> infos = new ArrayList<>();
+    @Override
+    public void info( String s, Throwable throwable )
+    {
+        infos.add( new Message( s, throwable ) );
+    }
+
+    public List<Message> getInfos()
+    {
+        return infos;
+    }
+
+    private final List<Message> warns = new ArrayList<>();
+    @Override
+    public void warn( String s, Throwable throwable )
+    {
+        warns.add( new Message( s, throwable ) );
+    }
+
+    public List<Message> getWarns()
+    {
+        return warns;
+    }
+
+    private final List<Message> errors = new ArrayList<>();
+    @Override
+    public void error( String s, Throwable throwable )
+    {
+        errors.add( new Message( s, throwable ) );
+    }
+
+    public List<Message> getErors()
+    {
+        return errors;
+    }
+
+    private final List<Message> fatals = new ArrayList<>();
+    @Override
+    public void fatalError( String s, Throwable throwable )
+    {
+        fatals.add( new Message( s, throwable ) );
+    }
+
+    public List<Message> getFatals()
+    {
+        return fatals;
+    }
+
+    @Override
+    public Logger getChildLogger( String s )
+    {
+        return null;
+    }
+}