ArgumentList added to Process.StartInfo (#27191)

Anipik · web-flow · commit 4cdbe10de119 · 2018-03-15T18:07:50.000-07:00
diff --git a/src/Common/src/System/PasteArguments.cs b/src/Common/src/System/PasteArguments.cs
@@ -2,14 +2,13 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Collections.Generic;
 using System.Text;
 
 namespace System
 {
     internal static partial class PasteArguments
     {
-        private static void AppendArgument(StringBuilder stringBuilder, string argument)
+        internal static void AppendArgument(StringBuilder stringBuilder, string argument)
         {
             if (stringBuilder.Length != 0)
             {
diff --git a/src/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs b/src/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs
@@ -158,6 +158,7 @@ public ProcessStartInfo() { }
         public ProcessStartInfo(string fileName) { }
         public ProcessStartInfo(string fileName, string arguments) { }
         public string Arguments { get { throw null; } set { } }
+        public System.Collections.ObjectModel.Collection<string> ArgumentList { get { throw null; } }
         public bool CreateNoWindow { get { throw null; } set { } }
         public string Domain { get { throw null; } set { } }
         [System.ComponentModel.DefaultValueAttribute(null)]
diff --git a/src/System.Diagnostics.Process/src/Resources/Strings.resx b/src/System.Diagnostics.Process/src/Resources/Strings.resx
@@ -315,4 +315,7 @@
   <data name="UserDoesNotExist" xml:space="preserve">
     <value>User with name '{0}' was not found.</value>
   </data>
+  <data name="ArgumentAndArgumentListInitialized" xml:space="preserve">
+    <value>Only one of Arguments or ArgumentList may be used.</value>
+  </data>
 </root>
diff --git a/src/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj
@@ -47,6 +47,9 @@
     <Compile Include="System\Diagnostics\ThreadWaitReason.cs" />
     <Compile Include="System\Diagnostics\MonitoringDescriptionAttribute.cs" />
     <Compile Include="System\Collections\Specialized\StringDictionaryWrapper.cs" />
+    <Compile Include="..\..\Common\src\System\PasteArguments.cs">
+      <Link>Common\System\PasteArguments.cs</Link>
+    </Compile>
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetsWindows)' == 'true' and '$(TargetGroup)' != 'uap'">
     <Compile Include="$(CommonPath)\Interop\Windows\kernel32\Interop.EnumProcessModules.cs">
diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs
@@ -376,15 +376,14 @@ private bool StartCore(ProcessStartInfo startInfo)
         /// <summary>Size to use for redirect streams and stream readers/writers.</summary>
         private const int StreamBufferSize = 4096;
 
-
         /// <summary>Converts the filename and arguments information from a ProcessStartInfo into an argv array.</summary>
         /// <param name="psi">The ProcessStartInfo.</param>
         /// <param name="alternativePath">alternative resolved path to use as first argument</param>
         /// <returns>The argv array.</returns>
         private static string[] ParseArgv(ProcessStartInfo psi, string alternativePath = null)
         {
             string argv0 = psi.FileName; // when no alternative path exists, pass filename (instead of resolved path) as argv[0], to match what caller supplied
-            if (string.IsNullOrEmpty(psi.Arguments) && string.IsNullOrEmpty(alternativePath))
+            if (string.IsNullOrEmpty(psi.Arguments) && string.IsNullOrEmpty(alternativePath) && psi.ArgumentList.Count == 0)
             {
                 return new string[] { argv0 };
             }
@@ -398,8 +397,16 @@ private static string[] ParseArgv(ProcessStartInfo psi, string alternativePath =
                     argvList.Add("openURL"); // kfmclient needs OpenURL
                 }
             }
+
             argvList.Add(argv0);
-            ParseArgumentsIntoList(psi.Arguments, argvList);
+            if (!string.IsNullOrEmpty(psi.Arguments))
+            {
+                ParseArgumentsIntoList(psi.Arguments, argvList);
+            }
+            else
+            {
+                argvList.AddRange(psi.ArgumentList);
+            }
             return argvList.ToArray();
         }
 
diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs
@@ -46,9 +46,21 @@ private unsafe bool StartWithShellExecuteEx(ProcessStartInfo startInfo)
             if (startInfo._environmentVariables != null)
                 throw new InvalidOperationException(SR.CantUseEnvVars);
 
+            string arguments;
+            if (startInfo.ArgumentList.Count > 0)
+            {
+                StringBuilder sb = new StringBuilder();
+                Process.AppendArguments(sb, startInfo.ArgumentList);
+                arguments = sb.ToString();
+            }
+            else
+            {
+                arguments = startInfo.Arguments;
+            }
+
             fixed (char* fileName = startInfo.FileName.Length > 0 ? startInfo.FileName : null)
             fixed (char* verb = startInfo.Verb.Length > 0 ? startInfo.Verb : null)
-            fixed (char* parameters = startInfo.Arguments.Length > 0 ? startInfo.Arguments : null)
+            fixed (char* parameters = arguments.Length > 0 ? arguments : null)
             fixed (char* directory = startInfo.WorkingDirectory.Length > 0 ? startInfo.WorkingDirectory : null)
             {
                 Interop.Shell32.SHELLEXECUTEINFO shellExecuteInfo = new Interop.Shell32.SHELLEXECUTEINFO()
diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs
@@ -445,8 +445,9 @@ private bool StartWithCreateProcess(ProcessStartInfo startInfo)
             //    * CreateProcess allows you to redirect all or none of the standard IO handles, so we use
             //      GetStdHandle for the handles that are not being redirected
 
-            StringBuilder commandLine = BuildCommandLine(startInfo.FileName, startInfo.Arguments);
-
+            StringBuilder commandLine = BuildCommandLine(startInfo.FileName, StartInfo.Arguments);
+            Process.AppendArguments(commandLine, StartInfo.ArgumentList);
+            
             Interop.Kernel32.STARTUPINFO startupInfo = new Interop.Kernel32.STARTUPINFO();
             Interop.Kernel32.PROCESS_INFORMATION processInfo = new Interop.Kernel32.PROCESS_INFORMATION();
             Interop.Kernel32.SECURITY_ATTRIBUTES unused_SecAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES();
diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs
@@ -3,11 +3,10 @@
 // See the LICENSE file in the project root for more information.
 
 using Microsoft.Win32.SafeHandles;
-using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.ComponentModel;
 using System.Globalization;
 using System.IO;
-using System.Security;
 using System.Text;
 using System.Threading;
 
@@ -1198,6 +1197,10 @@ public bool Start()
             {
                 throw new InvalidOperationException(SR.StandardErrorEncodingNotAllowed);
             }
+            if (!string.IsNullOrEmpty(startInfo.Arguments) && startInfo.ArgumentList.Count > 0)
+            {
+                throw new InvalidOperationException(SR.ArgumentAndArgumentListInitialized);
+            }
 
             //Cannot start a new process and store its handle if the object has been disposed, since finalization has been suppressed.            
             if (_disposed)
@@ -1467,6 +1470,17 @@ internal void ErrorReadNotifyUser(String data)
             }
         }
 
+        private static void AppendArguments(StringBuilder stringBuilder, Collection<string> argumentList)
+        {
+            if (argumentList.Count > 0)
+            {
+                foreach (string argument in argumentList)
+                {
+                    PasteArguments.AppendArgument(stringBuilder, argument);
+                }
+            }
+        }
+
         /// <summary>
         /// This enum defines the operation mode for redirected process stream.
         /// We don't support switching between synchronous mode and asynchronous mode.
diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs
@@ -4,6 +4,7 @@
 
 using System.Collections;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Collections.Specialized;
 using System.ComponentModel;
 using System.Text;
@@ -22,6 +23,7 @@ public sealed partial class ProcessStartInfo
         private string _directory;
         private string _userName;
         private string _verb;
+        private Collection<string> _argumentList;
         private ProcessWindowStyle _windowStyle;
 
         internal DictionaryWrapper _environmentVariables;
@@ -61,6 +63,18 @@ public string Arguments
             set => _arguments = value;
         }
 
+        public Collection<string> ArgumentList
+        {
+            get
+            {
+                if (_argumentList == null)
+                {
+                    _argumentList = new Collection<string>();
+                }
+                return _argumentList;
+            }
+        }
+
         public bool CreateNoWindow { get; set; }
 
         public StringDictionary EnvironmentVariables => new StringDictionaryWrapper(Environment as DictionaryWrapper);
diff --git a/src/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs
@@ -987,6 +987,7 @@ public void StartInfo_NotepadWithContent(bool useShellExecute)
                 }
             }
         }
