You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by sl...@apache.org on 2020/06/05 19:44:11 UTC

[maven-shared-utils] 02/03: [MSHARED-297] - BourneShell unconditionally single quotes executable and arguments

This is an automated email from the ASF dual-hosted git repository.

slachiewicz pushed a commit to branch MSHARED-297
in repository https://gitbox.apache.org/repos/asf/maven-shared-utils.git

commit 00a4057b28ce9318ed44df4d48ab25c8ea73ae2d
Author: Rob Oxspring <ro...@imapmail.org>
AuthorDate: Thu May 28 23:29:05 2020 +0100

    [MSHARED-297] - BourneShell unconditionally single quotes executable and arguments
---
 .../maven/shared/utils/cli/shell/BourneShell.java  | 49 +++++++++-------------
 .../apache/maven/shared/utils/cli/shell/Shell.java | 39 ++++++++++++-----
 .../shared/utils/cli/shell/BourneShellTest.java    | 32 ++++++++++----
 3 files changed, 70 insertions(+), 50 deletions(-)

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 02586af..3317788 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
@@ -23,7 +23,6 @@ package org.apache.maven.shared.utils.cli.shell;
 import java.util.ArrayList;
 import java.util.List;
 import org.apache.maven.shared.utils.Os;
-import org.apache.maven.shared.utils.StringUtils;
 
 /**
  * @author Jason van Zyl
@@ -31,19 +30,16 @@ import org.apache.maven.shared.utils.StringUtils;
 public class BourneShell
     extends Shell
 {
-    private static final char DOUBLE_QUOTATION = '"';
-
-    private static final char[] BASH_QUOTING_TRIGGER_CHARS =
-        { ' ', '$', ';', '&', '|', '<', '>', '*', '?', '(', ')', '[', ']', '{', '}', '`', '#' };
 
     /**
      * Create instance of BourneShell.
      */
     public BourneShell()
     {
+        setUnconditionalQuoting( true );
         setShellCommand( "/bin/sh" );
-        setArgumentQuoteDelimiter( DOUBLE_QUOTATION );
-        setExecutableQuoteDelimiter( DOUBLE_QUOTATION );
+        setArgumentQuoteDelimiter( '\'' );
+        setExecutableQuoteDelimiter( '\'' );
         setSingleQuotedArgumentEscaped( true );
         setSingleQuotedExecutableEscaped( false );
         setQuotedExecutableEnabled( true );
@@ -59,7 +55,7 @@ public class BourneShell
             return super.getExecutable();
         }
 
-        return unifyQuotes( super.getExecutable() );
+        return quoteOneItem( super.getExecutable(), true );
     }
 
     /** {@inheritDoc} */
@@ -112,47 +108,40 @@ public class BourneShell
         StringBuilder sb = new StringBuilder();
         sb.append( "cd " );
 
-        sb.append( unifyQuotes( dir ) );
+        sb.append( quoteOneItem( dir, false ) );
         sb.append( " && " );
 
         return sb.toString();
     }
 
-    /** {@inheritDoc} */
-    protected char[] getQuotingTriggerChars()
-    {
-        return BASH_QUOTING_TRIGGER_CHARS;
-    }
-
     /**
      * <p>Unify quotes in a path for the Bourne Shell.</p>
      * <p/>
      * <pre>
-     * BourneShell.unifyQuotes(null)                       = null
-     * BourneShell.unifyQuotes("")                         = (empty)
-     * BourneShell.unifyQuotes("/test/quotedpath'abc")     = /test/quotedpath\'abc
-     * BourneShell.unifyQuotes("/test/quoted path'abc")    = "/test/quoted path'abc"
-     * BourneShell.unifyQuotes("/test/quotedpath\"abc")    = "/test/quotedpath\"abc"
-     * BourneShell.unifyQuotes("/test/quoted path\"abc")   = "/test/quoted path\"abc"
-     * BourneShell.unifyQuotes("/test/quotedpath\"'abc")   = "/test/quotedpath\"'abc"
-     * BourneShell.unifyQuotes("/test/quoted path\"'abc")  = "/test/quoted path\"'abc"
+     * BourneShell.quoteOneItem(null)                       = null
+     * BourneShell.quoteOneItem("")                         = ''
+     * BourneShell.quoteOneItem("/test/quotedpath'abc")     = '/test/quotedpath'"'"'abc'
+     * BourneShell.quoteOneItem("/test/quoted path'abc")    = '/test/quoted pat'"'"'habc'
+     * BourneShell.quoteOneItem("/test/quotedpath\"abc")    = '/test/quotedpath"abc'
+     * BourneShell.quoteOneItem("/test/quoted path\"abc")   = '/test/quoted path"abc'
+     * BourneShell.quoteOneItem("/test/quotedpath\"'abc")   = '/test/quotedpath"'"'"'abc'
+     * BourneShell.quoteOneItem("/test/quoted path\"'abc")  = '/test/quoted path"'"'"'abc'
      * </pre>
      *
      * @param path not null path.
      * @return the path unified correctly for the Bourne shell.
      */
