From 956659b22e3ec76b2b19755a760bd2241d66371a Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Wed, 19 Aug 2026 20:27:03 +1000 Subject: [PATCH] [minor] Obsolete the command-string overloads The overloads taking a single command string separate the executable from its arguments by splitting on the first space, so an executable path containing a space is split in the middle and the call fails. On Windows that covers the default install location of most software. Quoting does not help, because the split happens before any quote handling, leaving a filename with a leading quote and a truncated path. The string form is inherently ambiguous: no parse handles every combination of spaces and quotes without adopting a shell's full grammar, and a half-grammar moves the surprise rather than removing it. Deprecate the eleven affected overloads in favour of the argument-vector ones, which have no such ambiguity because the executable is passed separately. Tests that exercise the obsolete overloads keep calling them, since those overloads still have to work until they are removed; the suppression is scoped to exactly those tests rather than the file. The cancellation test used a command string only incidentally, so it moves to the argument-vector form instead. Fixes #41 --- README.md | 67 +++++++++++++++++++++++------- RunCommand.Test/RunCommandTests.cs | 12 +++++- RunCommand/RunCommand.cs | 23 ++++++++++ 3 files changed, 86 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index fac9307..66a5c5a 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ Or you can use the NuGet Package Manager in Visual Studio to search for and inst ### Basic Execution -The simplest way to execute a command is to use the `Execute` method. All methods return the process exit code: +The simplest way to execute a command is to use the `Execute` method, passing the executable and its arguments separately. All methods return the process exit code: ```csharp using ktsu.RunCommand; @@ -33,7 +33,7 @@ class Program { static void Main() { - int exitCode = RunCommand.Execute("echo Hello World!"); + int exitCode = RunCommand.Execute("dotnet", ["--version"]); if (exitCode == 0) { @@ -47,6 +47,32 @@ class Program } ``` +### Deprecated: single command strings + +The overloads taking one `command` string are obsolete. They separate the executable from its arguments by splitting on the **first space**, which cannot represent an executable path that itself contains a space — on Windows that includes anything under `C:\Program Files\`: + +```csharp +// Obsolete, and broken: splits into "C:\Program" plus "Files\Git\bin\git.exe --version" +await RunCommand.ExecuteAsync(@"C:\Program Files\Git\bin\git.exe --version"); + +// Correct +await RunCommand.ExecuteAsync(@"C:\Program Files\Git\bin\git.exe", ["--version"]); +``` + +Quoting does not rescue it, because the split happens before any quote handling. The string form is inherently ambiguous — no parse handles every combination of spaces and quotes without adopting a shell's full grammar — so rather than grow a half-grammar that moves the surprise elsewhere, these overloads are deprecated in favour of the argument-list ones, which have no such ambiguity because the executable is passed separately. + +Migration is mechanical: split the string yourself at the boundaries you meant. + +| Obsolete | Replacement | +| --- | --- | +| `Execute(command)` | `Execute(fileName, arguments)` | +| `Execute(command, outputHandler)` | `Execute(fileName, arguments, outputHandler)` | +| `Execute(command, elevation)` | `Execute(fileName, arguments, outputHandler, options)` | +| `ExecuteAsync(command)` | `ExecuteAsync(fileName, arguments)` | +| `ExecuteAsync(command, outputHandler)` | `ExecuteAsync(fileName, arguments, outputHandler)` | +| `ExecuteAsync(command, cancellationToken)` | `ExecuteAsync(fileName, arguments, outputHandler, cancellationToken)` | +| `ExecuteAsync(command, outputHandler, elevation, cancellationToken)` | `ExecuteAsync(fileName, arguments, outputHandler, options, cancellationToken)` | + ### Custom Output Handling To handle the output of the command, you can provide delegates to the `OutputHandler` class: @@ -59,7 +85,8 @@ class Program static void Main() { int exitCode = RunCommand.Execute( - command: "echo Hello World!", + fileName: "dotnet", + arguments: ["--version"], outputHandler: new( onStandardOutput: Console.Write, onStandardError: Console.Write @@ -85,7 +112,8 @@ class Program static void Main() { int exitCode = RunCommand.Execute( - command: "echo Hello World!", + fileName: "dotnet", + arguments: ["--version"], outputHandler: new LineOutputHandler( onStandardOutput: line => Console.WriteLine($"Output: {line}"), onStandardError: line => Console.WriteLine($"Error: {line}") @@ -108,7 +136,7 @@ class Program { static async Task Main() { - int exitCode = await RunCommand.ExecuteAsync("echo Hello World!"); + int exitCode = await RunCommand.ExecuteAsync("dotnet", ["--version"]); if (exitCode == 0) { @@ -133,7 +161,11 @@ class Program { static void Main() { - int exitCode = RunCommand.Execute("powershell -Command \"Get-Service\"", Elevation.Elevated); + int exitCode = RunCommand.Execute( + fileName: "powershell", + arguments: ["-Command", "Get-Service"], + outputHandler: new(), + options: new() { Elevation = Elevation.Elevated }); Console.WriteLine($"Process exited with code: {exitCode}"); } @@ -207,7 +239,8 @@ class Program static void Main() { int exitCode = RunCommand.Execute( - command: "echo Hello World!", + fileName: "dotnet", + arguments: ["--version"], outputHandler: new( onStandardOutput: Console.Write, onStandardError: Console.Write, @@ -222,14 +255,18 @@ class Program ### RunCommand Class -- `Execute(string command)`: Executes a command synchronously and returns the process exit code. -- `Execute(string command, OutputHandler outputHandler)`: Executes a command synchronously with custom output handling and returns the process exit code. -- `Execute(string command, Elevation elevation)`: Executes a command synchronously at the given elevation level. -- `Execute(string command, OutputHandler outputHandler, Elevation elevation)`: Executes a command synchronously with custom output handling at the given elevation level. -- `ExecuteAsync(string command)`: Executes a command asynchronously and returns a task with the process exit code. -- `ExecuteAsync(string command, OutputHandler outputHandler)`: Executes a command asynchronously with custom output handling and returns a task with the process exit code. -- `ExecuteAsync(string command, Elevation elevation)`: Executes a command asynchronously at the given elevation level. -- `ExecuteAsync(string command, OutputHandler outputHandler, Elevation elevation)`: Executes a command asynchronously with custom output handling at the given elevation level. +Passing the executable and its arguments separately: + +- `Execute(string fileName, IEnumerable arguments)`: Executes a command synchronously and returns the process exit code. +- `Execute(string fileName, IEnumerable arguments, OutputHandler outputHandler)`: Executes a command synchronously with custom output handling. +- `ExecuteAsync(string fileName, IEnumerable arguments)`: The asynchronous equivalent. +- `ExecuteAsync(string fileName, IEnumerable arguments, OutputHandler outputHandler)`: The asynchronous equivalent with custom output handling. +- `ExecuteAsync(string fileName, IEnumerable arguments, OutputHandler outputHandler, CancellationToken cancellationToken)`: As above, terminating the process and its children if the token is signalled. + +**Obsolete** — see [Deprecated: single command strings](#deprecated-single-command-strings): + +- `Execute(string command)`, `Execute(string command, OutputHandler outputHandler)`, `Execute(string command, Elevation elevation)`, `Execute(string command, OutputHandler outputHandler, Elevation elevation)` +- `ExecuteAsync(string command)`, `ExecuteAsync(string command, OutputHandler outputHandler)`, `ExecuteAsync(string command, Elevation elevation)`, `ExecuteAsync(string command, OutputHandler outputHandler, Elevation elevation)`, `ExecuteAsync(string command, CancellationToken cancellationToken)`, `ExecuteAsync(string command, OutputHandler outputHandler, CancellationToken cancellationToken)`, `ExecuteAsync(string command, OutputHandler outputHandler, Elevation elevation, CancellationToken cancellationToken)` - `Execute(string fileName, IEnumerable arguments, OutputHandler outputHandler, CommandOptions options)`: Executes a command synchronously with the given process options, passing arguments individually so no manual quoting is required. - `ExecuteAsync(string fileName, IEnumerable arguments, OutputHandler outputHandler, CommandOptions options)`: The asynchronous equivalent. - `ExecuteAsync(string fileName, IEnumerable arguments, OutputHandler outputHandler, CommandOptions options, CancellationToken cancellationToken)`: As above, terminating the process and its children if the token is signalled. diff --git a/RunCommand.Test/RunCommandTests.cs b/RunCommand.Test/RunCommandTests.cs index b523908..4d0c40b 100644 --- a/RunCommand.Test/RunCommandTests.cs +++ b/RunCommand.Test/RunCommandTests.cs @@ -14,6 +14,11 @@ private static string GetCopyCommand(string source, string destination) => ? $"cmd /c copy \"{source}\" \"{destination}\"" : $"cp {source} {destination}"; + // These tests cover the command-string overloads themselves, which are obsolete but still + // supported, so they have to keep calling them until those overloads are removed. The region + // ends after the last such test rather than covering the file. +#pragma warning disable CS0618 // Type or member is obsolete + [TestMethod] public void ExecuteShouldExecuteCommandAndReturnExitCode() { @@ -301,6 +306,8 @@ public void ExecuteShouldThrowArgumentNullExceptionWhenCommandIsNull() Assert.IsTrue(didThrow, "Expected an ArgumentNullException to be thrown."); } +#pragma warning restore CS0618 // Type or member is obsolete + /// /// Returns a command that reads a single file, as an executable plus separate arguments. Both /// tools exit 0 only when they can open the file, so a path that was wrongly split on its @@ -393,7 +400,10 @@ public async Task ExecuteAsyncShouldFailWhenArgumentWithSpacesIsPassedAsOneStrin // The unquoted single-string overload splits the path on its spaces, which is precisely the // failure the argument-list overload exists to avoid. This pins that difference down. +#pragma warning disable CS0618 // Type or member is obsolete -- this test exists to pin the very + // behaviour that made the command-string overloads obsolete, so it must call one. int exitCode = await RunCommand.ExecuteAsync($"{fileName} {string.Join(" ", arguments)}").ConfigureAwait(false); +#pragma warning restore CS0618 // Type or member is obsolete Assert.AreNotEqual(0, exitCode, "Expected the unquoted command string to mis-split the path."); } @@ -417,7 +427,7 @@ public async Task ExecuteAsyncShouldThrowWhenTokenIsAlreadyCancelled() await cancellationTokenSource.CancelAsync().ConfigureAwait(false); await Assert.ThrowsAsync( - () => RunCommand.ExecuteAsync("dotnet --version", cancellationTokenSource.Token)).ConfigureAwait(false); + () => RunCommand.ExecuteAsync("dotnet", ["--version"], new OutputHandler(), cancellationTokenSource.Token)).ConfigureAwait(false); } [TestMethod] diff --git a/RunCommand/RunCommand.cs b/RunCommand/RunCommand.cs index 4267a41..bf0b386 100644 --- a/RunCommand/RunCommand.cs +++ b/RunCommand/RunCommand.cs @@ -2,6 +2,7 @@ namespace ktsu.RunCommand; +using System; using System.Collections.Generic; using System.Diagnostics; using System.Runtime.InteropServices; @@ -22,6 +23,8 @@ public static class RunCommand /// /// The command to execute. /// The exit code of the executed process. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static int Execute(string command) => ExecuteAsync(command).Result; @@ -31,6 +34,8 @@ public static int Execute(string command) => /// The command to execute. /// The handler for processing command output. /// The exit code of the executed process. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static int Execute(string command, OutputHandler outputHandler) => ExecuteAsync(command, outputHandler).Result; @@ -40,6 +45,8 @@ public static int Execute(string command, OutputHandler outputHandler) => /// The command to execute. /// The privilege level under which to run the command. /// The exit code of the executed process. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static int Execute(string command, Elevation elevation) => ExecuteAsync(command, elevation).Result; @@ -54,6 +61,8 @@ public static int Execute(string command, Elevation elevation) => /// /// The privilege level under which to run the command. /// The exit code of the executed process. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static int Execute(string command, OutputHandler outputHandler, Elevation elevation) => ExecuteAsync(command, outputHandler, elevation).Result; @@ -99,6 +108,8 @@ public static int Execute(string fileName, IEnumerable arguments, Output /// /// The command to execute. /// A task representing the asynchronous operation with the process exit code. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static async Task ExecuteAsync(string command) => await ExecuteAsync(command, new OutputHandler()).ConfigureAwait(false); @@ -108,6 +119,8 @@ public static async Task ExecuteAsync(string command) /// The command to execute. /// The handler for processing command output. /// A task representing the asynchronous operation with the process exit code. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static async Task ExecuteAsync(string command, OutputHandler outputHandler) => await ExecuteAsync(command, outputHandler, Elevation.Default).ConfigureAwait(false); @@ -117,6 +130,8 @@ public static async Task ExecuteAsync(string command, OutputHandler outputH /// The command to execute. /// The privilege level under which to run the command. /// A task representing the asynchronous operation with the process exit code. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static async Task ExecuteAsync(string command, Elevation elevation) => await ExecuteAsync(command, new(), elevation).ConfigureAwait(false); @@ -131,6 +146,8 @@ public static async Task ExecuteAsync(string command, Elevation elevation) /// /// The privilege level under which to run the command. /// A task representing the asynchronous operation with the process exit code. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static async Task ExecuteAsync(string command, OutputHandler outputHandler, Elevation elevation) => await ExecuteAsync(command, outputHandler, elevation, CancellationToken.None).ConfigureAwait(false); @@ -142,6 +159,8 @@ public static async Task ExecuteAsync(string command, OutputHandler outputH /// A token that, when cancelled, terminates the running process and its children. /// /// A task representing the asynchronous operation with the process exit code. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static async Task ExecuteAsync(string command, CancellationToken cancellationToken) => await ExecuteAsync(command, new OutputHandler(), cancellationToken).ConfigureAwait(false); @@ -155,6 +174,8 @@ public static async Task ExecuteAsync(string command, CancellationToken can /// A token that, when cancelled, terminates the running process and its children. /// /// A task representing the asynchronous operation with the process exit code. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static async Task ExecuteAsync(string command, OutputHandler outputHandler, CancellationToken cancellationToken) => await ExecuteAsync(command, outputHandler, Elevation.Default, cancellationToken).ConfigureAwait(false); @@ -174,6 +195,8 @@ public static async Task ExecuteAsync(string command, OutputHandler outputH /// /// A task representing the asynchronous operation with the process exit code. /// The token was cancelled before the process exited. + [Obsolete("A command string is split on its first space, which cannot handle an executable path " + + "containing spaces. Use the overload taking a file name and an argument list instead.")] public static async Task ExecuteAsync(string command, OutputHandler outputHandler, Elevation elevation, CancellationToken cancellationToken) { Ensure.NotNull(command);