Skip to content

Commit cf7c6d0

Browse files
mthmuldersmthmulders
committed
[MDEP-651] Optimise for situations where there is no conflict, and fix detecting of casing conflicts
1 parent b6f5499 commit cf7c6d0

File tree

2 files changed

+46
-29
lines changed

2 files changed

+46
-29
lines changed

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

+20-13
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import java.io.IOException;
2323
import java.io.InputStream;
2424
import java.io.OutputStream;
25+
import java.nio.file.Files;
26+
import java.nio.file.Paths;
2527
import java.util.ArrayList;
2628
import java.util.Date;
2729
import java.util.List;
@@ -343,7 +345,7 @@ protected void extractFile( final File srcF, final File dir, final InputStream c
343345

344346
try
345347
{
346-
if ( !shouldExtractEntry( targetFileName, entryName, entryDate ) )
348+
if ( !shouldExtractEntry( dir, targetFileName, entryName, entryDate ) )
347349
{
348350
return;
349351
}
@@ -365,7 +367,7 @@ else if ( isDirectory )
365367
}
366368
else
367369
{
368-
try ( OutputStream out = new FileOutputStream( f ) )
370+
try ( OutputStream out = new FileOutputStream( targetFileName ) )
369371
{
370372
IOUtil.copy( compressedInputStream, out );
371373
}
@@ -385,26 +387,31 @@ else if ( isDirectory )
385387
}
386388

387389
// Visible for testing
388-
protected boolean shouldExtractEntry( File targetFileName, String entryName, Date entryDate ) throws IOException
390+
protected boolean shouldExtractEntry( File targetDirectory, File targetFileName, String entryName, Date entryDate ) throws IOException
389391
{
390392
// entryname | entrydate | filename | filedate | behavior
391393
// (1) readme.txt | 1970 | readme.txt | 2020 | never overwrite
392394
// (2) readme.txt | 2020 | readme.txt | 1970 | only overwrite when isOverWrite()
393-
// (3) README.txt | 1970 | readme.txt | 2020 | warn + never overwrite
394-
// (4) README.txt | 2020 | readme.txt | 1970 | warn + only overwrite when isOverWrite()
395+
// (3) README.txt | 1970 | readme.txt | 2020 | case-insensitive filesystem: warn + never overwrite
396+
// case-sensitive filesystem: extract without warning
397+
// (4) README.txt | 2020 | readme.txt | 1970 | case-insensitive filesystem: warn + only overwrite when isOverWrite()
398+
// case-sensitive filesystem: extract without warning
399+
400+
// The canonical file name follows the name of the archive entry, but takes into account the case-
401+
// sensitivity of the filesystem. So on a case-sensitive file system, file.exists() returns false for
402+
// scenario (3) and (4).
403+
if ( !targetFileName.exists() )
404+
{
405+
return true;
406+
}
395407

396408
String canonicalDestPath = targetFileName.getCanonicalPath();
409+
String relativeCanonicalDestPath = canonicalDestPath.replace( targetDirectory.getCanonicalPath() + File.separatorChar, "" );
397410
boolean fileOnDiskIsNewerThanEntry = targetFileName.lastModified() >= entryDate.getTime();
398-
boolean differentCasing = !StringUtils.equalsIgnoreCase( entryName, canonicalDestPath );
411+
boolean differentCasing = !entryName.equals( relativeCanonicalDestPath );
399412

400413
String casingMessage = String.format( "Archive entry '%s' and existing file '%s' names differ only by case."
401-
+ " This may cause issues on case-insensitive filesystems.", entryName, canonicalDestPath );
402-
403-
// Optimise for situation where we need no checks
404-
if ( !targetFileName.exists() )
405-
{
406-
return true;
407-
}
414+
+ " This may lead to an unexpected outcome on case-insensitive filesystems.", entryName, canonicalDestPath );
408415

409416
// (1)
410417
if ( fileOnDiskIsNewerThanEntry )

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

+26-16
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,10 @@
1818

1919
import java.io.File;
2020
import java.io.IOException;
21-
import java.nio.file.Files;
2221
import java.util.Date;
2322

2423
import org.codehaus.plexus.components.io.filemappers.FileMapper;
2524
import org.hamcrest.BaseMatcher;
26-
import org.hamcrest.CoreMatchers;
2725
import org.hamcrest.Description;
2826
import org.hamcrest.core.StringContains;
2927
import org.junit.After;
@@ -112,65 +110,77 @@ public String getMappedFileName( String pName )
112110
// ArchiverException is thrown providing the rewritten path
113111
}
114112