+
         [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer), // Nano does not support UseShellExecute
                                                     nameof(PlatformDetection.IsNotWindows8x))] // https://github.com/dotnet/corefx/issues/20388
         [OuterLoop("Launches notepad")]
diff --git a/src/System.Diagnostics.Process/tests/ProcessStartInfoTests.netcoreapp.cs b/src/System.Diagnostics.Process/tests/ProcessStartInfoTests.netcoreapp.cs
@@ -0,0 +1,74 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.IO;
+using Xunit;
+
+namespace System.Diagnostics.Tests
+{
+    public partial class ProcessStartInfoTests
+    {
+        [Fact]
+        public void UnintializedArgumentList()
+        {
+            ProcessStartInfo psi = new ProcessStartInfo();
+            Assert.Equal(0, psi.ArgumentList.Count);
+
+            psi = new ProcessStartInfo("filename", "-arg1 -arg2");
+            Assert.Equal(0, psi.ArgumentList.Count);
+        }
+
+        [Fact]
+        public void InitializeWithArgumentList()
+        {
+            ProcessStartInfo psi = new ProcessStartInfo("filename");
+            psi.ArgumentList.Add("arg1");
+            psi.ArgumentList.Add("arg2");
+
+            Assert.Equal(2, psi.ArgumentList.Count);
+            Assert.Equal("arg1", psi.ArgumentList[0]);
+            Assert.Equal("arg2", psi.ArgumentList[1]);
+        }
+
+        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] // No Notepad on Nano
+        [MemberData(nameof(UseShellExecute))]
+        [OuterLoop("Launches notepad")]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        [SkipOnTargetFramework(TargetFrameworkMonikers.Uap, "WaitForInputIdle, ProcessName, and MainWindowTitle are not supported on UAP")]
+        public void StartInfo_NotepadWithContent_withArgumentList(bool useShellExecute)
+        {
+            string tempFile = GetTestFilePath() + ".txt";
+            File.WriteAllText(tempFile, $"StartInfo_NotepadWithContent({useShellExecute})");
+
+            ProcessStartInfo info = new ProcessStartInfo
+            {
+                UseShellExecute = useShellExecute,
+                FileName = @"notepad.exe",
+                Arguments = null,
+                WindowStyle = ProcessWindowStyle.Minimized
+            };
+
+            info.ArgumentList.Add(tempFile);
+
+            using (var process = Process.Start(info))
+            {
+                Assert.True(process != null, $"Could not start {info.FileName} {info.Arguments} UseShellExecute={info.UseShellExecute}");
+
+                try
+                {
+                    process.WaitForInputIdle(); // Give the file a chance to load
+                    Assert.Equal("notepad", process.ProcessName);
+
+                    // On some Windows versions, the file extension is not included in the title
+                    Assert.StartsWith(Path.GetFileNameWithoutExtension(tempFile), process.MainWindowTitle);
+                }
+                finally
+                {
+                    if (process != null && !process.HasExited)
+                        process.Kill();
+                }
+            }
+        }
+    }
+}
diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs
@@ -178,6 +178,31 @@ public void ProcessStart_OpenFileOnLinux_UsesSpecifiedProgram(string programToOp
             }
         }
 
