From c0b225b90d1056e29d681258a2008ae8aff2b508 Mon Sep 17 00:00:00 2001 From: Marian Koncek Date: Tue, 5 Apr 2022 13:56:20 +0200 Subject: [PATCH] Unconditionally single quote executable and arguments Upstream: https://github.com/apache/maven-shared-utils/pull/40/commits --- .../shared/utils/cli/shell/BourneShell.java | 48 ++++++++----------- .../maven/shared/utils/cli/shell/Shell.java | 39 ++++++++++----- .../utils/cli/shell/BourneShellTest.java | 35 ++++++++++---- 3 files changed, 72 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 11a447a..f0de631 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,17 +30,15 @@ import org.apache.maven.shared.utils.StringUtils; public class BourneShell extends Shell { - private static final char[] BASH_QUOTING_TRIGGER_CHARS = - { ' ', '$', ';', '&', '|', '<', '>', '*', '?', '(', ')', '[', ']', '{', '}', '`' }; - /** - * Create instance of BournShell. + * Create instance of BourneShell. */ public BourneShell() { + setUnconditionalQuoting( true ); setShellCommand( "/bin/sh" ); setArgumentQuoteDelimiter( '\'' ); - setExecutableQuoteDelimiter( '\"' ); + setExecutableQuoteDelimiter( '\'' ); setSingleQuotedArgumentEscaped( true ); setSingleQuotedExecutableEscaped( false ); setQuotedExecutableEnabled( true ); @@ -57,7 +54,7 @@ public class BourneShell return super.getExecutable(); } - return unifyQuotes( super.getExecutable() ); + return quoteOneItem( super.getExecutable(), true ); } /** {@inheritDoc} */ @@ -110,47 +107,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; - } - /** *

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 6fa2f73..96904cb 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 @@ -50,6 +50,8 @@ public class Shell private boolean quotedArgumentsEnabled = true; + private boolean unconditionalQuoting = false; + private String executable; private String workingDir; @@ -113,6 +115,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 * @@ -145,15 +160,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 ) @@ -165,10 +176,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 { @@ -285,7 +293,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 b5f23d9..f5143c1 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() @@ -78,13 +78,13 @@ public class BourneShellTest sh.setWorkingDirectory( "/usr/bin" ); sh.setExecutable( "chmod" ); - String[] args = { "\'some arg with spaces\'" }; + String[] args = { "\"some arg with spaces\"" }; List shellCommandLine = sh.getShellCommandLine( args ); 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 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 ); @@ -186,13 +200,14 @@ public class BourneShellTest 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 ) ); } -- 2.35.1