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

Command doesn't work as expected when there are double quotes in it #271

Open
ProgrammerAL opened this issue Sep 18, 2023 · 12 comments
Open
Labels
impact/usability Something that impacts users' ability to use the product easily and intuitively kind/enhancement Improvements or new features os/windows

Comments

@ProgrammerAL
Copy link

What happened?

Running a local command that uses double quotes has a side effect. With the code in the below sample, it ignores the Dir property.

I noticed another scenario where passing a command with double quotes in it (like wraping a path with a space in it), the double quotes are passed as a literal. And then the command would fail because it doesn't allow double quotes. I couldn't think of an easy example, but if you really need one I can work on creating one.

Example

Below is the code I used. Note that the CommandArgs.Dir property is hard coded to c:/temp. There are 2 lines that create the commandString variable. If you run the code with the first one, with the comment This line works as expected, it will create a new directory inside c:/temp. If you instead run the second one, with the comment This line doesn't work as expected, it will create a new directory, but it does it at the root path, at c:/. There is also nothing in the output to show that it was created in the wrong directory.

The only real difference between the 2 are what the command is. They are:
mkdir newdir_123
mkdir "newdir_123"

C# Code:

using System;
using System.Collections.Generic;
using Pulumi;

return await Deployment.RunAsync(() =>
{
    var number = new Random().Next(1, 100_000);

    //This line works as expected
    //var commandString = $"mkdir newdir_{number}";

    //This line doesn't work as expected
    var commandString = $"mkdir \"newdir_{number}\"";

    Log.Info($"Running command: {commandString}");

    var command = new Pulumi.Command.Local.Command("my-command", new Pulumi.Command.Local.CommandArgs
    {
        Create = commandString,
        Dir = "c:/temp",
    });

    _ = command.Stdout.Apply(x =>
    {
        Log.Info($"Stdout: {x}");
        return "";
    });

    _ = command.Stderr.Apply(x =>
    {
        Log.Info($"Stderr: {x}");
        return "";
    });

    return new Dictionary<string, object?>();
});

Csproj File:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Pulumi" Version="3.*" />
    <PackageReference Include="Pulumi.Command" Version="0.8.2" />
  </ItemGroup>

</Project>

Below is the output of running pulumi up with the first scenario, where it works as expected.

Please choose a stack, or create a new one: dev
Updating (dev)

View in Browser (Ctrl+O): https://app.pulumi.com/ProgrammerAl/pulumi-command-bug/dev/updates/25

     Type                      Name                    Status              Info
     pulumi:pulumi:Stack       pulumi-command-bug-dev                      3 messages
 ~   └─ command:local:Command  my-command              updated (0.20s)     [diff: ~create]

Diagnostics:
  pulumi:pulumi:Stack (pulumi-command-bug-dev):
    Running command: mkdir aa_newdir_74660
    Stdout:
    Stderr:

Resources:
    ~ 1 updated
    1 unchanged

Duration: 3s

Below is the output of running pulumi up with the second scenario, where it creates the directory in the c:/ directory. Other than the command string, the outputs are the same.

Please choose a stack, or create a new one: dev
Updating (dev)

View in Browser (Ctrl+O): https://app.pulumi.com/ProgrammerAl/pulumi-command-bug/dev/updates/26

     Type                      Name                    Status              Info
     pulumi:pulumi:Stack       pulumi-command-bug-dev                      3 messages
 ~   └─ command:local:Command  my-command              updated (0.31s)     [diff: ~create]

Diagnostics:
  pulumi:pulumi:Stack (pulumi-command-bug-dev):
    Running command: mkdir "aa_newdir_8786"
    Stdout:
    Stderr:

Resources:
    ~ 1 updated
    1 unchanged

Duration: 3s

Output of pulumi about

C:..pulumi-command-bug> pulumi about
running 'dotnet build -nologo .'
Determining projects to restore...