-    private static String unifyQuotes( String path )
+    protected String quoteOneItem( String path, boolean isExecutable )
     {
         if ( path == null )
         {
             return null;
         }
 
-        if ( path.indexOf( ' ' ) == -1 && path.indexOf( '\'' ) != -1 && path.indexOf( '"' ) == -1 )
-        {
-            return StringUtils.escape( path );
-        }
-
-        return StringUtils.quoteAndEscape( path, '\"', BASH_QUOTING_TRIGGER_CHARS );
+        StringBuilder sb = new StringBuilder();
+        sb.append( "'" );
+        sb.append( path.replace( "'", "'\"'\"'" ) );
+        sb.append( "'" );
+        return sb.toString();
     }
 }
diff --git a/src/main/java/org/apache/maven/shared/utils/cli/shell/Shell.java b/src/main/java/org/apache/maven/shared/utils/cli/shell/Shell.java
index bec555e..5ef0d04 100644
--- a/src/main/java/org/apache/maven/shared/utils/cli/shell/Shell.java
+++ b/src/main/java/org/apache/maven/shared/utils/cli/shell/Shell.java
@@ -49,6 +49,8 @@ public class Shell
 
     private boolean quotedArgumentsEnabled = true;
 
+    private boolean unconditionalQuoting = false;
+
     private String executable;
 
     private String workingDir;
@@ -112,6 +114,19 @@ public class Shell
         }
     }
 
