Skip to content

Unable to pass & in an argument to a shell script started with Process.start #59604

Closed

Description

@DanTup

I'm trying to fix a bug where we mishandle some characters like & in user-provided arguments because of having to spawn flutter.bat through a shell. There is a little info on Process.start() about this:

NOTE: On Windows, if executable is a batch file ('.bat' or '.cmd'), it may be launched by the operating system in a system shell regardless of the value of runInShell. This could result in arguments being parsed according to shell rules. For example:

  // Will launch notepad.
  Process.start('test.bat', ['test&notepad.exe']);
}

The issue I'm having is that I can't find any way to escape a & that comes through correctly.

Below is a script to reproduce the issue. It creates a temp folder with a space in the name, and then writes a simple .bat file that forwards all args to a Dart script (similar to what flutter.bat does). The Dart script it writes just prints all the arguments out to stdout. The test reads stdout and compares what's printed to what it sent as arguments to ensure they match.

If I remove the & from testCharacters, the test passes. If I add the & then it fails because the last argument is truncated, and it tried to execute after:

'after\""' is not recognized as an internal or external command,
operable program or batch file.
Expected: ['beforeaafter', 'beforebafter', 'beforecafter', 'before&after']
  Actual: ['beforeaafter', 'beforebafter', 'beforecafter', '"before']

The _escapeAndQuoteArg function needs to escape & in some way, but I've tried many combinations (including backslashes, the ^ character and combinations of quoting/not quoting the args) (I'm assuming https://ss64.com/nt/syntax-esc.html is a reasonable source), but none of them work. Based on @derekxu16 comment at #50076 (comment) it's not clear to me if Dart is also trying to do some of this escaping.

I'm not sure if this is a bug, or I'm doing it wrong. I'm hoping someone that understands the code in createProcess may be able to verify one way or the other.

import 'dart:convert';
import 'dart:io';
import 'package:path/path.dart' as path;
import 'package:test/test.dart';

const testCharacters = r'abc&'; // r' "&:\<>|%=+-_@~#<>?/';

final testArgs = [
  for (var char in testCharacters.split('')) 'before${char}after',
];

void main() {
  test('test escaping', () async {
    var tempDir =
        Directory.systemTemp.createTempSync('flutter dap args escape test');
    print('Writing scripts to $tempDir');

    // Write a shell script to simulate the Flutter .bat file and a Dart script
    // that just prints the arguments sent to it one-per-line.
    var tempShellScript = File(path.join(tempDir.path, 'fake_flutter.bat'));
    var tempDartScript = File(path.join(tempDir.path, 'print_args.dart'));
    var command = [
      '"${Platform.resolvedExecutable}"',
      '"${tempDartScript.path}"',
      "%*",
    ];
    tempShellScript.writeAsStringSync('@echo off\n${command.join(" ")}');
    tempDartScript.writeAsStringSync(r'''
void main(List<String> args) {
  print(args.join('\n'));
}
''');

    var executable = tempShellScript.path;
    var args = testArgs.map(_escapeAndQuoteArg).toList();

    print('''
Executing:
  executable: $executable
  args: ${args.join(' ')}
  runInShell: true
''');
    var proc = await Process.start(
      '"$executable"',
      args,
    );

    var stdoutFuture = proc.stdout.transform(utf8.decoder).toList();
    var stderrFuture = proc.stderr.transform(utf8.decoder).toList();
    await proc.exitCode;

    var stdout = (await stdoutFuture).join().trim();
    var stderr = (await stderrFuture).join().trim();

    if (stderr.isNotEmpty) {
      print(stderr);
    }

    var actual = stdout
        .split('\n')
        .map((l) => l.trim())
        .where((l) => l.isNotEmpty)
        .toList();

    expect(actual, testArgs);
  });
}

String _escapeAndQuoteArg(String input) {
  // What is the correct thing to do here to escape &?
  // return input.replaceAll('&', '^&');
  return input;
}

