From 3b069da6f7e1a656c16e7c92a27f0eb50d27e2ce Mon Sep 17 00:00:00 2001 From: Rob Oxspring Date: Wed, 27 May 2020 21:49:20 +0100 Subject: [PATCH 1/3] [MSHARED-431] Escape arguments including hash-signs --- .../org/apache/maven/shared/utils/cli/shell/BourneShell.java | 2 +- .../apache/maven/shared/utils/cli/shell/BourneShellTest.java | 3 ++- 2 files changed, 3 insertions(+), 2 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 1793cbb7..02586afd 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 @@ -34,7 +34,7 @@ public class BourneShell private static final char DOUBLE_QUOTATION = '"'; private static final char[] BASH_QUOTING_TRIGGER_CHARS = - { ' ', '$', ';', '&', '|', '<', '>', '*', '?', '(', ')', '[', ']', '{', '}', '`' }; + { ' ', '$', ';', '&', '|', '<', '>', '*', '?', '(', ')', '[', ']', '{', '}', '`', '#' }; /** * Create instance of BourneShell. 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 c83287c2..7d5c4565 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 @@ -186,13 +186,14 @@ public void testBourneShellQuotingCharacters() commandline.createArg().setValue( "{" ); commandline.createArg().setValue( "}" ); commandline.createArg().setValue( "`" ); + commandline.createArg().setValue( "#" ); List lines = commandline.getShell().getShellCommandLine( commandline.getArguments() ); System.out.println( lines ); assertEquals( "/bin/sh", lines.get( 0 ) ); assertEquals( "-c", lines.get( 1 ) ); - assertEquals( "chmod \" \" \"|\" \"&&\" \"||\" \";\" \";;\" \"&\" \"()\" \"<\" \"<<\" \">\" \">>\" \"*\" \"?\" \"[\" \"]\" \"{\" \"}\" \"`\"", + assertEquals( "chmod \" \" \"|\" \"&&\" \"||\" \";\" \";;\" \"&\" \"()\" \"<\" \"<<\" \">\" \">>\" \"*\" \"?\" \"[\" \"]\" \"{\" \"}\" \"`\" \"#\"", lines.get( 2 ) ); } From 2735facbbbc2e13546328cb02dbb401b3776eea3 Mon Sep 17 00:00:00 2001 From: Rob Oxspring Date: Thu, 28 May 2020 23:29:05 +0100 Subject: [PATCH 2/3] [MSHARED-297] - BourneShell unconditionally single quotes executable and arguments --- .../shared/utils/cli/shell/BourneShell.java | 49 +++++++------------ .../maven/shared/utils/cli/shell/Shell.java | 39 ++++++++++----- .../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 02586afd..33177889 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 @@ 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 @@ 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 String getExecutable() return super.getExecutable(); } - return unifyQuotes( super.getExecutable() ); + return quoteOneItem( super.getExecutable(), true ); } /** {@inheritDoc} */ @@ -112,47 +108,40 @@ protected String getExecutionPreamble() 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; - } - /** *

Unify quotes in a path for the Bourne Shell.

*

* 

-     * 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'
      *
* * @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 bec555e0..5ef0d04b 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 @@ String[] getShellArgs() } } + 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 @@ List getRawCommandLine( String executableParameter, String... argumentsP 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 @@ List getRawCommandLine( String executableParameter, String... argumentsP 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 List getShellCommandLine( String... arguments ) commandLine.addAll( getShellArgsList() ); } - commandLine.addAll( getCommandLine( getExecutable(), arguments ) ); + commandLine.addAll( getCommandLine( executable, arguments ) ); return commandLine; @@ -398,4 +406,13 @@ void setSingleQuotedExecutableEscaped( boolean singleQuotedExecutableEscaped ) 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 7d5c4565..4fbcf841 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 void testQuoteWorkingDirectoryAndExecutable() 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 void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes() 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 void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes_Backsl 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 void testPreserveSingleQuotesOnArgument() 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 void testAddSingleQuotesOnArgumentWithSpaces() 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 shellCommandLine = sh.getShellCommandLine( args ); + + assertEquals("cd '/usr/bin' && 'chmod' 'arg'\"'\"'withquote'", shellCommandLine.get(shellCommandLine.size() - 1)); } public void testArgumentsWithSemicolon() @@ -119,7 +133,7 @@ public void testArgumentsWithSemicolon() 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 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 +158,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 ); @@ -193,7 +207,7 @@ public void testBourneShellQuotingCharacters() assertEquals( "/bin/sh", lines.get( 0 ) ); assertEquals( "-c", lines.get( 1 ) ); - assertEquals( "chmod \" \" \"|\" \"&&\" \"||\" \";\" \";;\" \"&\" \"()\" \"<\" \"<<\" \">\" \">>\" \"*\" \"?\" \"[\" \"]\" \"{\" \"}\" \"`\" \"#\"", + assertEquals( "'chmod' ' ' '|' '&&' '||' ';' ';;' '&' '()' '<' '<<' '>' '>>' '*' '?' '[' ']' '{' '}' '`' '#'", lines.get( 2 ) ); } From bb6b6a4bf44cc09f120068bd29fed3e9ab10eb6f Mon Sep 17 00:00:00 2001 From: Rob Oxspring Date: Thu, 28 May 2020 23:50:09 +0100 Subject: [PATCH 3/3] [MSHARED-297] - Minor code cleanup --- .../shared/utils/cli/shell/BourneShell.java | 13 ++--------- .../maven/shared/utils/cli/shell/Shell.java | 8 +++---- .../utils/cli/CommandLineUtilsTest.java | 5 ++--- .../utils/cli/shell/BourneShellTest.java | 22 ++++++------------- 4 files changed, 15 insertions(+), 33 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 33177889..e3af6651 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 @@ -105,13 +105,8 @@ protected String getExecutionPreamble() } String dir = getWorkingDirectoryAsString(); - StringBuilder sb = new StringBuilder(); - sb.append( "cd " ); - sb.append( quoteOneItem( dir, false ) ); - sb.append( " && " ); - - return sb.toString(); + return "cd " + quoteOneItem( dir, false ) + " && "; } /** @@ -138,10 +133,6 @@ protected String quoteOneItem( String path, boolean isExecutable ) return null; } - StringBuilder sb = new StringBuilder(); - sb.append( "'" ); - sb.append( path.replace( "'", "'\"'\"'" ) ); - sb.append( "'" ); - return sb.toString(); + return "'" + path.replace( "'", "'\"'\"'" ) + "'"; } } 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 5ef0d04b..02681086 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 @@ -104,13 +104,13 @@ void setShellArgs( String[] shellArgs ) */ String[] getShellArgs() { - if ( ( shellArgs == null ) || shellArgs.isEmpty() ) + if ( shellArgs.isEmpty() ) { return null; } else { - return shellArgs.toArray( new String[shellArgs.size()] ); + return shellArgs.toArray( new String[0] ); } } @@ -146,7 +146,7 @@ List getCommandLine( String executableParameter, String... argumentsPara */ List getRawCommandLine( String executableParameter, String... argumentsParameter ) { - List commandLine = new ArrayList(); + List commandLine = new ArrayList<>(); StringBuilder sb = new StringBuilder(); if ( executableParameter != null ) @@ -280,7 +280,7 @@ char getExecutableQuoteDelimiter() public List getShellCommandLine( String... arguments ) { - List commandLine = new ArrayList(); + List commandLine = new ArrayList<>(); if ( getShellCommand() != null ) { 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 50d9336c..079d0d16 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 @@ -148,8 +148,8 @@ public void givenAnEscapedDoubleQuoteMarkInArgument_whenTranslatingToCmdLineArgs public void givenAnEscapedSingleQuoteMarkInArgument_whenTranslatingToCmdLineArgs_thenTheQuotationMarkRemainsEscaped() throws Exception { - final String command = "echo \"let\\\'s go\""; - final String[] expected = new String[] { "echo", "let\\\'s go" }; + final String command = "echo \"let\\'s go\""; + final String[] expected = new String[] { "echo", "let\\'s go"}; assertCmdLineArgs( expected, command ); } @@ -168,5 +168,4 @@ private void assertCmdLineArgs( final String[] expected, final String cmdLine ) assertEquals( expected.length, actual.length ); assertEquals( Arrays.asList( expected ), Arrays.asList( actual ) ); } - } 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 4fbcf841..199f36e4 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 @@ -42,7 +42,7 @@ public void testQuoteWorkingDirectoryAndExecutable() sh.setWorkingDirectory( "/usr/local/bin" ); sh.setExecutable( "chmod" ); - String executable = StringUtils.join( sh.getShellCommandLine( new String[]{} ).iterator(), " " ); + String executable = StringUtils.join( sh.getShellCommandLine().iterator(), " " ); assertEquals( "/bin/sh -c cd '/usr/local/bin' && 'chmod'", executable ); } @@ -54,7 +54,7 @@ public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes() sh.setWorkingDirectory( "/usr/local/'something else'" ); sh.setExecutable( "chmod" ); - String executable = StringUtils.join( sh.getShellCommandLine( new String[]{} ).iterator(), " " ); + String executable = StringUtils.join( sh.getShellCommandLine().iterator(), " " ); assertEquals( "/bin/sh -c cd '/usr/local/'\"'\"'something else'\"'\"'' && 'chmod'", executable ); } @@ -66,7 +66,7 @@ public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes_Backsl sh.setWorkingDirectory( "\\usr\\local\\'something else'" ); sh.setExecutable( "chmod" ); - String executable = StringUtils.join( sh.getShellCommandLine( new String[]{} ).iterator(), " " ); + String executable = StringUtils.join( sh.getShellCommandLine().iterator(), " " ); assertEquals( "/bin/sh -c cd '\\usr\\local\\'\"'\"'something else'\"'\"'' && 'chmod'", executable ); } @@ -78,9 +78,7 @@ public void testPreserveSingleQuotesOnArgument() sh.setWorkingDirectory( "/usr/bin" ); sh.setExecutable( "chmod" ); - final String[] args = { "\"some arg with spaces\"" }; - - List shellCommandLine = sh.getShellCommandLine( args ); + List shellCommandLine = sh.getShellCommandLine("\"some arg with spaces\""); String cli = StringUtils.join( shellCommandLine.iterator(), " " ); System.out.println( cli ); @@ -94,9 +92,7 @@ public void testAddSingleQuotesOnArgumentWithSpaces() sh.setWorkingDirectory( "/usr/bin" ); sh.setExecutable( "chmod" ); - String[] args = { "some arg with spaces" }; - - List shellCommandLine = sh.getShellCommandLine( args ); + List shellCommandLine = sh.getShellCommandLine("some arg with spaces"); String cli = StringUtils.join( shellCommandLine.iterator(), " " ); System.out.println( cli ); @@ -110,9 +106,7 @@ public void testAddArgumentWithSingleQuote() sh.setWorkingDirectory( "/usr/bin" ); sh.setExecutable( "chmod" ); - String[] args = { "arg'withquote" }; - - List shellCommandLine = sh.getShellCommandLine( args ); + List shellCommandLine = sh.getShellCommandLine("arg'withquote"); assertEquals("cd '/usr/bin' && 'chmod' 'arg'\"'\"'withquote'", shellCommandLine.get(shellCommandLine.size() - 1)); } @@ -127,9 +121,7 @@ public void testArgumentsWithSemicolon() sh.setWorkingDirectory( "/usr/bin" ); sh.setExecutable( "chmod" ); - String[] args = { ";some&argwithunix$chars" }; - - List shellCommandLine = sh.getShellCommandLine( args ); + List shellCommandLine = sh.getShellCommandLine(";some&argwithunix$chars"); String cli = StringUtils.join( shellCommandLine.iterator(), " " ); System.out.println( cli );