Skip to content

Commit e5a465c

Browse files
committed
[MDEP-651] Restore check on timestamps
Also verify warning log message.
1 parent e2f6f4c commit e5a465c

File tree

3 files changed

+241
-8
lines changed

3 files changed

+241
-8
lines changed

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

+37-7
Original file line numberDiff line numberDiff line change
@@ -343,14 +343,9 @@ protected void extractFile( final File srcF, final File dir, final InputStream c
343343

344344
try
345345
{
346-
if ( f.exists() && !StringUtils.equalsIgnoreCase( entryName, canonicalDestPath ) )
346+
if ( !shouldExtractEntry( f, entryName, entryDate ) )
347347
{
348-
String message = String.format( "Archive entry '%s' and existing file '%s' names differ only by case."
349-
+ " This may cause issues on case-insensitive filesystems.", entryName, canonicalDestPath );
350-
getLogger().warn( message );
351-
if ( !isOverwrite() ) {
352-
return;
353-
}
348+
return;
354349
}
355350

356351
// create intermediary directories - sometimes zip don't add them
@@ -389,4 +384,39 @@ else if ( isDirectory )
389384
}
390385
}
391386

387+
// Visible for testing
388+
protected boolean shouldExtractEntry( File file, String entryName, Date entryDate ) throws IOException
389+
{
390+
String canonicalDestPath = file.getCanonicalPath();
391+
boolean fileOnDiskIsNewerThanEntry = ( file.lastModified() >= entryDate.getTime() );
392+
boolean differentCasing = !StringUtils.equalsIgnoreCase( entryName, canonicalDestPath );
393+
394+
String casingMessage = String.format( "Archive entry '%s' and existing file '%s' names differ only by case."
395+
+ " This may cause issues on case-insensitive filesystems.", entryName, canonicalDestPath );
396+
397+
// Optimise for situation where we need no checks
398+
if ( !file.exists() )
399+
{
400+
return true;
401+
}
402+
403+
// (1)
404+
if ( fileOnDiskIsNewerThanEntry )
405+
{
406+
// (3)
407+
if ( differentCasing )
408+
{
409+
getLogger().warn( casingMessage );
410+
}
411+
return false;
412+
}
413+
414+
// (4)
415+
if ( differentCasing )
416+
{
417+
getLogger().warn( casingMessage );
418+
}
419+
return isOverwrite();
420+
}
421+
392422
}

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

+105-1
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,23 @@
1919
import java.io.File;
2020
import java.io.IOException;
2121
import java.nio.file.Files;
22+
import java.util.Date;
2223

2324
import org.codehaus.plexus.components.io.filemappers.FileMapper;
25+
import org.hamcrest.BaseMatcher;
26+
import org.hamcrest.CoreMatchers;
27+
import org.hamcrest.Description;
28+
import org.hamcrest.core.StringContains;
2429
import org.junit.After;
2530
import org.junit.Before;
2631
import org.junit.Rule;
2732
import org.junit.Test;
2833
import org.junit.rules.ExpectedException;
34+
import org.junit.rules.TemporaryFolder;
35+
36+
import static org.hamcrest.CoreMatchers.hasItem;
37+
import static org.hamcrest.CoreMatchers.is;
38+
import static org.junit.Assert.assertThat;
2939

3040
/**
3141
* Unit test for {@link AbstractUnArchiver}
@@ -37,7 +47,11 @@ public class AbstractUnArchiverTest
3747
@Rule
3848
public ExpectedException thrown = ExpectedException.none();
3949

50+
@Rule
51+
public TemporaryFolder temporaryFolder = new TemporaryFolder();
52+
4053
private AbstractUnArchiver abstractUnArchiver;
54+
private CapturingLog log = new CapturingLog( 0 /*debug*/, "AbstractUnArchiver" );
4155

4256
@Before
4357
public void setUp()
@@ -58,6 +72,7 @@ protected void execute()
5872
// unused
5973
}
6074
};
75+
this.abstractUnArchiver.enableLogging( log );
6176
}
6277

