You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2018/07/26 16:53:30 UTC

[GitHub] khmarbaise closed pull request #2: Bug/MSHARED-416

khmarbaise closed pull request #2: Bug/MSHARED-416
URL: https://github.com/apache/maven-shared-utils/pull/2
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/src/main/java/org/apache/maven/shared/utils/StringUtils.java b/src/main/java/org/apache/maven/shared/utils/StringUtils.java
index 58bf8be..6b9a55a 100644
--- a/src/main/java/org/apache/maven/shared/utils/StringUtils.java
+++ b/src/main/java/org/apache/maven/shared/utils/StringUtils.java
@@ -2290,9 +2290,9 @@ public static String quoteAndEscape( @Nullable String source, char quoteChar,
     /**
      * @param source The source.
      * @param quoteChar The quote character.
-     * @param escapedChars The escaped characters.
+     * @param escapedChars set of characters to escape.
      * @param quotingTriggers The quoting trigger.
-     * @param escapeChar The escape character.
+     * @param escapeChar prefix for escaping a character.
      * @param force true/false.
      * @return the String quoted and escaped
      */
@@ -2343,8 +2343,8 @@ else if ( !escaped.equals( source ) )
 
     /**
      * @param source The source.
-     * @param escapedChars escape characters.
-     * @param escapeChar escape character.
+     * @param escapedChars set of characters to escape.
+     * @param escapeChar prefix for escaping a character.
      * @return the String escaped
      */
     public static String escape( @Nullable String source, @Nonnull final char[] escapedChars, char escapeChar )
diff --git a/src/main/java/org/apache/maven/shared/utils/cli/shell/BourneShell.java b/src/main/java/org/apache/maven/shared/utils/cli/shell/BourneShell.java
index 8116707..1793cbb 100644
--- a/src/main/java/org/apache/maven/shared/utils/cli/shell/BourneShell.java
+++ b/src/main/java/org/apache/maven/shared/utils/cli/shell/BourneShell.java
@@ -31,17 +31,19 @@
 public class BourneShell
     extends Shell
 {
+    private static final char DOUBLE_QUOTATION = '"';
+
     private static final char[] BASH_QUOTING_TRIGGER_CHARS =
         { ' ', '$', ';', '&', '|', '<', '>', '*', '?', '(', ')', '[', ']', '{', '}', '`' };
 
     /**
-     * Create instance of BournShell.
+     * Create instance of BourneShell.
      */
     public BourneShell()
     {
         setShellCommand( "/bin/sh" );
-        setArgumentQuoteDelimiter( '\'' );
-        setExecutableQuoteDelimiter( '\"' );
+        setArgumentQuoteDelimiter( DOUBLE_QUOTATION );
+        setExecutableQuoteDelimiter( DOUBLE_QUOTATION );
         setSingleQuotedArgumentEscaped( true );
         setSingleQuotedExecutableEscaped( false );
         setQuotedExecutableEnabled( true );
diff --git a/src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java b/src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
index 0f8b799..4ed35ed 100644
--- a/src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
+++ b/src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
@@ -19,22 +19,27 @@
  * under the License.
  */
 
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Properties;
+
 import org.apache.maven.shared.utils.Os;
 
-import junit.framework.TestCase;
+import org.junit.Test;
 
 public class CommandLineUtilsTest
-    extends TestCase
 {
 
     /**
      * Tests that case-insensitive environment variables are normalized to upper case.
      */
+    @Test
     public void testGetSystemEnvVarsCaseInsensitive()
     {
         Properties vars = CommandLineUtils.getSystemEnvVars( false );
@@ -45,6 +50,7 @@ public void testGetSystemEnvVarsCaseInsensitive()
         }
     }
 
+    @Test
     public void testEnsureCaseSensitivity()
         throws Exception
     {
@@ -57,6 +63,7 @@ public void testEnsureCaseSensitivity()
     /**
      * Tests that environment variables on Windows are normalized to upper case. Does nothing on Unix platforms.
      */
+    @Test
     public void testGetSystemEnvVarsWindows()
         throws Exception
     {
@@ -75,6 +82,7 @@ public void testGetSystemEnvVarsWindows()
     /**
      * Tests the splitting of a command line into distinct arguments.
      */
+    @Test
     public void testTranslateCommandline()
         throws Exception
     {
@@ -91,6 +99,26 @@ public void testTranslateCommandline()
         assertCmdLineArgs( new String[] { "foo", " ' ", "bar" }, "foo \" ' \" bar" );
     }
 
+    @Test
+    public void givenASingleQuoteMarkInArgument_whenExecutingCode_thenExitCode0Returned() throws Exception {
+        final Process p = exec("echo \"let's go\"");
+
+        assertEquals(0, p.exitValue());
+    }
+
+    @Test
+    public void givenADoubleQuoteMarkInArgument_whenExecutingCode_thenExitCode0Returned() throws Exception {
+        final Process p = exec("echo \"let\"s go\"");
+
+        assertEquals(0, p.exitValue());
+    }
+
+    private Process exec(String cmd) throws CommandLineException, InterruptedException {
+        Process p = new Commandline(cmd).execute();
+        Thread.sleep(1000);
+        return p;
+    }
+
     private void assertCmdLineArgs( String[] expected, String cmdLine )
         throws Exception
     {
diff --git a/src/test/java/org/apache/maven/shared/utils/cli/shell/BourneShellTest.java b/src/test/java/org/apache/maven/shared/utils/cli/shell/BourneShellTest.java
index cfca54b..c83287c 100644
--- a/src/test/java/org/apache/maven/shared/utils/cli/shell/BourneShellTest.java
+++ b/src/test/java/org/apache/maven/shared/utils/cli/shell/BourneShellTest.java
@@ -78,7 +78,7 @@ public void testPreserveSingleQuotesOnArgument()
         sh.setWorkingDirectory( "/usr/bin" );
         sh.setExecutable( "chmod" );
 
-        String[] args = { "\'some arg with spaces\'" };
+        final String[] args = { "\"some arg with spaces\"" };
 
         List<String> shellCommandLine = sh.getShellCommandLine( args );
 
@@ -100,10 +100,10 @@ public void testAddSingleQuotesOnArgumentWithSpaces()
 
         String cli = StringUtils.join( shellCommandLine.iterator(), " " );
         System.out.println( cli );
-        assertTrue( cli.endsWith( "\'" + args[0] + "\'" ) );
+        assertTrue( cli.endsWith( "\"" + args[0] + "\"" ) );
     }
 
-    public void testArgumentsWithsemicolon()
+    public void testArgumentsWithSemicolon()
     {
 
         System.out.println( "---- semi colon tests ----" );
@@ -119,7 +119,7 @@ public void testArgumentsWithsemicolon()
 
         String cli = StringUtils.join( shellCommandLine.iterator(), " " );
         System.out.println( cli );
-        assertTrue( cli.endsWith( "\'" + args[0] + "\'" ) );
+        assertTrue( cli.endsWith( "\"" + args[0] + "\"" ) );
 
         Commandline commandline = new Commandline( newShell() );
         commandline.setExecutable( "chmod" );
@@ -132,7 +132,7 @@ public void testArgumentsWithsemicolon()
 
         assertEquals( "/bin/sh", lines.get( 0 ) );
         assertEquals( "-c", lines.get( 1 ) );
-        assertEquals( "chmod --password ';password'", lines.get( 2 ) );
+        assertEquals( "chmod --password \";password\"", lines.get( 2 ) );
 
         commandline = new Commandline( newShell() );
         commandline.setExecutable( "chmod" );
@@ -144,7 +144,7 @@ public void testArgumentsWithsemicolon()
 
         assertEquals( "/bin/sh", lines.get( 0) );
         assertEquals( "-c", lines.get( 1 ) );
-        assertEquals( "chmod --password ';password'", lines.get( 2 ) );
+        assertEquals( "chmod --password \";password\"", lines.get( 2 ) );
 
         commandline = new Commandline( new CmdShell() );
         commandline.getShell().setQuotedArgumentsEnabled( true );
@@ -192,7 +192,7 @@ public void testBourneShellQuotingCharacters()
 
         assertEquals( "/bin/sh", lines.get( 0 ) );
         assertEquals( "-c", lines.get( 1 ) );
-        assertEquals( "chmod ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`'",
+        assertEquals( "chmod \" \" \"|\" \"&&\" \"||\" \";\" \";;\" \"&\" \"()\" \"<\" \"<<\" \">\" \">>\" \"*\" \"?\" \"[\" \"]\" \"{\" \"}\" \"`\"",
                       lines.get( 2 ) );
     }
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services