You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by sg...@apache.org on 2008/07/18 21:39:05 UTC
svn commit: r678010 - in /commons/proper/exec/trunk/src:
main/java/org/apache/commons/exec/CommandLine.java
main/java/org/apache/commons/exec/util/StringUtils.java
test/java/org/apache/commons/exec/CommandLineTest.java
Author: sgoeschl
Date: Fri Jul 18 12:39:05 2008
New Revision: 678010
URL: http://svn.apache.org/viewvc?rev=678010&view=rev
Log:
[EXEC-25] Fixing variable substitution causing problems with platform specific file seperators
Modified:
commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/CommandLine.java
commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/util/StringUtils.java
commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/CommandLineTest.java
Modified: commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/CommandLine.java
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/CommandLine.java?rev=678010&r1=678009&r2=678010&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/CommandLine.java (original)
+++ commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/CommandLine.java Fri Jul 18 12:39:05 2008
@@ -44,7 +44,7 @@
/**
* The program to execute.
*/
- private String executable = null;
+ private String executable;
/**
* A map of name value pairs used to expand command line arguments
@@ -108,7 +108,7 @@
/**
* Create a command line without any arguments.
*
- * @param executable the executable
+ * @param executable the executable file
*/
public CommandLine(File executable) {
this.isFile=true;
@@ -156,6 +156,29 @@
}
/**
+ * Add exactly two arguments in one invocation. Handles parsing of quotes and whitespace.
+ *
+ * @param argument1 The first argument
+ * @param argument2 The second argument
+ * @return The command line itself
+ */
+ public CommandLine addArguments(final String argument1, final String argument2) {
+ return this.addArguments(argument1, true).addArguments(argument2, true);
+ }
+
+ /**
+ * Add exactly two arguments in one invocation.
+ *
+ * @param argument1 The first argument
+ * @param argument2 The second argument
+ * @param handleQuoting Add the argument with/without handling quoting
+ * @return The command line itself
+ */
+ public CommandLine addArguments(final String argument1, final String argument2, boolean handleQuoting) {
+ return this.addArguments(argument1, handleQuoting).addArguments(argument2, handleQuoting);
+ }
+
+ /**
* Add multiple arguments. Handles parsing of quotes and whitespace.
*
* @param arguments An string containing multiple arguments.
@@ -313,17 +336,20 @@
* @return The command line as an string array
*/
public String[] toStrings() {
+
final String[] result = new String[arguments.size() + 1];
- result[0] = executable;
+
+ // expand the executable and replace '/' and '\\' with the platform
+ // specific file seperator char
+ result[0] = StringUtils.fixFileSeperatorChar(expandArgument(executable));
int index = 1;
for (Iterator iter = arguments.iterator(); iter.hasNext();) {
- result[index] = (String) iter.next();
-
+ result[index] = expandArgument((String) iter.next());
index++;
}
- return expandArguments(result);
+ return result;
}
/**
@@ -428,8 +454,7 @@
} else if(executable.trim().length() == 0) {
throw new IllegalArgumentException("Executable can not be empty");
} else {
- this.executable = executable.replace('/', File.separatorChar).replace(
- '\\', File.separatorChar);
+ this.executable = StringUtils.fixFileSeperatorChar(executable);
}
}
Modified: commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/util/StringUtils.java
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/util/StringUtils.java?rev=678010&r1=678009&r2=678010&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/util/StringUtils.java (original)
+++ commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/util/StringUtils.java Fri Jul 18 12:39:05 2008
@@ -23,6 +23,7 @@
import java.util.List;
import java.util.Map;
import java.util.StringTokenizer;
+import java.io.File;
/**
* Supplement of commons-lang, the stringSubstitution() was in a simpler
@@ -32,119 +33,123 @@
*
* @author <a href="mailto:siegfried.goeschl@it20one.at">Siegfried Goeschl</a>
*/
-public class StringUtils
-{
- /**
- * Perform a series of substitutions. The substitions
- * are performed by replacing ${variable} in the target
- * string with the value of provided by the key "variable"
- * in the provided hashtable.
- *
- * @param argStr the argument string to be processed
- * @param vars name/value pairs used for substitution
- * @param isLenient ignore a key not found in vars?
- * @return String target string with replacements.
- */
- public static StringBuffer stringSubstitution(String argStr, Map vars, boolean isLenient)
- {
- StringBuffer argBuf = new StringBuffer();
-
- if(argStr == null || argStr.length() == 0) {
- return argBuf;
- }
-
- if(vars == null || vars.size() == 0) {
- return argBuf.append(argStr);
- }
-
- int argStrLength = argStr.length();
-
- for (int cIdx = 0 ; cIdx < argStrLength;)
- {
- char ch = argStr.charAt(cIdx);
- char del = ' ';
-
- switch (ch)
- {
- case '$':
- StringBuffer nameBuf = new StringBuffer();
- del = argStr.charAt(cIdx+1);
- if( del == '{')
- {
- cIdx++;
-
- for (++cIdx ; cIdx < argStr.length(); ++cIdx)
- {
- ch = argStr.charAt(cIdx);
- if (ch == '_' || ch == '.' || ch == '-' || ch == '+' || Character.isLetterOrDigit(ch))
- nameBuf.append(ch);
- else
- break;
- }
-
- if (nameBuf.length() > 0)
- {
- Object temp = vars.get(nameBuf.toString());
- String value = ( temp != null ? temp.toString() : null);
-
- if (value != null)
- {
- argBuf.append(value);
- }
- else
- {
- if (isLenient)
- {
- // just append the unresolved variable declaration
- argBuf.append("${" + nameBuf.toString() + "}");
- }
- else
- {
- // complain that no variable was found
- throw new RuntimeException("No value found for : " + nameBuf );
- }
- }
-
- del = argStr.charAt(cIdx);
-
- if( del != '}')
- {
- throw new RuntimeException("Delimiter not found for : " + nameBuf );
- }
- }
-
- cIdx++;
- }
- else
- {
- argBuf.append(ch);
- ++cIdx;
- }
-
- break;
-
- default:
- argBuf.append(ch);
- ++cIdx;
- break;
- }
- }
-
- return argBuf;
- }
-
- /**
- * Split a string into an array of strings
- * @param input what to split
- * @param splitChar what to split on
- * @return the array of strings
- */
- public static String[] split(String input, String splitChar){
- StringTokenizer tokens = new StringTokenizer(input, splitChar);
- List strList=new ArrayList();
- while (tokens.hasMoreTokens()) {
- strList.add(tokens.nextToken());
- }
- return (String[])strList.toArray(new String[0]);
- }
+public class StringUtils {
+ /**
+ * Perform a series of substitutions. The substitions
+ * are performed by replacing ${variable} in the target
+ * string with the value of provided by the key "variable"
+ * in the provided hashtable.
+ *
+ * @param argStr the argument string to be processed
+ * @param vars name/value pairs used for substitution
+ * @param isLenient ignore a key not found in vars?
+ * @return String target string with replacements.
+ */
+ public static StringBuffer stringSubstitution(String argStr, Map vars, boolean isLenient) {
+
+ StringBuffer argBuf = new StringBuffer();
+
+ if (argStr == null || argStr.length() == 0) {
+ return argBuf;
+ }
+
+ if (vars == null || vars.size() == 0) {
+ return argBuf.append(argStr);
+ }
+
+ int argStrLength = argStr.length();
+
+ for (int cIdx = 0; cIdx < argStrLength;) {
+
+ char ch = argStr.charAt(cIdx);
+ char del = ' ';
+
+ switch (ch) {
+
+ case '$':
+ StringBuffer nameBuf = new StringBuffer();
+ del = argStr.charAt(cIdx + 1);
+ if (del == '{') {
+ cIdx++;
+
+ for (++cIdx; cIdx < argStr.length(); ++cIdx) {
+ ch = argStr.charAt(cIdx);
+ if (ch == '_' || ch == '.' || ch == '-' || ch == '+' || Character.isLetterOrDigit(ch))
+ nameBuf.append(ch);
+ else
+ break;
+ }
+
+ if (nameBuf.length() > 0) {
+ Object temp = vars.get(nameBuf.toString());
+ String value = (temp != null ? temp.toString() : null);
+
+ if (value != null) {
+ argBuf.append(value);
+ } else {
+ if (isLenient) {
+ // just append the unresolved variable declaration
+ argBuf.append("${" + nameBuf.toString() + "}");
+ } else {
+ // complain that no variable was found
+ throw new RuntimeException("No value found for : " + nameBuf);
+ }
+ }
+
+ del = argStr.charAt(cIdx);
+
+ if (del != '}') {
+ throw new RuntimeException("Delimiter not found for : " + nameBuf);
+ }
+ }
+
+ cIdx++;
+ } else {
+ argBuf.append(ch);
+ ++cIdx;
+ }
+
+ break;
+
+ default:
+ argBuf.append(ch);
+ ++cIdx;
+ break;
+ }
+ }
+
+ return argBuf;
+ }
+
+ /**
+ * Split a string into an array of strings
+ *
+ * @param input what to split
+ * @param splitChar what to split on
+ * @return the array of strings
+ */
+ public static String[] split(String input, String splitChar) {
+ StringTokenizer tokens = new StringTokenizer(input, splitChar);
+ List strList = new ArrayList();
+ while (tokens.hasMoreTokens()) {
+ strList.add(tokens.nextToken());
+ }
+ return (String[]) strList.toArray(new String[0]);
+ }
+
+ /**
+ * Fixes the file sperator char for the target platform
+ * using the following replacement.
+ *
+ * <ul>
+ * <li> '/' ==> File.separatorChar
+ * <li> '\\' ==> File.separatorChar
+ * </ul>
+ */
+ public static String fixFileSeperatorChar(String arg) {
+ return arg.replace('/', File.separatorChar).replace(
+ '\\', File.separatorChar);
+ }
+
+
}
\ No newline at end of file
Modified: commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/CommandLineTest.java
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/CommandLineTest.java?rev=678010&r1=678009&r2=678010&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/CommandLineTest.java (original)
+++ commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/CommandLineTest.java Fri Jul 18 12:39:05 2008
@@ -23,6 +23,7 @@
import junit.framework.AssertionFailedError;
import junit.framework.TestCase;
+import org.apache.commons.exec.util.StringUtils;
public class CommandLineTest extends TestCase {
@@ -42,7 +43,7 @@
public void testExecutableZeroLengthString() {
try {
- CommandLine cmdl = new CommandLine("");
+ new CommandLine("");
fail("Must throw IllegalArgumentException");
} catch (IllegalArgumentException e) {
// Expected
@@ -51,7 +52,7 @@
public void testExecutableWhitespaceString() {
try {
- CommandLine cmdl = new CommandLine(" ");
+ new CommandLine(" ");
fail("Must throw IllegalArgumentException");
} catch (IllegalArgumentException e) {
// Expected
@@ -60,7 +61,7 @@
public void testNullExecutable() {
try {
- CommandLine cmdl = new CommandLine((String)null);
+ new CommandLine((String)null);
fail("Must throw IllegalArgumentException");
} catch (IllegalArgumentException e) {
// Expected
@@ -166,6 +167,13 @@
assertEquals(new String[] {"test"}, cmdl.toStrings());
}
+ public void testAddTwoArguments() {
+ CommandLine cmdl = new CommandLine("test");
+ cmdl.addArguments("foo", "bar");
+ assertEquals("test foo bar", cmdl.toString());
+ assertEquals(new String[] {"test", "foo", "bar"}, cmdl.toStrings());
+ }
+
public void testParseCommandLine() {
CommandLine cmdl = CommandLine.parse("test foo bar");
assertEquals("test foo bar", cmdl.toString());
@@ -211,13 +219,12 @@
* a "500x>" parameter (including quotes) and it is simply not possible to
* do that withoud adding a space, e.g. "500x> ".
*/
- public void testParseComplexCommandLine1() throws Exception {
+ public void testParseComplexCommandLine1() {
HashMap substitutionMap = new HashMap();
substitutionMap.put("in", "source.jpg");
substitutionMap.put("out", "target.jpg");
CommandLine cmdl = CommandLine.parse("cmd /C convert ${in} -resize \"\'500x> \'\" ${out}", substitutionMap);
- assertEquals("cmd /C convert source.jpg -resize \"500x> \" target.jpg", cmdl.toString());
- return;
+ assertEquals("cmd /C convert source.jpg -resize \"500x> \" target.jpg", cmdl.toString());
}
/**
@@ -296,13 +303,16 @@
}
/**
- * Test expanding the command line based on a user-supplied map.
+ * Test expanding the command line based on a user-supplied map. The main
+ * goal of the test is to setup a command line using macros and reuse
+ * it for multiple times.
*/
- public void testCommandLineParsingWithExpansion2() throws Exception {
+ public void testCommandLineParsingWithExpansion2() {
- CommandLine cmdl = null;
- String[] result = null;
+ CommandLine cmdl;
+ String[] result;
+ // build the user supplied parameters
HashMap substitutionMap = new HashMap();
substitutionMap.put("JAVA_HOME", "C:\\Programme\\jdk1.5.0_12");
substitutionMap.put("appMainClass", "foo.bar.Main");
@@ -317,12 +327,18 @@
substitutionMap.put("file", "C:\\Document And Settings\\documents\\432431.pdf");
cmdl.setSubstitutionMap(substitutionMap);
result = cmdl.toStrings();
- assertEquals(new String[] {"C:\\Programme\\jdk1.5.0_12/bin/java", "-class", "foo.bar.Main", "C:\\Document And Settings\\documents\\432431.pdf"}, result);
+ assertEquals(StringUtils.fixFileSeperatorChar("C:\\Programme\\jdk1.5.0_12\\bin\\java"), result[0]);
+ assertEquals("-class", result[1]);
+ assertEquals("foo.bar.Main", result[2]);
+ assertEquals("C:\\Document And Settings\\documents\\432431.pdf", result[3]);
- // build the second command line
+ // build the second command line with updated parameters resulting in a different command line
substitutionMap.put("file", "C:\\Document And Settings\\documents\\432432.pdf");
cmdl.setSubstitutionMap(substitutionMap);
result = cmdl.toStrings();
- assertEquals(new String[] {"C:\\Programme\\jdk1.5.0_12/bin/java", "-class", "foo.bar.Main", "C:\\Document And Settings\\documents\\432432.pdf"}, result);
+ assertEquals(StringUtils.fixFileSeperatorChar("C:\\Programme\\jdk1.5.0_12\\bin\\java"), result[0]);
+ assertEquals("-class", result[1]);
+ assertEquals("foo.bar.Main", result[2]);
+ assertEquals("C:\\Document And Settings\\documents\\432432.pdf", result[3]);
}
}