Fix mixtures of parentheses and spaces in windows command paths (#2138)

Author: jakemac53

.github/workflows/process.yaml

@@ -22,10 +22,10 @@ defaults:
 
 jobs:
   build:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        os: [ubuntu-latest]
+        os: [ubuntu-latest, windows-latest]
         sdk: [stable, dev]
     steps:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683

pkgs/process/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 5.0.5
+
+* Fix mixtures of parentheses and spaces in windows command paths.
+
 ## 5.0.4
 
 * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4.

pkgs/process/lib/src/interface/common.dart

@@ -18,24 +18,6 @@ const Map<String, String> _osToPathStyle = <String, String>{
   'windows': 'windows',
 };
 
-/// Sanitizes the executable path on Windows.
-/// https://github.com/dart-lang/sdk/issues/37751
-String sanitizeExecutablePath(String executable,
-    {Platform platform = const LocalPlatform()}) {
-  if (executable.isEmpty) {
-    return executable;
-  }
-  if (!platform.isWindows) {
-    return executable;
-  }
-  if (executable.contains(' ') && !executable.contains('"')) {
-    // Use quoted strings to indicate where the file name ends and the arguments begin;
-    // otherwise, the file name is ambiguous.
-    return '"$executable"';
-  }
-  return executable;
-}
-
 /// Searches the `PATH` for the executable that [executable] is supposed to launch.
 ///
 /// This first builds a list of candidate paths where the executable may reside.

pkgs/process/lib/src/interface/local_process_manager.dart

@@ -40,11 +40,11 @@ class LocalProcessManager implements ProcessManager {
   }) {
     try {
       return Process.start(
-        sanitizeExecutablePath(_getExecutable(
+        _getExecutable(
           command,
           workingDirectory,
           runInShell,
-        )),
+        ),
         _getArguments(command),
         workingDirectory: workingDirectory,
         environment: environment,
@@ -70,11 +70,11 @@ class LocalProcessManager implements ProcessManager {
   }) {
     try {
       return Process.run(
-        sanitizeExecutablePath(_getExecutable(
+        _getExecutable(
           command,
           workingDirectory,
           runInShell,
-        )),
+        ),
         _getArguments(command),
         workingDirectory: workingDirectory,
         environment: environment,
@@ -101,11 +101,11 @@ class LocalProcessManager implements ProcessManager {
   }) {
     try {
       return Process.runSync(
-        sanitizeExecutablePath(_getExecutable(
+        _getExecutable(
           command,
           workingDirectory,
           runInShell,
-        )),
+        ),
         _getArguments(command),
         workingDirectory: workingDirectory,
         environment: environment,

pkgs/process/pubspec.yaml

@@ -1,6 +1,6 @@
 name: process
 description: A pluggable, mockable process invocation abstraction for Dart.
-version: 5.0.4
+version: 5.0.5
 repository: https://github.com/dart-lang/tools/tree/main/pkgs/process
 issue_tracker: https://github.com/dart-lang/tools/issues?q=is%3Aissue+is%3Aopen+label%3Apackage%3Aprocess
 

pkgs/process/test/src/interface/common_test.dart

@@ -238,28 +238,6 @@ void main() {
                         '    C:\\.tmp_rand0\\dir2_rand0\n'))));
       });
 
-      test('when path has spaces', () {
-        expect(
-            sanitizeExecutablePath(r'Program Files\bla.exe',
-                platform: platform),
-            r'"Program Files\bla.exe"');
-        expect(
-            sanitizeExecutablePath(r'ProgramFiles\bla.exe', platform: platform),
-            r'ProgramFiles\bla.exe');
-        expect(
-            sanitizeExecutablePath(r'"Program Files\bla.exe"',
-                platform: platform),
-            r'"Program Files\bla.exe"');
-        expect(
-            sanitizeExecutablePath(r'"Program Files\bla.exe"',
-                platform: platform),
-            r'"Program Files\bla.exe"');
-        expect(
-            sanitizeExecutablePath(r'C:\"Program Files"\bla.exe',
-                platform: platform),
-            r'C:\"Program Files"\bla.exe');
-      });
-
       test('with absolute path when currentDirectory getter throws', () {
         final FileSystem fsNoCwd = MemoryFileSystemNoCwd(fs);
         final String command = fs.path.join(dir3.path, 'bla.exe');
@@ -378,13 +356,6 @@ void main() {
                         '    /.tmp_rand0/dir1_rand0\n'
                         '    /.tmp_rand0/dir2_rand0\n'))));
       });
-
-      test('when path has spaces', () {
-        expect(
-            sanitizeExecutablePath('/usr/local/bin/foo bar',
-                platform: platform),
-            '/usr/local/bin/foo bar');
-      });
     });
   });
   group('Real Filesystem', () {
@@ -571,6 +542,80 @@ void main() {
                       '    ${tmpDir.path}/path4\n'
                       '    ${tmpDir.path}/path5\n'))));
     });
+
+    group('can actually execute files', () {
+      void testCompileAndExecute(File mainFile) {
+        final localProcessManager = LocalProcessManager();
+        final exePath = '${mainFile.path}.exe';
+        // Create an executable we can actually run.
+        expect(
+            localProcessManager.runSync([
+              io.Platform.resolvedExecutable,
+              'compile',
+              'exe',
+              mainFile.path,
+              '-o',
+              exePath
+            ]).exitCode,
+            0);
+
+        for (final runInShell in const [true, false]) {
+          final result =
+              localProcessManager.runSync([exePath], runInShell: runInShell);
+          expect(result.exitCode, 0,
+              reason: 'runInShell: $runInShell\nstdout: ${result.stdout}\n'
+                  'stderr: ${result.stderr}');
+          expect(result.stdout, contains('hello'));
+        }
+      }
+
+      test('with spaces in the command name', () {
+        final dir = tmpDir.childDirectory('the path');
+        final main = dir.childFile('main.dart')
+          ..createSync(recursive: true)
+          ..writeAsStringSync('''
+void main() {
+  print('hello');
+}''');
+        testCompileAndExecute(main);
+      });
+
+      test('with parenthesis in the command name', () async {
+        final dir = tmpDir.childDirectory('theP()ath');
+        final main = dir.childFile('main.dart')
+          ..createSync(recursive: true)
+          ..writeAsStringSync('''
+void main() {
+  print('hello');
+}''');
+        testCompileAndExecute(main);
+      },
+          skip: io.Platform.isWindows
+              ? 'https://github.com/dart-lang/tools/issues/2139'
+              : null);
+
+      test('with spaces and parenthesis in the command name', () async {
+        final dir = tmpDir.childDirectory('the P()ath');
+        final main = dir.childFile('main.dart')
+          ..createSync(recursive: true)
+          ..writeAsStringSync('''
+void main() {
+  print('hello');
+}''');
+        testCompileAndExecute(main);
+      });
+
+      test('with spaces inside parenthesis in the command name', () async {
+        final dir = tmpDir.childDirectory('the P( )ath');
+        final main = dir.childFile('main.dart')
+          ..createSync(recursive: true)
+          ..writeAsStringSync('''
+void main() {
+  print('hello');
+}''');
+        testCompileAndExecute(main);
+      });
+    });
   });
 }