Activity

  1. DanTup commented on Nov 25, 2024

    @DanTup
    CollaboratorAuthor

    @brianquinlan I'm told that you might be knowledgeable in this area :-)

  2. dart-github-bot commented on Nov 25, 2024

    @dart-github-bot
    Collaborator

    Summary: User cannot escape & in arguments passed to a Windows batch script via Process.start. The shell interprets & before Process.start can handle it, leading to argument truncation.

  3. added
    area-dart-cliUse area-dart-cli for issues related to the 'dart' command like tool.
    triage-automationSee https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot.
    type-bugIncorrect behavior (everything from a crash to more subtle misbehavior)
    on Nov 25, 2024
  4. mraleph commented on Nov 26, 2024

    @mraleph
    Member

    You can make this test pass by doing ^^^&. Explanation: you need to escape & once on the way to the first batch file and then you need to escape it once more because of how %* works... So if you know that you are going through flutter.bat/dart.bat and you know that they are going to forward arguments using %* then you can be prepared for that, otherwise I think you are out of luck.

    This is also a reason why flutter.bat/dart.bat approach is kinda sorta broken on Windows.

    See also this SO question for gnarly details.

  5. DanTup commented on Nov 26, 2024

    @DanTup
    CollaboratorAuthor

    and then you need to escape it once more because of how %* works

    Thank you! I actually wondered if it was related to %* but hadn't thought about it enough to realise this would be the fix.

    So there's no bug here, and we'll probably have to double-escape these for now then (generally we know we are going through Flutter... it might break if users configure a customTool but I think there's not much we can do if it ends with .bat but doesn't do the same).

    It would be nice to come up with a better solution (like compiling a non-.bat entry point for Flutter), but that's unlikely to be simple.

    (FYI @bkonyi)

  6. DanTup commented on Nov 26, 2024

    @DanTup
    CollaboratorAuthor

    @mraleph I'm not sure if I may have still found a bug.. interested in your opinion.

    Firstly, I decided that maybe it's better to escape inside flutter.bat where %* is used, rather than double-escape here? Presumably we already have bugs from the command line if it's not re-escaping the args correctly? (I will verify this if I thinkw e should go ahead with this).

    So I have an updated test script below which writes the escaping into the shell script (in reality this would go into flutter.bat):

    SET "args=%*"
    SETLOCAL EnableDelayedExpansion
    SET "args=!args:^=^^!"
    SET "args=!args:&=^&!"
    SET "args=!args:|=^|!"
    SET "args=!args:<=^<!"
    SET "args=!args:>=^>!"
    SET "args=!args:\=^\!"
    
    "D:\Tools\Dart\Stable\bin\dart.exe" "C:\Users\danny\AppData\Local\Temp\flutter dap args escape test67432968\print_args.dart" %args%
    

    It seems to work for every character currently in the escape set (the test passes).. however, if I add a double quote to the test characters, it fails because the output contains ^" where only " is expected - however none of this code (as far as I can tell) ever escapes double quotes, so I don't know where this is coming from (could it be Dart?).

    import 'dart:convert';
    import 'dart:io';
    import 'package:path/path.dart' as path;
    import 'package:test/test.dart';
    
    final charactersToEscape = r'^&|<>\';
    // Adding double quote " into the set of the test characters breaks, because
    // we get back ^" even though we don't escape a double quote anywhere.
    const testCharacters = r'abc ^&|<>\:%=+-_@~#<>?/';
    
    final testArgs = [
      for (var char in testCharacters.split('')) 'before${char}after',
    ];
    
    void main() {
      test('test escaping', () async {
        var tempDir =
            Directory.systemTemp.createTempSync('flutter dap args escape test');
        print('Writing scripts to $tempDir');
    
        // Write a shell script to simulate the Flutter .bat file and a Dart script
        // that just prints the arguments sent to it one-per-line.
        var tempShellScript = File(path.join(tempDir.path, 'fake_flutter.bat'));
        var tempDartScript = File(path.join(tempDir.path, 'print_args.dart'));
        var shellScriptContents = '''
    @echo off
    
    SET "args=%*"
    SETLOCAL EnableDelayedExpansion
    ${charactersToEscape.split('').map((c) => 'SET "args=!args:$c=^$c!"').join('\n')}
    
    "${Platform.resolvedExecutable}" "${tempDartScript.path}" %args%
    ''';
        tempShellScript.writeAsStringSync(shellScriptContents);
        tempDartScript.writeAsStringSync(r'''
    void main(List<String> args) {
      print(args.join('\n'));
    }
    ''');
    
        var executable = tempShellScript.path;
        var args = testArgs.map(_escapeAndQuoteArg).toList();
    
        print('''
    Executing:
      executable: $executable
      args: ${args.join(' ')}
      runInShell: true
    ''');
        var proc = await Process.start(
          '"$executable"',
          args,
        );
    
        var stdoutFuture = proc.stdout.transform(utf8.decoder).toList();
        var stderrFuture = proc.stderr.transform(utf8.decoder).toList();
        await proc.exitCode;
    
        var stdout = (await stdoutFuture).join().trim();
        var stderr = (await stderrFuture).join().trim();
    
        if (stderr.isNotEmpty) {
          print(stderr);
        }
    
        var actual = stdout
            .split('\n')
            .map((l) => l.trim())
            .where((l) => l.isNotEmpty)
            .toList();
    
        expect(actual, testArgs);
      });
    }
    
    String _escapeAndQuoteArg(String input) {
      final escapedChars = RegExp.escape(charactersToEscape);
      return input.replaceAllMapped(
          new RegExp('[$escapedChars]'), (m) => '^${m.group(0)}');
    }
  7. mraleph commented on Nov 26, 2024

    @mraleph
    Member
    SETLOCAL EnableDelayedExpansion
    

    FWIW some variations involving delayed expansion were tried before but it caused its own bunch of problems, see flutter/flutter#84270

    I am also not sure you can just really escape things reliably like that.

    It seems to work for every character currently in the escape set (the test passes).. however, if I add a double quote to the test characters, it fails because the output contains ^" where only " is expected - however none of this code (as far as I can tell) ever escapes double quotes, so I don't know where this is coming from (could it be Dart?).

    Yeah, there is some escaping in Process implementation on Windows which was put there in ancient times ff7d45d. This logic might easily be borked FWIW, I am not entirely sure why is it there specifically for quotes.

    As a result of that escaping you get \\" on entry to the batch file, and that somehow morphs into ^" when you do %args% at the last line. I am actually not sure how that happens. I have tried printing ECHO %args% but that yields "before^\\"after" and not before^"after... Rules for these substitutions seem to be seriously out of this world.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area-dart-cliUse area-dart-cli for issues related to the 'dart' command like tool.triage-automationSee https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot.type-bugIncorrect behavior (everything from a crash to more subtle misbehavior)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions