diff --git a/pkgs/hooks_runner/lib/src/utils/run_process.dart b/pkgs/hooks_runner/lib/src/utils/run_process.dart index c949fad58a..d576f86f18 100644 --- a/pkgs/hooks_runner/lib/src/utils/run_process.dart +++ b/pkgs/hooks_runner/lib/src/utils/run_process.dart @@ -4,7 +4,8 @@ import 'dart:async'; import 'dart:developer'; -import 'dart:io' show Process, ProcessException, ProcessResult, systemEncoding; +import 'dart:io' + show Platform, Process, ProcessException, ProcessResult, systemEncoding; import 'package:file/file.dart'; import 'package:logging/logging.dart'; @@ -58,11 +59,21 @@ Future runProcess({ workingDirectory: workingDirectory?.toFilePath(), environment: environment, includeParentEnvironment: includeParentEnvironment, - // Never run through a shell. On Windows, running an executable through - // `cmd.exe /c` mangles the command line when more than one argument is - // quoted (cmd strips the outer quotes when the line contains more than - // two quote characters), which breaks any invocation whose executable - // and arguments contain spaces. + // On Windows, only launch through `cmd.exe` when strictly necessary. + // + // `CreateProcess` (used when `runInShell` is `false`) can directly launch + // `.exe`/`.com` executables and applies the correct command-line quoting, + // so we avoid the shell for those: running them through `cmd.exe /c` + // mangles the command line when more than one argument is quoted (cmd + // strips the outer quotes when the line contains more than two quote + // characters), which breaks any invocation whose executable and arguments + // contain spaces. + // + // However, `CreateProcess` cannot resolve a bare command name to a + // `.bat`/`.cmd` shim via `PATHEXT`, nor execute such a shim directly (for + // example the executables generated by `dart install`). Those must go + // through `cmd.exe`, so we opt into the shell for them. + runInShell: Platform.isWindows && _needsShellOnWindows(executable), ); final stdoutSub = process.stdout.listen((List data) { @@ -117,6 +128,24 @@ Future runProcess({ } } +/// Whether [executable] must be launched through `cmd.exe` on Windows. +/// +/// `CreateProcess` can only directly launch `.exe`/`.com` binaries. Anything +/// else (a `.bat`/`.cmd` shim, or a bare command name that resolves to one via +/// `PATHEXT`) has to be run through the shell. +bool _needsShellOnWindows(Uri executable) { + final path = executable.toFilePath(); + final lastSeparator = path.lastIndexOf(RegExp(r'[\\/]')); + final fileName = lastSeparator == -1 + ? path + : path.substring(lastSeparator + 1); + final dot = fileName.lastIndexOf('.'); + // No extension (e.g. a bare command name resolved via `PATHEXT`). + if (dot <= 0) return true; + final extension = fileName.substring(dot).toLowerCase(); + return extension != '.exe' && extension != '.com'; +} + /// Drop in replacement of [ProcessResult]. class RunProcessResult { final int pid; diff --git a/pkgs/hooks_runner/test/utils/run_process_test.dart b/pkgs/hooks_runner/test/utils/run_process_test.dart index 25686c6786..a8e3fbf5d3 100644 --- a/pkgs/hooks_runner/test/utils/run_process_test.dart +++ b/pkgs/hooks_runner/test/utils/run_process_test.dart @@ -6,12 +6,11 @@ // contain a space (e.g. the default pub cache under a Windows user name with // a space, `C:\Users\First Last\AppData\Local\Pub\Cache\...`). // -// On Windows, `runProcess` runs through `cmd.exe` (`runInShell`) whenever a -// `workingDirectory` is passed. `cmd.exe`'s `/c` quote-stripping rule mangles -// the command line as soon as more than one token needs quoting because it -// contains a space (the executable path and an argument, or two arguments), -// causing errors like -// `'C:\Program' is not recognized as an internal or external command`. +// On Windows, `runProcess` only runs through `cmd.exe` (`runInShell`) when +// strictly necessary: bare command names (resolved via `PATHEXT`) and +// `.bat`/`.cmd` shims. For `.exe`/`.com` binaries it uses `CreateProcess` +// directly so command lines with multiple quoted tokens are not mangled by +// `cmd.exe`'s `/c` quote-stripping rule. import 'dart:io'; @@ -78,6 +77,55 @@ void main(List args) { expect(result.stdout, contains('ARGV:first arg|second arg')); }); + test( + 'runProcess runs bare .bat shim from working directory on Windows', + () async { + if (!Platform.isWindows) return; + + final binDir = await tempDirForTest(); + final batUri = binDir.resolve('test shim.bat'); + await File.fromUri(batUri).writeAsString( + '@echo off\r\necho SHIM_OK %*\r\n', + ); + + final result = await runProcess( + executable: Uri.parse('test shim'), + arguments: ['--help'], + workingDirectory: binDir, + logger: logger, + ); + + expect(result.exitCode, 0); + expect(result.stdout, contains('SHIM_OK')); + expect(result.stdout, contains('--help')); + }, + ); + + test( + 'runProcess runs bare .bat shim from PATH on Windows', + () async { + if (!Platform.isWindows) return; + + final binDir = await tempDirForTest(); + final batUri = binDir.resolve('test shim.bat'); + await File.fromUri(batUri).writeAsString( + '@echo off\r\necho SHIM_OK %*\r\n', + ); + + final originalPath = Platform.environment['PATH'] ?? ''; + final result = await runProcess( + executable: Uri.parse('test shim'), + arguments: ['--help'], + environment: {'PATH': '${binDir.toFilePath()};$originalPath'}, + logger: logger, + ); + + expect(result.exitCode, 0); + expect(result.stdout, contains('SHIM_OK')); + expect(result.stdout, contains('--help')); + }, + ); + test('runProcess handles arguments containing a space and quotes', () async { final workingDir = await tempDirForTest(); final result = await runProcess(