commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sgoes...@apache.org
Subject 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
Date Fri, 18 Jul 2008 19:39:05 GMT
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]);
     }
 }



Mime
View raw message