6378
@After
@@ -72,7 +87,7 @@ public void shouldThrowExceptionBecauseRewrittenPathIsOutOfDirectory()
7287
{
7388
// given
7489
this.thrown.expectMessage( "Entry is outside of the target directory (../PREFIX/ENTRYNAME.SUFFIX)" );
75-
final File targetFolder = Files.createTempDirectory( null ).toFile();
90+
final File targetFolder = temporaryFolder.newFolder();
7691
final FileMapper[] fileMappers = new FileMapper[] { new FileMapper()
7792
{
7893
@Override
@@ -97,4 +112,93 @@ public String getMappedFileName( String pName )
97112
// ArchiverException is thrown providing the rewritten path
98113
}
99114

115+
@Test
116+
public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive() throws IOException
117+
{
118+
// given
119+
File file = temporaryFolder.newFile( "readme.txt" );
120+
file.setLastModified( System.currentTimeMillis() );
121+
String entryname = "readme.txt";
122+
Date entryDate = new Date( 0 );
123+
124+
// when & then
125+
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is ( false ) );
126+
}
127+
128+
@Test
129+
public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive_andWarnAboutDifferentCasing() throws IOException
130+
{
131+
// given
132+
File file = temporaryFolder.newFile( "readme.txt" );
133+
file.setLastModified( System.currentTimeMillis() );
134+
String entryname = "README.txt";
135+
Date entryDate = new Date( 0 );
136+
137+
// when & then
138+
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is ( false ) );
139+
assertThat( this.log.getWarns(), hasItem( new LogMessageMatcher( "names differ only by case" ) ) );
140+
}
141+
142+
@Test
143+
public void shouldExtractWhenEntryInArchiveIsNewerThanFileOnDisk() throws IOException
144+
{
145+
// given
146+
File file = temporaryFolder.newFile( "readme.txt" );
147+
file.setLastModified( 0 );
148+
String entryname = "readme.txt";
149+
Date entryDate = new Date( System.currentTimeMillis() );
150+
151+
// when & then
152+
this.abstractUnArchiver.setOverwrite( true );
153+
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( true ) );
154+
155+
// when & then
156+
this.abstractUnArchiver.setOverwrite( false );
157+
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( false ) );
158+
}
159+
160+
@Test
161+
public void shouldExtractWhenEntryInArchiveIsNewerThanFileOnDiskAndWarnAboutDifferentCasing() throws IOException
162+
{
163+
// given
164+
File file = temporaryFolder.newFile( "readme.txt" );
165+
file.setLastModified( 0 );
166+
String entryname = "README.txt";
167+
Date entryDate = new Date( System.currentTimeMillis() );
168+
169+
// when & then
170+
this.abstractUnArchiver.setOverwrite( true );
171+
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( true ) );
172+
this.abstractUnArchiver.setOverwrite( false );
173+
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( false ) );
174+
assertThat( this.log.getWarns(), hasItem( new LogMessageMatcher( "names differ only by case" ) ) );
175+
}
176+
177+
static class LogMessageMatcher extends BaseMatcher<CapturingLog.Message> {
178+
private final StringContains delegateMatcher;
179+
180+
LogMessageMatcher( String needle )
181+
{
182+
this.delegateMatcher = new StringContains( needle );
183+
}
184+
185+
@Override
186+
public void describeTo( Description description )
187+
{
188+
description.appendText( "a log message with " );
189+
delegateMatcher.describeTo( description );
190+
}
191+
192+
@Override
193+
public boolean matches( Object item )
194+
{
195+
if ( item instanceof CapturingLog.Message )
196+
{
197+
CapturingLog.Message message = (CapturingLog.Message) item;
198+
String haystack = message.message;
199+
return delegateMatcher.matches( haystack );
200+
}
201+
return false;
202+
}
203+
}
100204
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package org.codehaus.plexus.archiver;
2+
3+
import org.codehaus.plexus.logging.AbstractLogger;
4+
import org.codehaus.plexus.logging.Logger;
5+
6+
import java.util.ArrayList;
7+
import java.util.List;
8+
9+
public class CapturingLog extends AbstractLogger
10+
{
11+
public CapturingLog( int threshold, String name )
12+
{
13+
super( threshold, name );
14+
}
15+
16+
static class Message {
17+
public final String message;
18+
public final Throwable throwable;
19+
20+
public Message( String message, Throwable throwable )
21+
{
22+
this.message = message;
23+
this.throwable = throwable;
24+
}
25+
26+
@Override
27+
public String toString()
28+
{
29+
return "Message{" + "message='" + message + '\'' + ", throwable=" + throwable + '}';
30+
}
31+
}
32+
33+
private final List<Message> debugs = new ArrayList<>();
34+
@Override
35+
public void debug( String s, Throwable throwable )
36+
{
37+
debugs.add( new Message( s, throwable ) );
38+
}
39+
40+
public List<Message> getDebugs()
41+
{
42+
return debugs;
43+
}
44+
45+
46+
private final List<Message> infos = new ArrayList<>();
47+
@Override
48+
public void info( String s, Throwable throwable )
49+
{
50+
infos.add( new Message( s, throwable ) );
51+
}
52+
53+
public List<Message> getInfos()
54+
{
55+
return infos;
56+
}
57+
58+
private final List<Message> warns = new ArrayList<>();
59+
@Override
60+
public void warn( String s, Throwable throwable )
61+
{
62+
warns.add( new Message( s, throwable ) );
63+
}
64+
65+
public List<Message> getWarns()
66+
{
67+
return warns;
68+
}
69+
70+
private final List<Message> errors = new ArrayList<>();
71+
@Override
72+
public void error( String s, Throwable throwable )
73+
{
74+
errors.add( new Message( s, throwable ) );
75+
}
76+
77+
public List<Message> getErors()
78+
{
79+
return errors;
80+
}
81+
82+
private final List<Message> fatals = new ArrayList<>();
83+
@Override
84+
public void fatalError( String s, Throwable throwable )
85+
{
86+
fatals.add( new Message( s, throwable ) );
87+
}
88+
89+
public List<Message> getFatals()
90+
{
91+
return fatals;
92+
}
93+
94+
@Override
95+
public Logger getChildLogger( String s )
96+
{
97+
return null;
98+
}
99+
}

0 commit comments

Comments
 (0)