+        [Theory, InlineData("nano"), InlineData("vi")]
+        [PlatformSpecific(TestPlatforms.Linux)]
+        [OuterLoop("Opens program")]
+        public void ProcessStart_OpenFileOnLinux_UsesSpecifiedProgramUsingArgumentList(string programToOpenWith)
+        {
+            if (IsProgramInstalled(programToOpenWith))
+            {
+                string fileToOpen = GetTestFilePath() + ".txt";
+                File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_OpenFileOnLinux_UsesSpecifiedProgram)}");
+                ProcessStartInfo psi = new ProcessStartInfo(programToOpenWith);
+                psi.ArgumentList.Add(fileToOpen);
+                using (var px = Process.Start(psi))
+                {
+                    Assert.Equal(programToOpenWith, px.ProcessName);
+                    px.Kill();
+                    px.WaitForExit();
+                    Assert.True(px.HasExited);
+                }
+            }
+            else
+            {
+                Console.WriteLine($"Program specified to open file with {programToOpenWith} is not installed on this machine.");
+            }
+        }
+
         [Theory, InlineData("/usr/bin/open"), InlineData("/usr/bin/nano")]
         [PlatformSpecific(TestPlatforms.OSX)]
         [OuterLoop("Opens program")]
@@ -224,6 +249,34 @@ public void ProcessStart_UseShellExecuteTrue_OpenUrl_SuccessfullyReadsArgument(s
             }
         }
 
