In class/Microsoft.Build.Utilities/Microsoft.Build.Utilities:
authorAnkit Jain <radical@corewars.org>
Wed, 30 Jul 2008 19:45:41 +0000 (19:45 -0000)
committerAnkit Jain <radical@corewars.org>
Wed, 30 Jul 2008 19:45:41 +0000 (19:45 -0000)
* CommandLineBuilder.cs (chars): Use a char array instead of a
hashtable. Add ';' to the list.
(embeddedQuotes): Regex not required.
(IsQuotingRequired): Update.
(VerifyThrowNoEmbeddedDoubleQuotes): Update.
Use VerifyThrowNoEmbeddedDoubleQuotes at appropriate points (see tests).

In class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities:

* CommandLineBuilderTest.cs: Add more tests.

svn path=/trunk/mcs/; revision=109297

mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/ChangeLog
mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/CommandLineBuilder.cs
mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/ChangeLog
mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/CommandLineBuilderTest.cs [changed mode: 0644->0755]

index 637361ff0090d9004a72e140f83db6cc7d626a8e..092e3743507b6e6badd9446ab099fd8ea845d473 100644 (file)
@@ -1,3 +1,12 @@
+2008-07-31  Ankit Jain  <jankit@novell.com>
+
+       * CommandLineBuilder.cs (chars): Use a char array instead of a
+       hashtable. Add ';' to the list.
+       (embeddedQuotes): Regex not required.
+       (IsQuotingRequired): Update.
+       (VerifyThrowNoEmbeddedDoubleQuotes): Update.
+       Use VerifyThrowNoEmbeddedDoubleQuotes at appropriate points (see tests).
+
 2007-05-16  Marek Sieradzki  <marek.sieradzki@gmail.com>
 
        * CommandLineBuilder.cs: Add quotes when there are needed.
index 200118eec962b021e28707b139751f8842258995..417626a63d639e8ef8afcd3e92c0be410e4ab26a 100644 (file)
@@ -38,22 +38,11 @@ namespace Microsoft.Build.Utilities
        public class CommandLineBuilder
        {
                StringBuilder commandLine;
-               static Hashtable chars;
-               static Regex embeddedQuotes;
+               static char [] chars;
        
                static CommandLineBuilder ()
                {
-                       chars = new Hashtable ();
-
-                       chars.Add (' ', null);
-                       chars.Add ('\t', null);
-                       chars.Add ('\n', null);
-                       chars.Add ('\u000b', null);
-                       chars.Add ('\u000c', null);
-                       chars.Add ('\'', null);
-                       chars.Add ('\"', null);
-
-                       embeddedQuotes = new Regex ("\"\"");
+                       chars = new char [] {' ', '\t', '\n', '\u000b', '\u000c', '\'', '\"', ';'};
                }
                
                public CommandLineBuilder ()
@@ -66,6 +55,7 @@ namespace Microsoft.Build.Utilities
                        if (fileName == null)
                                return;
                        
+                       VerifyThrowNoEmbeddedDoubleQuotes (null, fileName);
                        AppendSpaceIfNotEmpty ();
                        AppendFileNameWithQuoting (fileName);
                }
@@ -75,8 +65,10 @@ namespace Microsoft.Build.Utilities
                        if (fileItem == null)
                                return;
                        
+                       string filename = fileItem.ToString ();
+                       VerifyThrowNoEmbeddedDoubleQuotes (null, filename);
                        AppendSpaceIfNotEmpty ();
