pulumi-command icon indicating copy to clipboard operation
pulumi-command copied to clipboard

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

Open ProgrammerAL opened this issue 2 years ago • 12 comments

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 avatar Sep 18 '23 02:09 ProgrammerAL

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?

mikhailshilkov avatar Sep 20 '23 11:09 mikhailshilkov

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.

ProgrammerAL avatar Sep 20 '23 13:09 ProgrammerAL

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 avatar Sep 20 '23 16:09 iwahbe

@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"]?

iwahbe avatar Sep 20 '23 16:09 iwahbe

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.

ProgrammerAL avatar Sep 20 '23 17:09 ProgrammerAL

I believe that the command provider just runs (on windows) ["cmd", "/C", yourInput]: https://github.com/pulumi/pulumi-command/blob/18d96268096693afbecb077d23e67a1f99c205fd/provider/pkg/provider/local/commandOutputs.go#L39-L48

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

iwahbe avatar Sep 20 '23 18:09 iwahbe

@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 avatar Sep 21 '23 09:09 mikhailshilkov

@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.

ProgrammerAL avatar Sep 21 '23 14:09 ProgrammerAL

@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.

iwahbe avatar Sep 21 '23 15:09 iwahbe

@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.

ProgrammerAL avatar Sep 21 '23 17:09 ProgrammerAL

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.

iwahbe avatar Sep 21 '23 18:09 iwahbe

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.

Frassle avatar Sep 21 '23 19:09 Frassle