Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-standard command line parsing misinterprets semicolons inside arguments #13264

Open
michkot opened this issue Jun 10, 2022 · 5 comments
Open
Labels
Area-Commandline wt.exe's commandline arguments Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. this-will-be-a-breaking-change
Milestone

Comments

@michkot
Copy link

michkot commented Jun 10, 2022

Windows Terminal version

1.13.11431.0

Windows build number

10.0.19044.1706

Other Software

MSYS2 / Git for Windows bash (but really anything that regularly uses semicolons in their arguments, including filepaths)

Steps to reproduce

Relates to

#11314 (comment)
#11314 (comment)
I hope that given @lhecker already expressed his doubts about the current behaviour, there would be some motivation for a fix-by-breaking-change here.

Use your favorite way to create a process on windows, I used

using System;

namespace CreateProcess
{
   class Program
   {
      static void Main(string[] args)
      {

         while (true)
         {
            var line = System.Console.ReadLine();
            try
            {
               var command = line.Substring(0, line.IndexOf(" "));
               var args2 = line.Substring(line.IndexOf(" ") + 1);
               System.Diagnostics.Process.Start(command, args2);
            } catch (Exception e) { }
         }
      }
   }
}

start the following command line
wt.exe sh -c 'echo abcd\n && read -p prompt'
wt:
image
sh:
image

all works fine:
image

now change && for ; -> start:
wt sh -c 'echo abcd\n ; read -p prompt'

wt:
image
sh:
will fail
image

Now, I am able to take that somebody decided that semicolon ; as an argument is a good tab splitting character, however obviously this is problematic in context of standard *nix shells.

However, it is worse. WT ignores the "good citizens" rules about how command line is supposed to be parsed on windows (I really enjoyed reading this one again after long time).
https://daviddeley.com/autohotkey/parameters/parameters.htm

WT interprets semicolon ; anywhere, in any argument, as command to spawn new tab, even if it is in a middle of "what should be parsed as a single argument" on windows!

repro steps:
command line: wt.exe sh -c "'echo abcd\n ; read -p prompt'"

wt:
image

the argument gets torn apart by the semicolon and the argument fails to start
image
image

there should be just one tab, and the command line to start sh should like sh -c "'echo abcd\n ; read -p prompt'" (does not matter if sh can handle that well or not, see note bellow too)

also:
command line: wt.exe sh -c 'echo abcd\n; read -p prompt' should work too, because the ; is not an isolated argument, but part of the argument abcd\n; So the command line passed used to start sh should be simply sh -c 'echo abcd\n; read -p prompt'.

note: sh is parsing the command line in a shell-way, that's why the argument with inner command line, passed after the -c argument, can be enclosed in single quotes '; However that is just a feature of this particular reproduction steps and does not affect the "core issue" in WT.

Expected Behavior

WT does not treat ; that are part of an command line argument (https://daviddeley.com/autohotkey/parameters/parameters.htm#WINCRULESCHANGE ,referencing MSDN docs) that contains other characters then the single ; (assuming spaces/tabs around arguments are trimmed by the command line argument parsing code) as create new tab commands.

Actual Behavior

WT treats all ; in all arguments as create new tab commands. As a consequence it is miss-interpreting arguments designed to reach "target CLI application".
Such behaviour does not have any grounds in standard windows command line parsing procedures and leads to confusion and need for special workarounds, that would need to dynamically rewrite arguments being passed to WT.

@michkot michkot added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Jun 10, 2022
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 10, 2022
@DHowett
Copy link
Member

DHowett commented Jun 10, 2022

This seems very similar to #13255. We should consolidate them into one issue (prefer this one as it is more on-target.)

@zadjii-msft
Copy link
Member

I agree.

Finding a way to split this up might be tricky.

  • What about something like wt -p "foo;bar"? That one seems like it would be looking up the foo;bar profile.
  • But then something like wt -p foo;nt - what should that do?
  • Or even just wt nt;nt;nt? I would think that should still get broken into subcommands, even though they'd all get parsed as one arg, nt;nt;nt

@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. Product-Terminal The new Windows Terminal. Area-Commandline wt.exe's commandline arguments this-will-be-a-breaking-change labels Jun 10, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jun 10, 2022
@zadjii-msft zadjii-msft added this to the Backlog milestone Jun 10, 2022
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jun 10, 2022
@lhecker
Copy link
Member

lhecker commented Jun 10, 2022

In my personal opinion we should have as little surprising behavior as possible here.
For instance if I'm scripting and I write wt -p $profileName in PowerShell I don't expect that I have to escape semicolons in the string first. I think that's a problem. 🤔

As such I'd say wt -p foo;nt looks for a profile called foo;nt and wt -p foo ; nt does the alternative behavior with new-tab spawning (assuming the shell doesn't parse semicolons). I don't think we can use quotes alone as a differentiator to decide whether semicolons are part of the string or not. After all, we don't exactly know whether wt -p $profileName always sends the variable quoted and will continue to do so in the future.

At this time I can think of two approaches:

  • A strict one: ; only acts as a seperator if it's a standalone argument in the argv array
  • A lax one: ; also acts as a seperator if it appears outside of quotes, while quotes are present (e.g. "foo";bar, foo;"bar", or "foo";"bar", but not foo;bar) - this one is still a bit "surprising"

@michkot
Copy link
Author

michkot commented Jun 10, 2022 via email

@michkot
Copy link
Author

michkot commented Jun 16, 2022

I just I though about this again. I realized that forcing "windows standard CLI parsing" might actually not rly improve life of people who regulary invoke complicated bash CLIs through WT - the need to wrap the whole inner command line into " quotes would clash with the " quotes used within the inner command line.

However I can imagine that people like me (which struggles with this ;-mess in automation scenarios) would be made happier with a "workaround" - an alternative WT command line parsing modes being added.

It would end-up having (maybe) 3 cli parsing modes
"current one"
"standard windows" parsing mode, wt -cw command-a first-param second;param third-param ; command-b "first ; param" second-param
"raw string mode", ala raw string literals in C++/python" wt -cr MAGICRAW MAGICRAW command-a and its command ; line that does not end until I write MAGIC and RAW as a single word MAGICRAW ; command-b;command-c

... there could be alternations to this; The thing is, I am not that familiar with WT's CLI switches; Must all switches (like -p xxx come before all commands and command-separators? Or can they be interleaved? How do we know if it's a swtich to WT or parrt of the inner command line to start?
Based on these thing, the "raw string mode" might be a great thing or a great overkill ; Maybe it would be enough to allow to specify custom command-separator character instead of the default";" 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Commandline wt.exe's commandline arguments Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. this-will-be-a-breaking-change
Projects
None yet
Development

No branches or pull requests

4 participants