You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by ti...@apache.org on 2019/05/09 15:56:20 UTC
[maven-surefire] 01/02: [SUREFIRE-1617] Surefire fails with bad
message when path contains whitespaces and colon
This is an automated email from the ASF dual-hosted git repository.
tibordigana pushed a commit to branch SUREFIRE-1617
in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
commit 03261d87a1783cd81cf82b69fd812169e15c6353
Author: tibordigana <ti...@apache.org>
AuthorDate: Sun Apr 7 23:06:45 2019 +0200
[SUREFIRE-1617] Surefire fails with bad message when path contains whitespaces and colon
---
.../booterclient/JarManifestForkConfiguration.java | 56 ++++++++-----
.../JarManifestForkConfigurationTest.java | 61 +++++++++++++-
.../org/apache/maven/surefire/its/UmlautDirIT.java | 95 ++++++++++++++++++++--
.../maven/surefire/its/fixture/MavenLauncher.java | 7 +-
4 files changed, 187 insertions(+), 32 deletions(-)
diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/JarManifestForkConfiguration.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/JarManifestForkConfiguration.java
index 62fa4c1..1a3dd4f 100644
--- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/JarManifestForkConfiguration.java
+++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/JarManifestForkConfiguration.java
@@ -31,8 +31,9 @@ import javax.annotation.Nullable;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.Charset;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Iterator;
@@ -43,8 +44,11 @@ import java.util.jar.JarEntry;
import java.util.jar.JarOutputStream;
import java.util.jar.Manifest;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static java.nio.file.Files.isDirectory;
import static org.apache.maven.plugin.surefire.SurefireHelper.escapeToPlatformPath;
+import static org.apache.maven.plugin.surefire.booterclient.JarManifestForkConfiguration.ClasspathElementUri.absolute;
+import static org.apache.maven.plugin.surefire.booterclient.JarManifestForkConfiguration.ClasspathElementUri.relative;
import static org.apache.maven.surefire.util.internal.StringUtils.NL;
/**
@@ -168,14 +172,11 @@ public final class JarManifestForkConfiguration
@Nonnull Path classPathElement,
@Nonnull File dumpLogDirectory,
boolean dumpError )
- throws IOException
{
try
{
- String relativeUriPath = relativize( parent, classPathElement )
- .replace( '\\', '/' );
-
- return new ClasspathElementUri( new URI( null, relativeUriPath, null ) );
+ String relativePath = relativize( parent, classPathElement );
+ return relative( escapeUri( relativePath, UTF_8 ) );
}
catch ( IllegalArgumentException e )
{
@@ -190,15 +191,7 @@ public final class JarManifestForkConfiguration
.dumpStreamText( error, dumpLogDirectory );
}
- return new ClasspathElementUri( toAbsoluteUri( classPathElement ) );
- }
- catch ( URISyntaxException e )
- {
- // This is really unexpected, so fail
- throw new IOException( "Could not create a relative path "
- + classPathElement
- + " against "
- + parent, e );
+ return absolute( toAbsoluteUri( classPathElement ) );
}
}
@@ -207,16 +200,37 @@ public final class JarManifestForkConfiguration
final String uri;
final boolean absolute;
- ClasspathElementUri( String uri )
+ private ClasspathElementUri( String uri, boolean absolute )
{
this.uri = uri;
- absolute = true;
+ this.absolute = absolute;
}
- ClasspathElementUri( URI uri )
+ static ClasspathElementUri absolute( String uri )
+ {
+ return new ClasspathElementUri( uri, true );
+ }
+
+ static ClasspathElementUri relative( String uri )
+ {
+ return new ClasspathElementUri( uri, false );
+ }
+ }
+
+ static String escapeUri( String input, Charset encoding )
+ {
+ try
+ {
+ String uriFormEncoded = URLEncoder.encode( input, encoding.name() );
+
+ String uriPathEncoded = uriFormEncoded.replaceAll( "\\+", "%20" );
+ uriPathEncoded = uriPathEncoded.replaceAll( "%2F|%5C", "/" );
+
+ return uriPathEncoded;
+ }
+ catch ( UnsupportedEncodingException e )
{
- this.uri = uri.toASCIIString();
- absolute = false;
+ throw new IllegalStateException( "avoided by using Charset" );
}
}
}
diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/JarManifestForkConfigurationTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/JarManifestForkConfigurationTest.java
index 5e71238..08b3030 100644
--- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/JarManifestForkConfigurationTest.java
+++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/JarManifestForkConfigurationTest.java
@@ -22,14 +22,18 @@ package org.apache.maven.plugin.surefire.booterclient;
import java.io.File;
import java.net.URI;
import java.net.URISyntaxException;
+import java.net.URLDecoder;
+import java.nio.charset.Charset;
import java.nio.file.Path;
import org.apache.maven.plugin.surefire.booterclient.JarManifestForkConfiguration.ClasspathElementUri;
import org.apache.maven.plugin.surefire.booterclient.output.InPluginProcessDumpSingleton;
+import static java.nio.charset.StandardCharsets.UTF_8;
import static org.apache.maven.plugin.surefire.booterclient.JarManifestForkConfiguration.relativize;
import static org.apache.maven.plugin.surefire.booterclient.JarManifestForkConfiguration.toAbsoluteUri;
import static org.apache.maven.plugin.surefire.booterclient.JarManifestForkConfiguration.toClasspathElementUri;
+import static org.apache.maven.plugin.surefire.booterclient.JarManifestForkConfiguration.escapeUri;
import static org.fest.assertions.Assertions.assertThat;
import org.junit.AfterClass;
@@ -39,7 +43,9 @@ import org.junit.runner.RunWith;
import static org.fest.util.Files.delete;
import static org.fest.util.Files.newTemporaryFolder;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.same;
import static org.powermock.api.mockito.PowerMockito.mock;
import static org.powermock.api.mockito.PowerMockito.mockStatic;
@@ -88,6 +94,8 @@ public class JarManifestForkConfigurationTest
.thenReturn( "../../../.m2/repository/grp/art/1.0/art-1.0.jar" );
when( toClasspathElementUri( same( parent ), same( classPathElement ), same( dumpDirectory ), anyBoolean() ) )
.thenCallRealMethod();
+ when( escapeUri( anyString(), any( Charset.class ) ) )
+ .thenCallRealMethod();
assertThat( toClasspathElementUri( parent, classPathElement, dumpDirectory, true ).uri )
.isEqualTo( "../../../.m2/repository/grp/art/1.0/art-1.0.jar" );
}
@@ -105,6 +113,8 @@ public class JarManifestForkConfigurationTest
.thenReturn( "../../../../../the Maven repo/grp/art/1.0/art-1.0.jar" );
when( toClasspathElementUri( same( parent ), same( classPathElement ), same( dumpDirectory ), anyBoolean() ) )
.thenCallRealMethod();
+ when( escapeUri( anyString(), any( Charset.class ) ) )
+ .thenCallRealMethod();
assertThat( toClasspathElementUri( parent, classPathElement, dumpDirectory, true ).uri )
.isEqualTo( "../../../../../the%20Maven%20repo/grp/art/1.0/art-1.0.jar" );
}
@@ -122,6 +132,8 @@ public class JarManifestForkConfigurationTest
.thenReturn( "..\\..\\..\\Users\\me\\.m2\\repository\\grp\\art\\1.0\\art-1.0.jar" );
when( toClasspathElementUri( same( parent ), same( classPathElement ), same( dumpDirectory ), anyBoolean() ) )
.thenCallRealMethod();
+ when( escapeUri( anyString(), any( Charset.class ) ) )
+ .thenCallRealMethod();
assertThat( toClasspathElementUri( parent, classPathElement, dumpDirectory, true ).uri )
.isEqualTo( "../../../Users/me/.m2/repository/grp/art/1.0/art-1.0.jar" );
}
@@ -134,11 +146,14 @@ public class JarManifestForkConfigurationTest
Path parent = mock( Path.class );
when( parent.toString() ).thenReturn( "C:\\Windows\\Temp\\surefire" );
Path classPathElement = mock( Path.class );
- when( classPathElement.toString() ).thenReturn( "C:\\Test User\\me\\.m2\\repository\\grp\\art\\1.0\\art-1.0.jar" );
+ when( classPathElement.toString() )
+ .thenReturn("C:\\Test User\\me\\.m2\\repository\\grp\\art\\1.0\\art-1.0.jar");
when( relativize( parent, classPathElement ) )
.thenReturn( "..\\..\\..\\Test User\\me\\.m2\\repository\\grp\\art\\1.0\\art-1.0.jar" );
when( toClasspathElementUri( same( parent ), same( classPathElement ), same( dumpDirectory ), anyBoolean() ) )
.thenCallRealMethod();
+ when( escapeUri( anyString(), any( Charset.class ) ) )
+ .thenCallRealMethod();
assertThat( toClasspathElementUri( parent, classPathElement, dumpDirectory, true ).uri )
.isEqualTo( "../../../Test%20User/me/.m2/repository/grp/art/1.0/art-1.0.jar" );
}
@@ -168,6 +183,8 @@ public class JarManifestForkConfigurationTest
.thenThrow( new IllegalArgumentException() );
when( toClasspathElementUri( same( parent ), same( classPathElement ), same( dumpDirectory ), anyBoolean() ) )
.thenCallRealMethod();
+ when( escapeUri( anyString(), any( Charset.class ) ) )
+ .thenCallRealMethod();
when( toAbsoluteUri( same( classPathElement ) ) )
.thenCallRealMethod();
assertThat( toClasspathElementUri( parent, classPathElement, dumpDirectory, true ).uri )
@@ -175,6 +192,46 @@ public class JarManifestForkConfigurationTest
}
@Test
+ public void shouldEscapeUri()
+ throws Exception
+ {
+ assertThat( escapeUri( "a", UTF_8 ) ).isEqualTo( "a" );
+ assertThat( escapeUri( " ", UTF_8 ) ).isEqualTo( "%20" );
+ assertThat( escapeUri( "%", UTF_8 ) ).isEqualTo( "%25" );
+ assertThat( escapeUri( "+", UTF_8 ) ).isEqualTo( "%2B" );
+ assertThat( escapeUri( ",", UTF_8 ) ).isEqualTo( "%2C" );
+ assertThat( escapeUri( "/", UTF_8 ) ).isEqualTo( "/" );
+ assertThat( escapeUri( "7", UTF_8 ) ).isEqualTo( "7" );
+ assertThat( escapeUri( ":", UTF_8 ) ).isEqualTo( "%3A" );
+ assertThat( escapeUri( "@", UTF_8 ) ).isEqualTo( "%40" );
+ assertThat( escapeUri( "A", UTF_8 ) ).isEqualTo( "A" );
+ assertThat( escapeUri( "[", UTF_8 ) ).isEqualTo( "%5B" );
+ assertThat( escapeUri( "\\", UTF_8 ) ).isEqualTo( "/" );
+ assertThat( escapeUri( "]", UTF_8 ) ).isEqualTo( "%5D" );
+ assertThat( escapeUri( "`", UTF_8 ) ).isEqualTo( "%60" );
+ assertThat( escapeUri( "a", UTF_8 ) ).isEqualTo( "a" );
+ assertThat( escapeUri( "{", UTF_8 ) ).isEqualTo( "%7B" );
+ assertThat( escapeUri( "" + (char) 0xFF, UTF_8 ) ).isEqualTo( "%C3%BF" );
+
+ assertThat( escapeUri( "..\\..\\..\\Test : User\\me\\.m2\\repository\\grp\\art\\1.0\\art-1.0.jar",
+ UTF_8 ) )
+ .isEqualTo( "../../../Test%20%3A%20User/me/.m2/repository/grp/art/1.0/art-1.0.jar" );
+
+ assertThat( escapeUri( "..\\..\\..\\Test : User\\me\\.m2\\repository\\grp\\art\\1.0\\art-1.0.jar",
+ Charset.defaultCharset() ) )
+ .isEqualTo( "../../../Test%20%3A%20User/me/.m2/repository/grp/art/1.0/art-1.0.jar" );
+
+ assertThat( escapeUri( "../../surefire-its/target/junit-pathWithÜmlaut_1/target/surefire", UTF_8 ) )
+ .isEqualTo( "../../surefire-its/target/junit-pathWith%C3%9Cmlaut_1/target/surefire" );
+
+ String source = "../../surefire-its/target/junit-pathWithÜmlaut_1/target/surefire";
+ String encoded = escapeUri( "../../surefire-its/target/junit-pathWithÜmlaut_1/target/surefire", UTF_8 );
+ String decoded = URLDecoder.decode( encoded, UTF_8.name() );
+ assertThat( decoded )
+ .isEqualTo( source );
+ }
+
+ @Test
public void shouldRelativizeOnRealPlatform()
{
Path parentDir = new File( TMP, "test-parent-1" )
@@ -218,6 +275,6 @@ public class JarManifestForkConfigurationTest
ClasspathElementUri testDirUriPath = toClasspathElementUri( parentDir, testDir, dumpDirectory, true );
assertThat( testDirUriPath.uri )
- .isEqualTo( "../@3%20test%20with%20white%20spaces" );
+ .isEqualTo( "../%403%20test%20with%20white%20spaces" );
}
}
diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/UmlautDirIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/UmlautDirIT.java
index 7b008bc..3690492 100644
--- a/surefire-its/src/test/java/org/apache/maven/surefire/its/UmlautDirIT.java
+++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/UmlautDirIT.java
@@ -19,28 +19,70 @@ package org.apache.maven.surefire.its;
* under the License.
*/
-import org.apache.maven.it.VerificationException;
import org.apache.maven.surefire.its.fixture.MavenLauncher;
import org.apache.maven.surefire.its.fixture.SurefireJUnit4IntegrationTestCase;
import org.apache.maven.surefire.its.fixture.SurefireLauncher;
+import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
import java.io.File;
import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import static java.nio.file.Files.copy;
+import static java.nio.file.Files.createDirectories;
+import static java.nio.file.Files.exists;
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+import static java.util.Objects.requireNonNull;
+import static org.apache.commons.lang3.SystemUtils.IS_OS_LINUX;
+import static org.junit.Assume.assumeTrue;
/**
* Test a directory with an umlaut
*
* @author <a href="mailto:dfabulich@apache.org">Dan Fabulich</a>
*/
-public class UmlautDirIT
- extends SurefireJUnit4IntegrationTestCase
+public class UmlautDirIT extends SurefireJUnit4IntegrationTestCase
{
+ private String localRepo;
+
+ @Before
+ public void backupLocalRepo()
+ {
+ localRepo = System.getProperty( "maven.repo.local" );
+ }
+
+ @After
+ public void restoreLocalRepo()
+ {
+ if ( localRepo == null )
+ {
+ System.clearProperty( "maven.repo.local" );
+ }
+ else
+ {
+ System.setProperty( "maven.repo.local", localRepo );
+ }
+ }
+
+ @Test
+ public void surefire1617()
+ throws Exception
+ {
+ assumeTrue( IS_OS_LINUX );
+ unpackWithNewLocalRepo()
+ .executeTest()
+ .verifyErrorFreeLog()
+ .assertTestSuiteResults( 1, 0, 0, 0 );
+ }
+
@Test
public void testUmlaut()
throws Exception
{
- specialUnpack( "1" )
+ unpackToGermanUmplautDirectory( "1" )
.executeTest()
.verifyErrorFreeLog()
.assertTestSuiteResults( 1, 0, 0, 0 );
@@ -50,20 +92,57 @@ public class UmlautDirIT
public void testUmlautIsolatedClassLoader()
throws Exception
{
- specialUnpack( "2" )
+ unpackToGermanUmplautDirectory( "2" )
.useSystemClassLoader( false )
.executeTest()
.assertTestSuiteResults( 1, 0, 0, 0 );
}
- SurefireLauncher specialUnpack( String postfix )
- throws IOException
+ private SurefireLauncher unpackToGermanUmplautDirectory( String postfix ) throws IOException
{
SurefireLauncher unpack = unpack( "junit-pathWithUmlaut" );
MavenLauncher maven = unpack.maven();
- File dest = new File( maven.getUnpackedAt().getParentFile().getPath(), "/junit-pathWith\u00DCmlaut_" + postfix );
+ File dest = new File( maven.getUnpackedAt().getParentFile().getPath(),
+ "/junit-pathWith\u00DCmlaut_" + postfix );
maven.moveUnpackTo( dest );
return unpack;
}
+
+ private SurefireLauncher unpackWithNewLocalRepo() throws IOException
+ {
+ String newLocalRepo =
+ Paths.get( System.getProperty( "user.dir" ), "target", "local repo for : SUREFIRE-1617" ).toString();
+ String defaultLocalRepo = new MavenLauncher( getClass(), "", null ).getLocalRepository();
+
+ copyFolder( Paths.get( defaultLocalRepo, "org", "apache", "maven", "surefire" ),
+ Paths.get( newLocalRepo, "org", "apache", "maven", "surefire" ) );
+
+ copyFolder( Paths.get( defaultLocalRepo, "org", "apache", "maven", "plugins", "maven-surefire-plugin" ),
+ Paths.get( newLocalRepo, "org", "apache", "maven", "plugins", "maven-surefire-plugin" ) );
+
+ System.setProperty( "maven.repo.local", newLocalRepo );
+ return unpack( "junit-pathWithUmlaut" );
+ }
+
+ private static void copyFolder( Path src, Path dest ) throws IOException
+ {
+ if ( !exists( dest ) )
+ {
+ createDirectories( dest );
+ }
+
+ for ( File from : requireNonNull( src.toFile().listFiles() ) )
+ {
+ Path to = dest.resolve( from.getName() );
+ if ( from.isDirectory() )
+ {
+ copyFolder( from.toPath(), to );
+ }
+ else if ( from.isFile() )
+ {
+ copy( from.toPath(), to, REPLACE_EXISTING );
+ }
+ }
+ }
}
diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/MavenLauncher.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/MavenLauncher.java
index c36da47..586ce8c 100755
--- a/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/MavenLauncher.java
+++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/MavenLauncher.java
@@ -77,7 +77,7 @@ public final class MavenLauncher
resetCliOptions();
}
- MavenLauncher( Class testClass, String resourceName, String suffix )
+ public MavenLauncher( Class testClass, String resourceName, String suffix )
{
this( testClass, resourceName, suffix, null );
}
@@ -376,6 +376,11 @@ public final class MavenLauncher
getVerifier().setForkJvm( forkJvm );
}
+ public String getLocalRepository()
+ {
+ return getVerifier().getLocalRepository();
+ }
+
private Verifier getVerifier()
{
if ( verifier == null )