C:\Program Files\dotnet\sdk\7.0.401\NuGet.targets(158,5): warning : No Authorization header detected [C:\temp\pulumi-command-bug\pulumi-command-bug.sln]

All projects are up-to-date for restore.

C:\temp\pulumi-command-bug\pulumi-command-bug.csproj : warning Undefined: No Authorization header detected

pulumi-command-bug -> C:\temp\pulumi-command-bug\bin\Debug\net6.0\pulumi-command-bug.dll

Build succeeded.

C:\Program Files\dotnet\sdk\7.0.401\NuGet.targets(158,5): warning : No Authorization header detected [C:\temp\pulumi-command-bug\pulumi-command-bug.sln]
C:\temp\pulumi-command-bug\pulumi-command-bug.csproj : warning Undefined: No Authorization header detected
2 Warning(s)
0 Error(s)

Time Elapsed 00:00:01.83

'dotnet build -nologo .' completed successfully
CLI
Version 3.83.0
Go Version go1.21.1
Go Compiler gc

Plugins
NAME VERSION
command 0.8.2
dotnet unknown

Host
OS Microsoft Windows 11 Home
Version 10.0.22621 Build 22621
Arch x86_64

This project is written in dotnet: executable='C:\Program Files\dotnet\dotnet.exe' version='7.0.401'

Backend
Name pulumi.com
URL https://app.pulumi.com/ProgrammerAl
User ProgrammerAl
Organizations ProgrammerAl

Dependencies:
NAME VERSION
Pulumi 3.56.2
Pulumi.Command 0.8.2

Pulumi locates its logs in C:\Users\Progr\AppData\Local\Temp by default
warning: Failed to get information about the current stack: No current stack

Additional context

I only ran this on Windows. I do not know what happens when this is run on Linux or Mac OS.

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@ProgrammerAL ProgrammerAL added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Sep 18, 2023
@mikhailshilkov
Copy link
Member

Hi @ProgrammerAL thank you for opening this issue. I tried reproducing this issue on my Mac (changed the dir to ./temp) but both variants seem to work as expected... You mentioned you may have some other repro?

@iwahbe Any thoughts here off the top of your head?

@ProgrammerAL
Copy link
Author

Thanks @mikhailshilkov
The other example I had was when using Cloudflare's Wrangler CLI. I wanted to think of another example that doesn't need you to have Wrangler installed and configured. The problem I saw there was that Wrangler was erroring because it received the double quotes I was using to enclose file paths.

I believe I'm able to reproduce that error with the echo command. The difference between the two outputs are in the Stdout field. You can use the same code above, just change out the line that sets the commandString variable.

Command: var commandString = $"echo aa_newdir_{number}";
Output:

C:..pulumi-command-bug> pulumi up -f
Please choose a stack, or create a new one: dev
Updating (dev)

View in Browser (Ctrl+O): https://app.pulumi.com/ProgrammerAl/pulumi-command-bug/dev/updates/28

     Type                      Name                    Status              Info
     pulumi:pulumi:Stack       pulumi-command-bug-dev                      3 messages
 ~   └─ command:local:Command  my-command              updated (0.23s)     [diff: ~create]

Diagnostics:
  pulumi:pulumi:Stack (pulumi-command-bug-dev):
    Running command: echo aa_newdir_5694
    Stderr:
    Stdout: aa_newdir_5694

Resources:
    ~ 1 updated
    1 unchanged

Duration: 3s

Command" var commandString = $"echo \"aa_newdir_{number}\"";
Output:

C:..pulumi-command-bug> pulumi up -f
Please choose a stack, or create a new one: dev
Updating (dev)

View in Browser (Ctrl+O): https://app.pulumi.com/ProgrammerAl/pulumi-command-bug/dev/updates/29

     Type                      Name                    Status              Info
     pulumi:pulumi:Stack       pulumi-command-bug-dev                      3 messages
 ~   └─ command:local:Command  my-command              updated (0.32s)     [diff: ~create]

