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 ed82d7f6cea63a6aa3aef057f5e784fc22854565
Author: tibordigana <[email protected]>
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 +++++++++++++++++++++-
 2 files changed, 94 insertions(+), 23 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" );
     }
 }

Reply via email to