Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MSHARED-297] Unconditionally single quote executable and arguments #40

Closed
wants to merge 3 commits into from
Closed

[MSHARED-297] Unconditionally single quote executable and arguments #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b069da
[MSHARED-431] Escape arguments including hash-signs
roxspring May 27, 2020
2735fac
[MSHARED-297] - BourneShell unconditionally single quotes executable …
roxspring May 28, 2020
bb6b6a4
[MSHARED-297] - Minor code cleanup
roxspring May 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 15 additions & 35 deletions src/main/java/org/apache/maven/shared/utils/cli/shell/BourneShell.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,23 @@
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
*/
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 );
Expand All @@ -59,7 +55,7 @@ public String getExecutable()
return super.getExecutable();
}

return unifyQuotes( super.getExecutable() );
return quoteOneItem( super.getExecutable(), true );
}

/** {@inheritDoc} */
Expand Down Expand Up @@ -109,50 +105,34 @@ protected String getExecutionPreamble()
}

String dir = getWorkingDirectoryAsString();
StringBuilder sb = new StringBuilder();
sb.append( "cd " );

sb.append( unifyQuotes( dir ) );
sb.append( " && " );

return sb.toString();
}

/** {@inheritDoc} */
protected char[] getQuotingTriggerChars()
{
return BASH_QUOTING_TRIGGER_CHARS;
return "cd " + quoteOneItem( dir, false ) + " && ";
}

/**
* <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 );
return "'" + path.replace( "'", "'\"'\"'" ) + "'";
}
}
47 changes: 32 additions & 15 deletions src/main/java/org/apache/maven/shared/utils/cli/shell/Shell.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public class Shell

private boolean quotedArgumentsEnabled = true;

private boolean unconditionalQuoting = false;

private String executable;

private String workingDir;
Expand Down Expand Up @@ -102,16 +104,29 @@ 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] );
}
}

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
*
Expand All @@ -131,7 +146,7 @@ List<String> getCommandLine( String executableParameter, String... argumentsPara
*/
List<String> getRawCommandLine( String executableParameter, String... argumentsParameter )
{
List<String> commandLine = new ArrayList<String>();
List<String> commandLine = new ArrayList<>();
StringBuilder sb = new StringBuilder();

if ( executableParameter != null )
Expand All @@ -144,15 +159,11 @@ List<String> 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 )
Expand All @@ -164,10 +175,7 @@ List<String> 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
{
Expand Down Expand Up @@ -272,7 +280,7 @@ char getExecutableQuoteDelimiter()
public List<String> getShellCommandLine( String... arguments )
{

List<String> commandLine = new ArrayList<String>();
List<String> commandLine = new ArrayList<>();

if ( getShellCommand() != null )
{
Expand All @@ -284,7 +292,7 @@ public List<String> getShellCommandLine( String... arguments )
commandLine.addAll( getShellArgsList() );
}

commandLine.addAll( getCommandLine( getExecutable(), arguments ) );
commandLine.addAll( getCommandLine( executable, arguments ) );

return commandLine;

Expand Down Expand Up @@ -398,4 +406,13 @@ void setSingleQuotedExecutableEscaped( boolean singleQuotedExecutableEscaped )
this.singleQuotedExecutableEscaped = singleQuotedExecutableEscaped;
}

public boolean isUnconditionalQuoting()
{
return unconditionalQuoting;
}

public void setUnconditionalQuoting( boolean unconditionalQuoting )
{
this.unconditionalQuoting = unconditionalQuoting;
}
}
5 changes: 2 additions & 3 deletions src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}

Expand All @@ -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 ) );
}

}
49 changes: 28 additions & 21 deletions src/test/java/org/apache/maven/shared/utils/cli/shell/BourneShellTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ 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 );
assertEquals( "/bin/sh -c cd '/usr/local/bin' && 'chmod'", executable );
}

public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes()
Expand All @@ -54,9 +54,9 @@ 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 );
assertEquals( "/bin/sh -c cd '/usr/local/'\"'\"'something else'\"'\"'' && 'chmod'", executable );
}

public void testQuoteWorkingDirectoryAndExecutable_WDPathWithSingleQuotes_BackslashFileSep()
Expand All @@ -66,9 +66,9 @@ 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 );
assertEquals( "/bin/sh -c cd '\\usr\\local\\'\"'\"'something else'\"'\"'' && 'chmod'", executable );
}

public void testPreserveSingleQuotesOnArgument()
Expand All @@ -78,13 +78,11 @@ public void testPreserveSingleQuotesOnArgument()
sh.setWorkingDirectory( "/usr/bin" );
sh.setExecutable( "chmod" );

final String[] args = { "\"some arg with spaces\"" };

List<String> shellCommandLine = sh.getShellCommandLine( args );
List<String> shellCommandLine = sh.getShellCommandLine("\"some arg with spaces\"");

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()
Expand All @@ -94,13 +92,23 @@ public void testAddSingleQuotesOnArgumentWithSpaces()
sh.setWorkingDirectory( "/usr/bin" );
sh.setExecutable( "chmod" );

String[] args = { "some arg with spaces" };

List<String> shellCommandLine = sh.getShellCommandLine( args );
List<String> shellCommandLine = sh.getShellCommandLine("some arg with spaces");

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" );

List<String> shellCommandLine = sh.getShellCommandLine("arg'withquote");

assertEquals("cd '/usr/bin' && 'chmod' 'arg'\"'\"'withquote'", shellCommandLine.get(shellCommandLine.size() - 1));
}

public void testArgumentsWithSemicolon()
Expand All @@ -113,13 +121,11 @@ public void testArgumentsWithSemicolon()
sh.setWorkingDirectory( "/usr/bin" );
sh.setExecutable( "chmod" );

String[] args = { ";some&argwithunix$chars" };

List<String> shellCommandLine = sh.getShellCommandLine( args );
List<String> shellCommandLine = sh.getShellCommandLine(";some&argwithunix$chars");

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" );
Expand All @@ -132,7 +138,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" );
Expand All @@ -144,7 +150,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 );
Expand Down Expand Up @@ -186,13 +192,14 @@ public void testBourneShellQuotingCharacters()
commandline.createArg().setValue( "{" );
commandline.createArg().setValue( "}" );
commandline.createArg().setValue( "`" );
commandline.createArg().setValue( "#" );

List<String> 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 ) );
}

Expand Down