Diagnostics:
  pulumi:pulumi:Stack (pulumi-command-bug-dev):
    Running command: echo "aa_newdir_90087"
    Stdout: \"aa_newdir_90087\"
    Stderr:

Resources:
    ~ 1 updated
    1 unchanged

Duration: 3s

Again, I only ran this on Windows 11.

@iwahbe
Copy link
Member

iwahbe commented Sep 20, 2023

Nothing jumps out to me. This is one of the few providers where we would expect platform specific behavior, so my guess is that cmd /C responds differently then /bin/sh -c in terms of automatic unquoting. My suspicion is that we will need a windows computer to repro.

@iwahbe
Copy link
Member

iwahbe commented Sep 20, 2023

@ProgrammerAL If you have access to a UNIX interpreter on your machine, do you still get the same result when you specify Interpreter to be ["/bin/sh", "-c"]?

@ProgrammerAL
Copy link
Author

Think I found the reason for this when running on Windows. And it depends on which terminal is used to run the command.

Take these two commands as examples when run manually:
cmd /C echo aaa
cmd /C echo "aaa"

I use Windows Terminal which I have setup to use PowerShell by default. And with that I'm getting the same output of aaa for both commands. I had done this before making this GitHub Issue, which is why I assumed the problem was inside the Pulumi Provider. But just now I tried it from within the Windows Command Prompt, and that got different results. One with, and the other without, the quotes.

So the summarize, I guess this is the intended behavior on Windows. This will cause problems to anyone running on Windows, and I don't have a workaround in mind other than to run in PowerShell by default...which I think is installed on all supported Windows machines now. No idea if that's a good idea or not.

Side note, @iwahbe I don't have a Unix interpreter on my machine. If you still want, I can install one and try it, but I don't think it matters anymore.

@iwahbe
Copy link
Member

iwahbe commented Sep 20, 2023

I believe that the command provider just runs (on windows) ["cmd", "/C", yourInput]:

var args []string
if in.Interpreter != nil && len(*in.Interpreter) > 0 {
args = append(args, *in.Interpreter...)
} else {
if runtime.GOOS == "windows" {
args = []string{"cmd", "/C"}
} else {
args = []string{"/bin/sh", "-c"}
}
}

@ProgrammerAL I think the experiment you already performed satisfies my curiosity. I don't think you need to install a Unix shell.

@mikhailshilkov
Copy link
Member

@ProgrammerAL @iwahbe Thank you for diagnosing this issue further! It sounds like this is mostly "by design" but can be confusing for Windows users. Do you have suggestions for improvements on the command provider to help with that?

@mikhailshilkov mikhailshilkov added kind/enhancement Improvements or new features impact/usability Something that impacts users' ability to use the product easily and intuitively os/windows and removed kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Sep 21, 2023
@ProgrammerAL
Copy link
Author

@iwahbe @mikhailshilkov Since I was still having issues with the Cloudflare Wrangler CLI I spent some more time messing around with this and it looks like the problem is in how the arguments are received by the console app. However I don't know if the fault is on the Pulumi provider or Windows, I'm still experimenting.

I created a new C# console application to log the arguments when Pulumi call it.

//Launch debugger to view args values when run from Pulumi, remove this to just let the app run to end
Debugger.Launch();

foreach (var arg in args)
{
    Console.WriteLine(arg);
}

The command I ran from Pulumi is: C:/temp/Practice/Practice/bin/Debug/net7.0/Practice.exe -aaa -bbb b -ccc "cc cc cc cc". The exe path is just to the executable created after compiling the C# code above, so if you want to reproduce this, feel free to change that path.

When running the command manually through the Windows Command Prompt, the C# app receives 5 arguments as expected:

-aaa
-bbb
b
-ccc
cc cc cc cc

But when run through Pulumi, the app received 8 arguments because the last one is split up based on spaces, and also includes the double quotes:

-aaa
-bbb
b
-ccc
"cc
cc
cc
cc"