-                       AppendFileNameWithQuoting (fileItem.ToString ());
+                       AppendFileNameWithQuoting (filename);
                }
                
                public void AppendFileNamesIfNotNull (string[] fileNames,
@@ -91,13 +83,15 @@ namespace Microsoft.Build.Utilities
                        bool appendDelimiter = false;
                        AppendSpaceIfNotEmpty ();
                        for (int i = 0; i < fileNames.Length; i++) {
-                               if (fileNames [i] == null)
+                               string filename = fileNames [i];
+                               if (filename == null)
                                        continue;
+                               VerifyThrowNoEmbeddedDoubleQuotes (null, filename);
                                if (appendDelimiter) {
                                        commandLine.Append (delimiter);
-                                       AppendFileNameWithQuoting (fileNames [i]);
+                                       AppendFileNameWithQuoting (filename);
                                } else {
-                                       AppendFileNameWithQuoting (fileNames [i]);
+                                       AppendFileNameWithQuoting (filename);
                                        appendDelimiter = true;
                                }
                        }
@@ -115,13 +109,16 @@ namespace Microsoft.Build.Utilities
                        bool appendDelimiter = false;
                        AppendSpaceIfNotEmpty ();
                        for (int i = 0; i < fileItems.Length; i++) {
+                               string filename = fileItems [i].ToString ();
                                if (fileItems [i] == null)
                                        continue;
+
+                               VerifyThrowNoEmbeddedDoubleQuotes (null, filename);
                                if (appendDelimiter) {
                                        commandLine.Append (delimiter);
-                                       AppendFileNameWithQuoting (fileItems [i].ToString ());
+                                       AppendFileNameWithQuoting (filename);
                                } else {
-                                       AppendFileNameWithQuoting (fileItems [i].ToString ());
+                                       AppendFileNameWithQuoting (filename);
                                        appendDelimiter = true;
                                }
                        }
@@ -161,10 +158,11 @@ namespace Microsoft.Build.Utilities
                
                        if (parameter == null)
                                return;
-                       
+
+                       VerifyThrowNoEmbeddedDoubleQuotes (switchName, parameter);
                        AppendSpaceIfNotEmpty ();
-                       commandLine.AppendFormat ("{0}{1}",switchName,
-                               parameter);
+                       commandLine.Append (switchName);
+                       AppendTextWithQuoting (parameter);
                }
                
                public void AppendSwitchIfNotNull (string switchName,
@@ -176,9 +174,11 @@ namespace Microsoft.Build.Utilities
                        if (parameter == null)
                                return;
                        
+                       string value = parameter.ToString ();
+                       VerifyThrowNoEmbeddedDoubleQuotes (switchName, value);
                        AppendSpaceIfNotEmpty ();
-                       commandLine.AppendFormat ("{0}{1}",switchName,
-                               parameter.ToString ());
+                       commandLine.Append (switchName);
+                       AppendTextWithQuoting (value);
                }
                
                public void AppendSwitchIfNotNull (string switchName,
@@ -198,13 +198,16 @@ namespace Microsoft.Build.Utilities
                        commandLine.AppendFormat ("{0}",switchName);
                        bool appendDelimiter = false;
                        for (int i = 0; i < parameters.Length; i++) {
-                               if (parameters [i] == null)
+                               string value = parameters [i];
+                               if (value == null)
                                        continue;
+
+                               VerifyThrowNoEmbeddedDoubleQuotes (switchName, value);
                                if (appendDelimiter) {
                                        commandLine.Append (delimiter);
-                                       commandLine.Append (parameters [i]);
+                                       commandLine.Append (value);
                                } else {
-                                       commandLine.Append (parameters [i]);
+                                       commandLine.Append (value);
                                        appendDelimiter = true;
                                }
                        }
@@ -227,13 +230,16 @@ namespace Microsoft.Build.Utilities
                        commandLine.AppendFormat ("{0}",switchName);
                        bool appendDelimiter = false;
                        for (int i = 0; i < parameters.Length; i++) {
-                               if (parameters [i] == null)
+                               string value = parameters [i].ToString ();
+                               if (value == null)
                                        continue;
+
+                               VerifyThrowNoEmbeddedDoubleQuotes (switchName, value);
                                if (appendDelimiter) {
                                        commandLine.Append (delimiter);
-                                       commandLine.Append (parameters [i].ToString ());
+                                       commandLine.Append (value);
                                } else {
-                                       commandLine.Append (parameters [i].ToString ());
+                                       commandLine.Append (value);
                                        appendDelimiter = true;
                                }
                        }
@@ -341,27 +347,16 @@ namespace Microsoft.Build.Utilities
                
                protected virtual bool IsQuotingRequired (string parameter)
                {
-                       parameter.Trim ();
-                       // FIXME: change this to regex
-                       foreach (char c in parameter) {
-                               if (chars.Contains (c))
-                                       return true;
-                       }
-                       return false;
+                       return parameter != null && parameter.IndexOfAny (chars) >= 0;
                }
                
                protected virtual void VerifyThrowNoEmbeddedDoubleQuotes (string switchName,
                                                                         string parameter)
                {
-                       if (parameter != null) {
-                               Match m = embeddedQuotes.Match (parameter);
-
-                               if (m.Success)
-                                       throw new ArgumentException (
-                                               String.Format ("Illegal quote passed to the command line switch named \"{0}\". The value was [{1}].",
-                                                       switchName, parameter));
-                       }
-
+                       if (parameter != null && parameter.IndexOf ('"') >= 0)
+                               throw new ArgumentException (
+                                       String.Format ("Illegal quote passed to the command line switch named \"{0}\". The value was [{1}].",
+                                               switchName, parameter));
                }
                
                public override string ToString ()
index 058dc5ce4f3ea8d63c6cf5acbf655fadceab4d20..01b3d725761b672e15e180a9a51dd2017882187b 100644 (file)
@@ -1,3 +1,7 @@
+2008-07-31  Ankit Jain  <jankit@novell.com>
+
+       * CommandLineBuilderTest.cs: Add more tests.
+
 2007-05-16  Marek Sieradzki  <marek.sieradzki@gmail.com>
 
        * CommandLineBuilderTest.cs: Added tests for filenames that contain
old mode 100644 (file)
new mode 100755 (executable)
index 2c8eda0..5a95afe
@@ -112,7 +112,6 @@ namespace MonoTests.Microsoft.Build.Utilities {
                [Test]
                public void TestAppendFileNameIfNotNull2 ()
                {
-                       
                        string filename = "filename.txt";
                        
                        clb = new CommandLineBuilder ();        
@@ -341,19 +340,20 @@ namespace MonoTests.Microsoft.Build.Utilities {
                        
                        clb.AppendSwitchIfNotNull ("/switch", array, null);
                }
-               
+
                [Test]
                public void TestAppendSwitchIfNotNull7 ()
                {
                        clb = new CommandLineBuilder ();
-                       
+
                        clb.AppendSwitchIfNotNull ("/switch:", (string[]) null, ";");
-                       
                        Assert.AreEqual (String.Empty, clb.ToString (), "A1");
                        
                        clb.AppendSwitchIfNotNull ("/switch:", array, ";");
-                       
                        Assert.AreEqual ("/switch:a;b;c", clb.ToString (), "A2");
+
+                       clb.AppendSwitchIfNotNull ("/switch:", "a;b;c");
+                       Assert.AreEqual ("/switch:a;b;c /switch:\"a;b;c\"", clb.ToString (), "A3");
                }
 
                [Test]
@@ -380,14 +380,38 @@ namespace MonoTests.Microsoft.Build.Utilities {
                        clb = new CommandLineBuilder ();
                        
                        clb.AppendSwitchIfNotNull ("/switch:", (ITaskItem[]) null, ";");
-                       
                        Assert.AreEqual (String.Empty, clb.ToString (), "A1");
                        
                        clb.AppendSwitchIfNotNull ("/switch:", items, ";");
-                       
                        Assert.AreEqual ("/switch:a;b", clb.ToString (), "A2");
                }
 
+               [Test]
+               public void TestAppendSwitchIfNotNul11()
+               {
+                       clb = new CommandLineBuilder();
+
+                       clb.AppendSwitchIfNotNull("/z:", " a  b");
+                       clb.AppendSwitchIfNotNull("/z:", "c\tb");
+                       clb.AppendSwitchIfNotNull("/z:", "ab\n");
+                       clb.AppendSwitchIfNotNull("/z:", "xyz\u000babc");
+                       clb.AppendSwitchIfNotNull("/z:", "\u000cabc");
+                       clb.AppendSwitchIfNotNull("/z:", "a 'hello' b");
+                       clb.AppendSwitchIfNotNull("/z:", "a;b");
+
+                       Assert.AreEqual("/z:\" a  b\" /z:\"c\tb\" /z:\"ab\n\" " +
+                               "/z:\"xyz\u000babc\" /z:\"\u000cabc\" /z:\"a 'hello' b\" /z:\"a;b\"",
+                               clb.ToString(), "A1");
+               }
+
+               [Test]
+               [ExpectedException (typeof (ArgumentException))]
+               public void TestAppendSwitchIfNotNull12()
+               {
+                       clb = new CommandLineBuilder();
+                       clb.AppendSwitchIfNotNull("/z:", "x \"hello\" y");
+               }
+
                [Test]
                [ExpectedException (typeof (ArgumentNullException), "Parameter \"switchName\" cannot be null.")]
                public void TestAppendSwitchUnquotedIfNotNull1 ()
@@ -545,6 +569,7 @@ namespace MonoTests.Microsoft.Build.Utilities {
                {
                        CLBTester clbt = new CLBTester ();
 
+                       Assert.AreEqual (false, clbt.IsQuotingRequired (null), "A0");
                        Assert.AreEqual (false, clbt.IsQuotingRequired (""), "A1");
                        Assert.AreEqual (true, clbt.IsQuotingRequired (" "), "A2");
                        Assert.AreEqual (false, clbt.IsQuotingRequired ("a"), "A3");
@@ -557,6 +582,11 @@ namespace MonoTests.Microsoft.Build.Utilities {
                        Assert.AreEqual (true, clbt.IsQuotingRequired ("\n \n"), "A10");
                        Assert.AreEqual (true, clbt.IsQuotingRequired ("\t\t"), "A11");
                        Assert.AreEqual (true, clbt.IsQuotingRequired ("\t \t"), "A12");
+                       Assert.AreEqual (true, clbt.IsQuotingRequired("a;b"), "A13");
+                       Assert.AreEqual (true, clbt.IsQuotingRequired("a\u000bc"), "A14");
+                       Assert.AreEqual (true, clbt.IsQuotingRequired("a\u000cx"), "A15");
+                       Assert.AreEqual (true, clbt.IsQuotingRequired("\""), "A16");
+                       Assert.AreEqual (true, clbt.IsQuotingRequired(" abc"), "A17");
                }
 
                [Test]
@@ -569,18 +599,118 @@ namespace MonoTests.Microsoft.Build.Utilities {
                        clbt.VerifyThrowNoEmbeddedDoubleQuotes (null, "");
                        clbt.VerifyThrowNoEmbeddedDoubleQuotes (" ", "");
                        clbt.VerifyThrowNoEmbeddedDoubleQuotes ("", " ");
-                       clbt.VerifyThrowNoEmbeddedDoubleQuotes ("\"\"", "");
+                       clbt.VerifyThrowNoEmbeddedDoubleQuotes ("\"abc\"", "");
+                       clbt.VerifyThrowNoEmbeddedDoubleQuotes ("x\"y", "");
                        clbt.VerifyThrowNoEmbeddedDoubleQuotes ("\'\'", "\'\'");
                }
 
                [Test]
-               [ExpectedException (typeof (ArgumentException),
-                       "Illegal quote passed to the command line switch named \"a\". The value was [\"\"].")]
                public void TestVerifyThrowNoEmbeddedDoubleQuotes2 ()
                {
-                       CLBTester clbt = new CLBTester ();
+                       CheckVerifyThrowNoEmbeddedDoubleQuotes (new CLBTester (), "a", 
+                                       "\"a\"", true, typeof (ArgumentException), "A1");
+               }
+
+               [Test]
+               public void TestVerifyThrowNoEmbeddedDoubleQuotes3 ()
+               {
+                       CheckVerifyThrowNoEmbeddedDoubleQuotes (new CLBTester (), "a", "\"\"", true, typeof(ArgumentException), "A1");
+               }
+
+               [Test]
+               public void TestVerifyThrowNoEmbeddedDoubleQuotes4 ()
+               {
+                       CheckVerifyThrowNoEmbeddedDoubleQuotes (new CLBTester (), "a", "\"foo", true, typeof(ArgumentException), "A1");
+               }
+
+               [Test]
+               public void TestVerifyThrowNoEmbeddedDoubleQuotes5 ()
+               {
+                       CheckVerifyThrowNoEmbeddedDoubleQuotes (new CLBTester (), null, "a\"b", true, typeof(ArgumentException), "A1");
+               }
+
+               private void CheckVerifyThrowNoEmbeddedDoubleQuotes(CLBTester clbt, string switchName, string parameter,
+                               bool expectException, Type exceptionType, string id)
+               {
+                       try
+                       {
+                               clbt.VerifyThrowNoEmbeddedDoubleQuotes(switchName, parameter);
+                       } catch (Exception e) {
+                               if (!expectException)
+                                       Assert.Fail("({0}) Unexpected exception : {1}", id, exceptionType.ToString());
+                               if (e.GetType () != exceptionType)
+                                       Assert.Fail("({0}) Expected exception of {1} type but got {2}", id, exceptionType.ToString(), e.ToString());
+                               return;
+                       }
 
-                       clbt.VerifyThrowNoEmbeddedDoubleQuotes ("a", "\"\"");
+                       if (expectException)
+                               Assert.Fail("({0}) Didn't get expected exception {1}", id, exceptionType.ToString());
+               }
+
+               [Test]
+               [ExpectedException (typeof (ArgumentException))]
+               public void TestEmbeddedQuotes1()
+               {
+                       new CommandLineBuilder().AppendFileNameIfNotNull("a\"b");
+               }
+
+               [Test]
+               [ExpectedException(typeof(ArgumentException))]
+               public void TestEmbeddedQuotes2()
+               {
+                       new CommandLineBuilder().AppendFileNameIfNotNull(new TaskItem ("a\"b"));
+               }
+
+               [Test]
+               [ExpectedException(typeof(ArgumentException))]
+               public void TestEmbeddedQuotes3()
+               {
+                       new CommandLineBuilder().AppendFileNamesIfNotNull(new ITaskItem[] { new TaskItem ("xyz"), new TaskItem("a\"b") }, ":");
+               }
+
+               [Test]
+               [ExpectedException(typeof(ArgumentException))]
+               public void TestEmbeddedQuotes4()
+               {
+                       new CommandLineBuilder().AppendFileNamesIfNotNull(new string[] { "xyz", "a\"b" }, ":");
+               }
+
+               [Test]
+               [ExpectedException(typeof(ArgumentException))]
+               public void TestEmbeddedQuotes5()
+               {
+                       new CommandLineBuilder().AppendSwitchIfNotNull("foo", new TaskItem("a\"b"));
+               }
+
+               [Test]
+               [ExpectedException(typeof(ArgumentException))]
+               public void TestEmbeddedQuotes6()
+               {
+                       new CommandLineBuilder().AppendSwitchIfNotNull("foo", "a\"b");
+               }
+
+               [Test]
+               [ExpectedException(typeof(ArgumentException))]
+               public void TestEmbeddedQuotes7()
+               {
+                       new CommandLineBuilder().AppendSwitchIfNotNull("foo", new ITaskItem[] { new TaskItem ("xyz"), new TaskItem("a\"b") }, ":");
+               }
+
+               [Test]
+               [ExpectedException(typeof(ArgumentException))]
+               public void TestEmbeddedQuotes8()
+               {
+                       new CommandLineBuilder().AppendSwitchIfNotNull("foo", new string[] {"xyz", "a\"b" }, ":");
+               }
+
+               [Test]
+               public void TestEmbeddedQuotes9()
+               {
+                       CommandLineBuilder clb = new CommandLineBuilder();
+                       clb.AppendSwitchUnquotedIfNotNull("foo", new TaskItem("a\"b"));
+                       clb.AppendSwitchUnquotedIfNotNull("foo", "a\"b");
+                       clb.AppendSwitchUnquotedIfNotNull("foo", new ITaskItem[] { new TaskItem ("xyz"), new TaskItem("a\"b") }, ":");
+                       clb.AppendSwitchUnquotedIfNotNull("foo", new string[] { "xyz", "a\"b" }, ":");
                }
        }
 }