+        public static TheoryData<string[]> StartOSXProcessWithArgumentList => new TheoryData<string[]>
+        {
+            { new string[] { "-a", "Safari" } },
+            { new string[] { "-a", "\"Google Chrome\"" } }
+        };
+
+        [Theory,
+            MemberData(nameof(StartOSXProcessWithArgumentList))]
+        [PlatformSpecific(TestPlatforms.OSX)]
+        [OuterLoop("Opens browser")]
+        public void ProcessStart_UseShellExecuteTrue_OpenUrl_SuccessfullyReadsArgument(string[] argumentList)
+        {
+            var startInfo = new ProcessStartInfo { UseShellExecute = true, FileName = "https://github.com/dotnet/corefx"};
+
+            foreach (string item in argumentList)
+            {
+                startInfo.ArgumentList.Add(item);
+            }
+
+            using (var px = Process.Start(startInfo))
+            {
+                Assert.NotNull(px);
+                px.Kill();
+                px.WaitForExit();
+                Assert.True(px.HasExited);
+            }
+        }
+
         [Fact]
         [Trait(XunitConstants.Category, XunitConstants.RequiresElevation)]
         public void TestPriorityClassUnix()
diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.netcoreapp.cs b/src/System.Diagnostics.Process/tests/ProcessTests.netcoreapp.cs
diff --git a/src/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj b/src/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj

Original file line number	Diff line number	Diff line change
`@@ -2,14 +2,13 @@`
`2`	`2`	`// The .NET Foundation licenses this file to you under the MIT license.`
`3`	`3`	`// See the LICENSE file in the project root for more information.`
`4`	`4`
`5`		`-using System.Collections.Generic;`
`6`	`5`	`using System.Text;`
`7`	`6`
`8`	`7`	`namespace System`
`9`	`8`	`{`
`10`	`9`	`internal static partial class PasteArguments`
`11`	`10`	`{`
`12`		`- private static void AppendArgument(StringBuilder stringBuilder, string argument)`
	`11`	`+ internal static void AppendArgument(StringBuilder stringBuilder, string argument)`
`13`	`12`	`{`
`14`	`13`	`if (stringBuilder.Length != 0)`
`15`	`14`	`{`
Original file line number	Diff line number	Diff line change
`@@ -376,15 +376,14 @@ private bool StartCore(ProcessStartInfo startInfo)`
`376`	`376`	`/// <summary>Size to use for redirect streams and stream readers/writers.</summary>`
`377`	`377`	`private const int StreamBufferSize = 4096;`
`378`	`378`
`379`		`-`
`380`	`379`	`/// <summary>Converts the filename and arguments information from a ProcessStartInfo into an argv array.</summary>`
`381`	`380`	`/// <param name="psi">The ProcessStartInfo.</param>`
`382`	`381`	`/// <param name="alternativePath">alternative resolved path to use as first argument</param>`
`383`	`382`	`/// <returns>The argv array.</returns>`
`384`	`383`	`private static string[] ParseArgv(ProcessStartInfo psi, string alternativePath = null)`
`385`	`384`	`{`
`386`	`385`	`string argv0 = psi.FileName; // when no alternative path exists, pass filename (instead of resolved path) as argv[0], to match what caller supplied`
`387`		`- if (string.IsNullOrEmpty(psi.Arguments) && string.IsNullOrEmpty(alternativePath))`
	`386`	`+ if (string.IsNullOrEmpty(psi.Arguments) && string.IsNullOrEmpty(alternativePath) && psi.ArgumentList.Count == 0)`
`388`	`387`	`{`
`389`	`388`	`return new string[] { argv0 };`
`390`	`389`	`}`
`@@ -398,8 +397,16 @@ private static string[] ParseArgv(ProcessStartInfo psi, string alternativePath =`
`398`	`397`	`argvList.Add("openURL"); // kfmclient needs OpenURL`
`399`	`398`	`}`
`400`	`399`	`}`
	`400`	`+`
`401`	`401`	`argvList.Add(argv0);`
`402`		`- ParseArgumentsIntoList(psi.Arguments, argvList);`
	`402`	`+ if (!string.IsNullOrEmpty(psi.Arguments))`
	`403`	`+ {`
	`404`	`+ ParseArgumentsIntoList(psi.Arguments, argvList);`
	`405`	`+ }`
	`406`	`+ else`
	`407`	`+ {`
	`408`	`+ argvList.AddRange(psi.ArgumentList);`
	`409`	`+ }`
`403`	`410`	`return argvList.ToArray();`
`404`	`411`	`}`
`405`	`412`
Original file line number	Diff line number	Diff line change
`@@ -987,6 +987,7 @@ public void StartInfo_NotepadWithContent(bool useShellExecute)`
`987`	`987`	`}`
`988`	`988`	`}`
`989`	`989`	`}`
	`990`	`+`
`990`	`991`	`[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer), // Nano does not support UseShellExecute`
`991`	`992`	`nameof(PlatformDetection.IsNotWindows8x))] // https://github.com/dotnet/corefx/issues/20388`
`992`	`993`	`[OuterLoop("Launches notepad")]`