113+
@Test
114+
public void shouldExtractWhenFileOnDiskDoesNotExist() throws IOException
115+
{
116+
// given
117+
File file = new File( temporaryFolder.getRoot(), "whatever.txt" ); // does not create the file!
118+
String entryname = file.getName();
119+
Date entryDate = new Date();
120+
121+
// when & then
122+
assertThat( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder.getRoot(), file, entryname, entryDate ), is ( true ) );
123+
}
124+
115125
@Test
116126
public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive() throws IOException
117127
{
118128
// given
119-
File file = temporaryFolder.newFile( "readme.txt" );
129+
File file = temporaryFolder.newFile();
120130
file.setLastModified( System.currentTimeMillis() );
121-
String entryname = "readme.txt";
131+
String entryname = file.getName();
122132
Date entryDate = new Date( 0 );
123133

124134
// when & then
125-
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is ( false ) );
135+
assertThat( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder.getRoot(), file, entryname, entryDate ), is ( false ) );
126136
}
127137

128138
@Test
129139
public void shouldNotExtractWhenFileOnDiskIsNewerThanEntryInArchive_andWarnAboutDifferentCasing() throws IOException
130140
{
131141
// given
132-
File file = temporaryFolder.newFile( "readme.txt" );
142+
File file = temporaryFolder.newFile();
133143
file.setLastModified( System.currentTimeMillis() );
134-
String entryname = "README.txt";
144+
String entryname = file.getName().toUpperCase();
135145
Date entryDate = new Date( 0 );
136146

137147
// when & then
138-
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is ( false ) );
148+
assertThat( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder.getRoot(), file, entryname, entryDate ), is ( false ) );
139149
assertThat( this.log.getWarns(), hasItem( new LogMessageMatcher( "names differ only by case" ) ) );
140150
}
141151

142152
@Test
143153
public void shouldExtractWhenEntryInArchiveIsNewerThanFileOnDisk() throws IOException
144154
{
145155
// given
146-
File file = temporaryFolder.newFile( "readme.txt" );
156+
File file = temporaryFolder.newFile();
147157
file.setLastModified( 0 );
148-
String entryname = "readme.txt";
158+
String entryname = file.getName().toUpperCase();
149159
Date entryDate = new Date( System.currentTimeMillis() );
150160

151161
// when & then
152162
this.abstractUnArchiver.setOverwrite( true );
153-
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( true ) );
163+
assertThat( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder.getRoot(), file, entryname, entryDate ), is( true ) );
154164

155165
// when & then
156166
this.abstractUnArchiver.setOverwrite( false );
157-
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( false ) );
167+
assertThat( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder.getRoot(), file, entryname, entryDate ), is( false ) );
158168
}
159169

160170
@Test
161171
public void shouldExtractWhenEntryInArchiveIsNewerThanFileOnDiskAndWarnAboutDifferentCasing() throws IOException
162172
{
163173
// given
164-
File file = temporaryFolder.newFile( "readme.txt" );
174+
File file = temporaryFolder.newFile();
165175
file.setLastModified( 0 );
166-
String entryname = "README.txt";
176+
String entryname = file.getName().toUpperCase();
167177
Date entryDate = new Date( System.currentTimeMillis() );
168178

169179
// when & then
170180
this.abstractUnArchiver.setOverwrite( true );
171-
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( true ) );
181+
assertThat( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder.getRoot(), file, entryname, entryDate ), is( true ) );
172182
this.abstractUnArchiver.setOverwrite( false );
173-
assertThat( this.abstractUnArchiver.shouldExtractEntry( file, entryname, entryDate ), is( false ) );
183+
assertThat( this.abstractUnArchiver.shouldExtractEntry( temporaryFolder.getRoot(), file, entryname, entryDate ), is( false ) );
174184
assertThat( this.log.getWarns(), hasItem( new LogMessageMatcher( "names differ only by case" ) ) );
175185
}
176186

0 commit comments

Comments
 (0)