Honestly I don't have any ideas what to do. This scenario is not expected behavior because it's different from what happens when run manually. But I haven't been able find any kind of workaround.

Do you think it's possible there's something in how Go runs apps external processes that could be affecting this? I have no Go experience, so not sure where to even start looking at this.

@iwahbe
Copy link
Member

iwahbe commented Sep 21, 2023

@ProgrammerAL Can you try running cmd /C "C:/temp/Practice/Practice/bin/Debug/net7.0/Practice.exe -aaa -bbb b -ccc \"cc cc cc cc\""? That is much closer to what Pulumi.Command actually executes.

It's possible that Go is doing something weird when forking into the windows process. It's also possible that cmd parses CLI arguments differently than it parses interactive input.


I agree that our desired behavior is matching what users see everywhere else.

@ProgrammerAL
Copy link
Author

ProgrammerAL commented Sep 21, 2023

@iwahbe Ah good catch, I forgot to use the cmd /C at the beginning. I tried running the command from a Windows Command Prompt a few different ways. Below are the results

Version 1: cmd /C "C:/temp/Practice/Practice/bin/Debug/net7.0/Practice.exe -aaa -bbb b -ccc \"cc cc cc cc\""

-aaa
-bbb
b
-ccc
"cc
cc
cc
cc"

Version 2: cmd /C "C:/temp/Practice/Practice/bin/Debug/net7.0/Practice.exe -aaa -bbb b -ccc "cc cc cc cc""

-aaa
-bbb
b
-ccc
cc cc cc cc

Version 3: cmd /C C:/temp/Practice/Practice/bin/Debug/net7.0/Practice.exe -aaa -bbb b -ccc "cc cc cc cc"

-aaa
-bbb
b
-ccc
cc cc cc cc

Version 4: cmd /C "C:/temp/Practice/Practice/bin/Debug/net7.0/Practice.exe -aaa -bbb b -ccc """cc cc cc cc""""

-aaa
-bbb
b
-ccc
"cc cc cc cc"

Version 5: cmd /C C:/temp/Practice/Practice/bin/Debug/net7.0/Practice.exe -aaa -bbb b -ccc ""cc cc cc cc""

-aaa
-bbb
b
-ccc
cc
cc
cc
cc

It seems that versions 2 and 3 work the way I'd expect it to. So that's promising.

@iwahbe
Copy link
Member

iwahbe commented Sep 21, 2023

Thanks @ProgrammerAL! That is super helpful!

It sounds like the best response is to disambiguate the cmd process on windows so it always does the same thing (sounds like we should call powershell directly). This will fix the reliability concern when running the same command from different environments.

I don't want to end up re-implementing argument splitting for windows interpreters.


I have no idea what semantics Version 2 is using. It seems like it's nesting the double quotes. On a sh terminal, Version 2 does this:

𝛌 python3 -c "import sys; print(sys.argv)" "C:/temp/Practice/Practice/bin/Debug/net7.0/Practice.exe -aaa -bbb b -ccc "cc cc cc cc""
['-c', 'C:/temp/Practice/Practice/bin/Debug/net7.0/Practice.exe -aaa -bbb b -ccc cc', 'cc', 'cc', 'cc']

CC @Frassle @AaronFriel as Windows users.

@Frassle
Copy link
Member

Frassle commented Sep 21, 2023

From https://pkg.go.dev/os/exec#Command

On Windows, processes receive the whole command line as a single string and do their own parsing. Command combines and quotes Args into a command line string with an algorithm compatible with applications using CommandLineToArgvW (which is the most common way). Notable exceptions are msiexec.exe and cmd.exe (and thus, all batch files), which have a different unquoting algorithm.

Also https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/cmd has some info on how to correctly use /c with quotes.

But it's tricky getting this right in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/usability Something that impacts users' ability to use the product easily and intuitively kind/enhancement Improvements or new features os/windows
Projects
None yet
Development

No branches or pull requests

4 participants