Skip to content

Commit fc2f007

Browse files
Copilotdanmoseley
Copilot
and
danmoseley
authored
Require RedirectStandardOutput/Error on PSI overloads instead of mutating
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/73a14058-c419-4277-b5e2-885c9c6c8607 Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
1 parent 9ff3e62 commit fc2f007

3 files changed

Lines changed: 36 additions & 10 deletions

File tree

‎src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,9 @@
366366
<data name="UseShellExecuteNotSupportedForScenario" xml:space="preserve">
367367
<value>UseShellExecute is not supported by Process.{0}. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID.</value>
368368
</data>
369+
<data name="RedirectStandardOutputAndErrorRequired" xml:space="preserve">
370+
<value>RedirectStandardOutput and RedirectStandardError must both be set to true when calling Process.{0} with a ProcessStartInfo.</value>
371+
</data>
369372
<data name="NotSupportedForNonChildProcess" xml:space="preserve">
370373
<value>On Unix, it's impossible to obtain the exit status of a non-child process.</value>
371374
</data>

‎src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,9 @@ public static Task<ProcessExitStatus> RunAsync(string fileName, IList<string>? a
197197
/// <returns>The captured text output and exit status of the process.</returns>
198198
/// <exception cref="ArgumentNullException"><paramref name="startInfo"/> is <see langword="null"/>.</exception>
199199
/// <exception cref="InvalidOperationException">
200-
/// <see cref="ProcessStartInfo.UseShellExecute"/> is set to <see langword="true"/>.
200+
/// <para><see cref="ProcessStartInfo.UseShellExecute"/> is set to <see langword="true"/>.</para>
201+
/// <para>-or-</para>
202+
/// <para><see cref="ProcessStartInfo.RedirectStandardOutput"/> or <see cref="ProcessStartInfo.RedirectStandardError"/> is not set to <see langword="true"/>.</para>
201203
/// </exception>
202204
[UnsupportedOSPlatform("ios")]
203205
[UnsupportedOSPlatform("tvos")]
@@ -207,9 +209,7 @@ public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, Ti
207209
ArgumentNullException.ThrowIfNull(startInfo);
208210

209211
ThrowIfUseShellExecute(startInfo, nameof(RunAndCaptureText));
210-
211-
startInfo.RedirectStandardOutput = true;
212-
startInfo.RedirectStandardError = true;
212+
ThrowIfNotRedirected(startInfo, nameof(RunAndCaptureText));
213213

214214
long startTimestamp = Stopwatch.GetTimestamp();
215215

@@ -263,7 +263,7 @@ public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, Ti
263263
[UnsupportedOSPlatform("tvos")]
264264
[SupportedOSPlatform("maccatalyst")]
265265
public static ProcessTextOutput RunAndCaptureText(string fileName, IList<string>? arguments = null, TimeSpan? timeout = default)
266-
=> RunAndCaptureText(CreateStartInfo(fileName, arguments), timeout);
266+
=> RunAndCaptureText(CreateStartInfoForCapture(fileName, arguments), timeout);
267267

268268
/// <summary>
269269
/// Asynchronously starts the process described by <paramref name="startInfo"/>, captures its standard output and error,
@@ -277,7 +277,9 @@ public static ProcessTextOutput RunAndCaptureText(string fileName, IList<string>
277277
/// <returns>A task that represents the asynchronous operation. The value of the task contains the captured text output and exit status of the process.</returns>
278278
/// <exception cref="ArgumentNullException"><paramref name="startInfo"/> is <see langword="null"/>.</exception>
279279
/// <exception cref="InvalidOperationException">
280-
/// <see cref="ProcessStartInfo.UseShellExecute"/> is set to <see langword="true"/>.
280+
/// <para><see cref="ProcessStartInfo.UseShellExecute"/> is set to <see langword="true"/>.</para>
281+
/// <para>-or-</para>
282+
/// <para><see cref="ProcessStartInfo.RedirectStandardOutput"/> or <see cref="ProcessStartInfo.RedirectStandardError"/> is not set to <see langword="true"/>.</para>
281283
/// </exception>
282284
[UnsupportedOSPlatform("ios")]
283285
[UnsupportedOSPlatform("tvos")]
@@ -287,9 +289,7 @@ public static async Task<ProcessTextOutput> RunAndCaptureTextAsync(ProcessStartI
287289
ArgumentNullException.ThrowIfNull(startInfo);
288290

289291
ThrowIfUseShellExecute(startInfo, nameof(RunAndCaptureTextAsync));
290-
291-
startInfo.RedirectStandardOutput = true;
292-
startInfo.RedirectStandardError = true;
292+
ThrowIfNotRedirected(startInfo, nameof(RunAndCaptureTextAsync));
293293

294294
using Process process = Start(startInfo)!;
295295

@@ -331,7 +331,7 @@ public static async Task<ProcessTextOutput> RunAndCaptureTextAsync(ProcessStartI
331331
[UnsupportedOSPlatform("tvos")]
332332
[SupportedOSPlatform("maccatalyst")]
333333
public static Task<ProcessTextOutput> RunAndCaptureTextAsync(string fileName, IList<string>? arguments = null, CancellationToken cancellationToken = default)
334-
=> RunAndCaptureTextAsync(CreateStartInfo(fileName, arguments), cancellationToken);
334+
=> RunAndCaptureTextAsync(CreateStartInfoForCapture(fileName, arguments), cancellationToken);
335335

336336
private static ProcessStartInfo CreateStartInfo(string fileName, IList<string>? arguments)
337337
{
@@ -349,12 +349,29 @@ private static ProcessStartInfo CreateStartInfo(string fileName, IList<string>?
349349
return startInfo;
350350
}
351351

352+
private static ProcessStartInfo CreateStartInfoForCapture(string fileName, IList<string>? arguments)
353+
{
354+
ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments);
355+
startInfo.RedirectStandardOutput = true;
356+
startInfo.RedirectStandardError = true;
357+
358+
return startInfo;
359+
}
360+
352361
private static void ThrowIfUseShellExecute(ProcessStartInfo startInfo, string methodName)
353362
{
354363
if (startInfo.UseShellExecute)
355364
{
356365
throw new InvalidOperationException(SR.Format(SR.UseShellExecuteNotSupportedForScenario, methodName));
357366
}
358367
}
368+
369+
private static void ThrowIfNotRedirected(ProcessStartInfo startInfo, string methodName)
370+
{
371+
if (!startInfo.RedirectStandardOutput || !startInfo.RedirectStandardError)
372+
{
373+
throw new InvalidOperationException(SR.Format(SR.RedirectStandardOutputAndErrorRequired, methodName));
374+
}
375+
}
359376
}
360377
}

‎src/libraries/System.Diagnostics.Process/tests/RunTests.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ public async Task RunAndCaptureText_CapturesOutput(bool useAsync)
9090
return RemoteExecutor.SuccessExitCode;
9191
});
9292

93+
template.StartInfo.RedirectStandardOutput = true;
94+
template.StartInfo.RedirectStandardError = true;
95+
9396
ProcessTextOutput result = useAsync
9497
? await Process.RunAndCaptureTextAsync(template.StartInfo)
9598
: Process.RunAndCaptureText(template.StartInfo);
@@ -131,6 +134,9 @@ public async Task RunAndCaptureText_EmptyOutput(bool useAsync)
131134
{
132135
using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode);
133136

137+
template.StartInfo.RedirectStandardOutput = true;
138+
template.StartInfo.RedirectStandardError = true;
139+
134140
ProcessTextOutput result = useAsync
135141
? await Process.RunAndCaptureTextAsync(template.StartInfo)
136142
: Process.RunAndCaptureText(template.StartInfo);

0 commit comments

Comments
 (0)