+    protected String quoteOneItem( String inputString, boolean isExecutable )
+    {
+        char[] escapeChars = getEscapeChars( isSingleQuotedExecutableEscaped(), isDoubleQuotedExecutableEscaped() );
+        return StringUtils.quoteAndEscape(
+                inputString,
+                isExecutable ? getExecutableQuoteDelimiter() : getArgumentQuoteDelimiter(),
+                escapeChars,
+                getQuotingTriggerChars(),
+                '\\',
+                unconditionalQuoting
+        );
+    }
+
     /**
      * Get the command line for the provided executable and arguments in this shell
      *
@@ -144,15 +159,11 @@ public class Shell
 
             if ( isQuotedExecutableEnabled() )
             {
-                char[] escapeChars =
-                    getEscapeChars( isSingleQuotedExecutableEscaped(), isDoubleQuotedExecutableEscaped() );
-
-                sb.append( StringUtils.quoteAndEscape( getExecutable(), getExecutableQuoteDelimiter(), escapeChars,
-                                                       getQuotingTriggerChars(), '\\', false ) );
+                sb.append( quoteOneItem( executableParameter, true ) );
             }
             else
             {
-                sb.append( getExecutable() );
+                sb.append( executableParameter );
             }
         }
         for ( String argument : argumentsParameter )
@@ -164,10 +175,7 @@ public class Shell
 
             if ( isQuotedArgumentsEnabled() )
             {
-                char[] escapeChars = getEscapeChars( isSingleQuotedArgumentEscaped(), isDoubleQuotedArgumentEscaped() );
-
-                sb.append( StringUtils.quoteAndEscape( argument, getArgumentQuoteDelimiter(), escapeChars,
-                                                       getQuotingTriggerChars(), '\\', false ) );
+                sb.append( quoteOneItem( argument, false ) );
             }
             else
             {
@@ -284,7 +292,7 @@ public class Shell
             commandLine.addAll( getShellArgsList() );
         }
 
-        commandLine.addAll( getCommandLine( getExecutable(), arguments ) );
+        commandLine.addAll( getCommandLine( executable, arguments ) );
 
         return commandLine;
 
@@ -398,4 +406,13 @@ public class Shell
         this.singleQuotedExecutableEscaped = singleQuotedExecutableEscaped;
     }
 
+    public boolean isUnconditionalQuoting()
+    {
+        return unconditionalQuoting;
+    }
+
+    public void setUnconditionalQuoting( boolean unconditionalQuoting )
+    {
+        this.unconditionalQuoting = unconditionalQuoting;
+    }
 }
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 7d5c456..4fbcf84 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
@@ -44,7 +44,7 @@ public class BourneShellTest
 
         String executable = StringUtils.join( sh.getShellCommandLine( new String[]{} ).iterator(), " " );
 
-        assertEquals( "/bin/sh -c cd /usr/local/bin && chmod", executable );
+        assertEquals( "/bin/sh -c cd '/usr/local/bin' && 'chmod'", executable );
     }
 
     public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes()
@@ -56,7 +56,7 @@ public class BourneShellTest
 
         String executable = StringUtils.join( sh.getShellCommandLine( new String[]{} ).iterator(), " " );
 
-        assertEquals( "/bin/sh -c cd \"/usr/local/\'something else\'\" && chmod", executable );
+        assertEquals( "/bin/sh -c cd '/usr/local/'\"'\"'something else'\"'\"'' && 'chmod'", executable );
     }
 
     public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes_BackslashFileSep()
@@ -68,7 +68,7 @@ public class BourneShellTest
 
         String executable = StringUtils.join( sh.getShellCommandLine( new String[]{} ).iterator(), " " );
 
-        assertEquals( "/bin/sh -c cd \"\\usr\\local\\\'something else\'\" && chmod", executable );
+        assertEquals( "/bin/sh -c cd '\\usr\\local\\'\"'\"'something else'\"'\"'' && 'chmod'", executable );
     }
 
     public void testPreserveSingleQuotesOnArgument()
@@ -84,7 +84,7 @@ public class BourneShellTest
 
         String cli = StringUtils.join( shellCommandLine.iterator(), " " );
         System.out.println( cli );
-        assertTrue( cli.endsWith( args[0] ) );
+        assertTrue( cli.endsWith( "'\"some arg with spaces\"'" ) );
     }
 
     public void testAddSingleQuotesOnArgumentWithSpaces()
@@ -100,7 +100,21 @@ public class BourneShellTest
 
         String cli = StringUtils.join( shellCommandLine.iterator(), " " );
         System.out.println( cli );
-        assertTrue( cli.endsWith( "\"" + args[0] + "\"" ) );
+        assertTrue( cli.endsWith("'some arg with spaces'"));
+    }
+
+    public void testAddArgumentWithSingleQuote()
+    {
+        Shell sh = newShell();
+
+        sh.setWorkingDirectory( "/usr/bin" );
+        sh.setExecutable( "chmod" );
+
+        String[] args = { "arg'withquote" };
+
+        List<String> shellCommandLine = sh.getShellCommandLine( args );
+
+        assertEquals("cd '/usr/bin' && 'chmod' 'arg'\"'\"'withquote'", shellCommandLine.get(shellCommandLine.size() - 1));
     }
 
     public void testArgumentsWithSemicolon()
@@ -119,7 +133,7 @@ public class BourneShellTest
 
         String cli = StringUtils.join( shellCommandLine.iterator(), " " );
         System.out.println( cli );
-        assertTrue( cli.endsWith( "\"" + args[0] + "\"" ) );
+        assertTrue( cli.endsWith( "';some&argwithunix$chars'" ) );
 
         Commandline commandline = new Commandline( newShell() );
         commandline.setExecutable( "chmod" );
@@ -132,7 +146,7 @@ public class BourneShellTest
 
         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 +158,7 @@ public class BourneShellTest
 
         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 );
@@ -193,7 +207,7 @@ public class BourneShellTest
 
         assertEquals( "/bin/sh", lines.get( 0 ) );
         assertEquals( "-c", lines.get( 1 ) );
-        assertEquals( "chmod \" \" \"|\" \"&&\" \"||\" \";\" \";;\" \"&\" \"()\" \"<\" \"<<\" \">\" \">>\" \"*\" \"?\" \"[\" \"]\" \"{\" \"}\" \"`\" \"#\"",
+        assertEquals( "'chmod' ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`' '#'",
                       lines.get( 2 ) );
     }