Skip to content

Parameter invalid when it contains a colon. Works prior to 7.3.0. #18554

Closed

Description

@rmegal

Prerequisites

Steps to reproduce

Enter:

$test = "&sqlcmd -v MDF_FILEPATH=```"C:\Databases\junk.mdf```""
invoke-expression $test

Expected behavior

sqlcmd is launched and you can enter:

select '$(MDF_FILEPATH)'
go

And be presented with:

----------------------------
C:\Databases\junk.mdf

Actual behavior

sqlcmd is no longer able to parse parameter value and you get the following after the invoke-expression:

Sqlcmd: 'MDF_FILEPATH=\"C:\Databases\junk.mdf\""': Invalid argument. Enter '-?' for help.

Error details

n/a

Environment data

Name                           Value
----                           -----
PSVersion                      7.3.0
PSEdition                      Core
GitCommitId                    7.3.0
OS                             Microsoft Windows 10.0.22621
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visuals

PowerShell_7 3

PowerShell_prior_to_7 3

Activity

  1. SteveL-MSFT commented on Nov 14, 2022

    @SteveL-MSFT
    Member

    Use:

     $PSNativeCommandArgumentPassing = 'Legacy'

    for now as a workaround. We probably need to add sqlcmd to the legacy list. cc James Truher (@JamesWTruher)

  2. SteveL-MSFT commented on Nov 14, 2022

    @SteveL-MSFT
    Member

    Ray Megal (@rmegal) if you can confirm using Legacy mode works for you, I'll update the list to include sqlcmd

  3. rmegal commented on Nov 14, 2022

    @rmegal
    Author

    Using $PSNativeCommandArgumentPassing = 'Legacy' works for sqlcmd.

    I will use Legacy for the scripts I have that use sqlcmd directly. Will I need to keep doing this once sqlcmd is in the legacy list? Not sure how the "legacy list" works.

  4. SteveL-MSFT commented on Nov 14, 2022

    @SteveL-MSFT
    Member

    Once we have the list updated and backported to 7.3.x, the idea is that you can keep $PSNativeCommandArgumentPassing = 'Windows' (which is the default) and commands in the list automatically use legacy when needed for compat. So you shouldn't have to make any changes. However, if you uset legacy at the top of your script, then the changes we're making should still not break you and you can keep it like that as well.

  5. ghost added
    In-PRIndicates that a PR is out for the issue
    on Nov 14, 2022
  6. JamesWTruher commented on Nov 14, 2022

    @JamesWTruher
    Collaborator

    it is also likely that the triple back-tick is no longer needed (a single might do the trick with this new behavior)

  7. mklement0 commented on Nov 14, 2022

    @mklement0
    Contributor

    Ray Megal (@rmegal), note that there's generally no reason to use Invoke-Expression for invoking external programs.

    The problem boils down to the following call, which is a workaround that builds on the fundamentally broken handling of embedded " chars. in external-program calls up to v7.2 - that's why it stopped working in v7.3:

    & sqlcmd -v MDF_FILEPATH=`"C:\Databases\junk.mdf`"

    In your particular case, given that value C:\Databases\junk.mdf contains no spaces (or other metacharacters), no workaround is actually needed:

    & sqlcmd -v MDF_FILEPATH=C:\Databases\junk.mdf

    However, if the value part - to the right of = - indeed needs selective quoting in sqlcmd (I don't know), a workaround is indeed needed for values that do contain spaces or other metacharacters.

    A workaround is then needed, because - behind the scenes, justifiably - PowerShell passes something like
    MDF_FILEPATH="C:\My Databases\junk.mdf" as "MDF_FILEPATH=C:\My Databases\junk.mdf", i.e. double-quoted as a whole. External programs should treat these forms as interchangeable, but, regrettably, owing to the anarchy that is argument-passing on Window, some do not.

    However, a workaround like MDF_FILEPATH=`"C:\Databases\junk.mdf`" should never have worked, and only works if the broken legacy behavior is in place - hence the need for $PSNativeCommandArgumentPassing = 'Legacy'

    A built-in, program-agnostic workaround that would make (non-workaround) calls simply work as-is going forward, was proposed in #15143, but, unfortunately, not implemented.

    The upshot is:

    • An eternal game of catch-up will be played of putting specific external programs on the "legacy list" that the default $PSNativeCommandArgumentPassing value of Windows then implicitly treats as if $PSNativeCommandArgumentPassing = 'Legacy' were in effect, which guarantees confusion as to which specific PowerShell version has which external programs on that list and whether a workaround is needed or not.

    • Given that the legacy-mode workaround builds on broken legacy behavior, future generations of PowerShell users will be saddled with having to know the specifics of this broken behavior.

  8. rmegal commented on Nov 15, 2022

    @rmegal
    Author

    Ray Megal (@rmegal), note that there's generally no reason to use Invoke-Expression for invoking external programs.

    Michael Klement (@mklement0) Right. I could have reproduced the problem without using Invoke-Expresstion. The original script uses it because the command is dynamically built; there are a bunch of different parameters that might get used.

    The -v parameter accepts [var = value]. I only included the one that was failing. You are also correct that the quotes are not needed when there are no spaces in the value.

    A workaround is then needed, because - behind the scenes, justifiably - PowerShell passes something like MDF_FILEPATH="C:\My Databases\junk.mdf" as "MDF_FILEPATH=C:\My Databases\junk.mdf", i.e. double-quoted as a whole. External programs should treat these forms as interchangeable, but, regrettably, owing to the anarchy that is argument-passing on Window, some do not.

    Thanks for the nice explanation. I remember building that script years ago and wondering why including the back ticks made it work. Regardless, your point is, if I'm understanding correctly, sqlcmd will need the 'Legacy' support unless it is changed to accept "MDF_FILEPATH=C:\My Databases\junk.mdf"?

    I tried entering sqlcmd -v MDF_FILEPATH=c:\test\junk.mdf and got:
    image

    Is there something I'm missing regarding passing a colon in the value? Note: This is the same behavior in the cmd shell. I have to wrap the value in quotes to get it to work there too. I'm sure the problem lies in how sqlcmd parses the value containing a colon, which forces the user to wrap the value in quotes.

  9. jborean93 commented on Nov 15, 2022

    @jborean93
    Collaborator

    The original script uses it because the command is dynamically built; there are a bunch of different parameters that might get used.

    You can still do that without Invoke-Expression

    $sqlCmdArgs = @(
        'arg that is always set'
        if ($someCondition) { 'args for sqlcml' }
        'arg that is always set'
    )
    sqlcmd @sqlCmdArgs 

    You can also splat multiple arrays or use args inline with splats so you aren't restricted to just this. Using Invoke-Expression is dangerous if the data comes from a user source and adds yet another layer you need to deal with when it comes to strings. It is best to try and avoid it in as many cases as you can.

    Is there something I'm missing regarding passing a colon in the value? Note: This is the same behavior in the cmd shell. I have to wrap the value in quotes to get it to work there too. I'm sure the problem lies in how sqlcmd parses the value containing a colon, which forces the user to wrap the value in quotes.

    You need to quote the argument to avoid pwsh parameter binder from reading it differently

    sqlcmd -v 'MDF_FILEPATH=c:\test\junk.mdf'
  10. mklement0 commented on Nov 15, 2022

    @mklement0
    Contributor

    Jordan Borean (@jborean93) , good point about avoiding Invoke-Expression, but quoting MDF_FILEPATH=c:\test\junk.mdf doesn't make a difference, because the problem is on the sqlcmd side.

    Ray Megal (@rmegal):

    if I'm understanding correctly, sqlcmd will need the 'Legacy' support unless it is changed to accept "MDF_FILEPATH=C:\My Databases\junk.mdf"?

    In principle, yes, but there seems to be another wrinkle:

    PowerShell - again, justifiably - never uses double quotes behind the scenes if an argument doesn't contain spaces, even if in the original PowerShell command line quotes were used; that is, 'MDF_FILEPATH=c:\test\junk.mdf' "MDF_FILEPATH=c:\test\junk.mdf" and MDF_FILEPATH=c:\test\junk.mdf all end up as unquoted MDF_FILEPATH=c:\test\junk.mdf on the process command line.

    It seems that sqlcmd.exe (which I'm not familiar with) breaks an unqoted value that contains : by that character, which is what your error message suggests (the docs don't seem to mention that).

    If that is the case, then truly only partial quoting helps; that is, the following must be placed on the process command line: MDF_FILEPATH="c:\test\junk.mdf", which you can only do in one of the following ways:

    • with your original legacy workaround that builds on the broken behavior, which now requires you to (temporarily) set $PSNativeCommandArgumentPassing to 'Legacy'

    • with --%, which comes with pitfalls and fundamental limitations, however

      • sqlcmd -v --% MDF_FILEPATH="c:\test\junk.mdf"
    • by calling via cmd /c

      • cmd /c 'sqlcmd -v MDF_FILEPATH="c:\test\junk.mdf"'

    P.S.: I've written up a generalization of the above with additional background information in this Stack Overflow answer (albeit without discussing the specific sqlcmd challenges).

  11. ghost removed
    In-PRIndicates that a PR is out for the issue
    on Nov 15, 2022
  12. 6 remaining items

  13. mklement0 commented on Nov 16, 2022

    @mklement0
    Contributor

    $test = "&sqlcmd -v MDF_FILEPATH=`"C:\Databases\junk.mdf`""; `invoke-expression $test

    can be reduced to &sqlcmd -v MDF_FILEPATH="C:\Databases\junk.mdf", which - due to not containing spaces - is placed as unqouoted, verbatim MDF_FILEPATH=C:\Databases\junk.mdf on the process command line.

    Since there are no embedded " characters to pass, v7.2- and v7.3 behave the same.

    As Ray Megal (@rmegal) reports, such an unquoted token appears not to work with sqlcmd, because the unquoted : appears to have special meaning to sqlcmd.

    The workaround is to ensure double-quoting, selectively around the value part, so that verbatim MDF_FILEPATH="C:\Databases\junk.mdf" is placed on the process command line.

    • In v7.3, with (effective) Standard behavior, this cannot be done - except with the --% and cmd /c workarounds previously discussed.

    • In v7.2, or in v7.3 with Legacy in effect, you can build a workaround on the old, broken behavior, which is what the command in the OP does:

    & { $PSNativeCommandArgumentPassing = 'Legacy'; sqlcmd -v MDF_FILEPATH=`"C:\Databases\junk.mdf`" }
  14. daxian-dbw commented on Nov 16, 2022

    @daxian-dbw
    Member

    In v7.2, or in v7.3 with Legacy in effect, you can build a workaround on the old, broken behavior, which is what the command in the OP does:

    & { $PSNativeCommandArgumentPassing = 'Legacy'; sqlcmd -v MDF_FILEPATH=`"C:\Databases\junk.mdf`" }
    

    This is the part that I'm confused with. See my screenshots below (running in 7.2.7, which essentially uses the Legacy mode):

    image

    I replaced sqlcmd with testexe.exe -echoargs to print out the passed in arguments. The results show no difference in having `" or " surrounding C:\Databases\junk.mdf. Both have verbatim MDF_FILEPATH=C:\Databases\junk.mdf on the process command line. Did I do something wrong?

  15. mklement0 commented on Nov 16, 2022

    @mklement0
    Contributor

    You need to look at the raw command line to see the difference, which testexe -echoargs doesn't show.

    Allow me to use some canned advice on how to get a utility that does:

    • If you use Chocolatey, you can use choco install echoargs -y from an elevated session to install echoargs.exe, which shows the raw command line that PowerShell builds behind the scenes, and how (most) external programs parse it into arguments. Alternatively, you can compile a utility ad hoc: see this answer.

    The difference is that your first command places MDF_FILEPATH="C:\Databases\junk.mdf" on the process command line (note the double quotes), whereas it is MDF_FILEPATH=C:\Databases\junk.mdf for the second command.

    To an executable that uses the runtime-provided Microsoft C/C++/.NET argument parsing of the process command line, as testexe -echoargs does, these two arguments result in the same verbatim argument, namely MDF_FILEPATH=C:\Databases\junk.mdf - that is, the (unescaped) " characters are assumed to have syntactic function and are removed.

    However, here we're talking about an executable, sqlcmd.exe, that seemingly defines its own rules and treats those raw arguments differently - that is the curse of the anarchy that is argument-passing on Windows, which - rather than passing an array of verbatim tokens, as is the case on Unix - forces each executable to act as its own quasi-shell and parse its arguments from a single string that encodes all arguments.

    To an executable that uses the runtime-provided Microsoft C/C++/.NET argument parsing, all of the following are equivalent, but to sqlcmd and other high-profile CLIs such as msiexec and msdeploy, they aren't:

    • MDF_FILEPATH=C:\Databases\junk.md
    • MDF_FILEPATH="C:\Databases\junk.md"
    • "MDF_FILEPATH=C:\Databases\junk.md"

    The related executable-agnostic accommodation I proposed as part of #15143 focuses on values (the part of the argument to the right of =) that require double-quoting, due to containing spaces, so that PowerShell would place whatever PowerShell syntax variation results in verbatim value foo=bar baz to pass as selectively double-quoted foo="bar baz" on the process command line.

    However, it seems that sqlcmd elevates the anarchy to new heights, by also distinguishing between a space-less argument that is placed on the process command line as foo=c:\bar vs. foo="c:\bar"

  16. daxian-dbw commented on Nov 16, 2022

    @daxian-dbw
    Member

    However, here we're talking about an executable, sqlcmd.exe, that seemingly defines its own rules and treats those raw arguments differently ...
    To an executable that uses the runtime-provided Microsoft C/C++/.NET argument parsing, all of the following are equivalent, but to sqlcmd and other high-profile CLIs such as msiexec and msdeploy, they aren't:

    Michael Klement (@mklement0) This makes perfect sense to me now, thank you!
    So, it makes sense to have sqlcmd in the legacy command list, and we are all good.

  17. daxian-dbw commented on Nov 16, 2022

    @daxian-dbw
    Member

    Michael Klement (@mklement0) One more question regarding Environment.CommandLine.

    When using the utility compiled from this answer, the Environment.CommandLine shows raw: ["E:\yard\tmp\echoArgsPause.exe" -v MDF_FILEPATH="C:\Databases\junk.mdf"]

    image

    However, when using the testexe.exe -echoargs after updating it to write out CommandLine, it shows different value: raw: [testexe.dll -echoargs -v MDF_FILEPATH=C:\Databases\junk.mdf]

    image

    Both were on PS 7.2.7.
    Do you know why there is such a difference between echoArgsPause.exe (built in PowerShell.exe, so targeting .NET Framework) and testexe.exe -echoargs (built targeting .NET 7)? Does Environment.CommandLine not reflect the RAW process argument anymore?

  18. mklement0 commented on Nov 16, 2022

    @mklement0
    Contributor

    Indeed, a .NET (Core) application (as opposed to a .NET Framework application) does not report the true raw command line: see dotnet/runtime#11305 (comment)

  19. rmegal commented on Nov 17, 2022

    @rmegal
    Author

    Dongbo Wang (@daxian-dbw) and Michael Klement (@mklement0) This was an eye opening conversation. Thanks a ton for weighing in.

  20. jborean93 commented on Nov 17, 2022

    @jborean93
    Collaborator

    Dongbo Wang (@daxian-dbw) if it helps this is what I use to test out argv stuff on Windows

    Add-Type -OutputType ConsoleApplication -OutputAssembly print_argv.exe -TypeDefinition @'
    using System;
    using System.Collections.Generic;
    using System.Runtime.InteropServices;
    
    namespace PrintArgv
    {
        class Program
        {
            [DllImport("Kernel32.dll")]
            public static extern IntPtr GetCommandLineW();
    
            static void Main(string[] args)
            {
                IntPtr cmdLinePtr = GetCommandLineW();
                string cmdLine = Marshal.PtrToStringUni(cmdLinePtr);
    
                Console.WriteLine(cmdLine);
                for (int i = 0; i < args.Length; i++)
                {
                    Console.WriteLine("[{0}] {1}", i, args[i]);
                }
            }
        }
    }
    '@

    This unfortunately needs to run in WinPS but it can be run anywhere. It will output the raw command line value as well as the arguments as parsed by dotnet

    image

    Note I haven't updated to 7.3.0 yet

    For Linux I've also used the following before

    #include<stdio.h>
    
    int main(int argc, char *argv[])
    {
        int i;
        for(i = 1;i < argc;i++)
        {
            printf("[%d] %s\n", i, argv[i]);
        }
        return 0;
    }

    You can compile it with gcc print_argv.c -o print_argv

    image

    It is a bit more confusing there because Linux doesn't have a command line string but uses char *argv[].

  21. daxian-dbw commented on Nov 17, 2022

    @daxian-dbw
    Member

    Jordan Borean (@jborean93) Thank you for sharing the code!
    I submitted #18591 to update testext with a -echoraw switch, to show the raw command line and the individual args.
    I think the existing testexe -echoraw is sufficient on Unix platforms, but let me know if that's not the case.

    image

  22. jborean93 commented on Nov 17, 2022

    @jborean93
    Collaborator

    Looks good, I have no idea what dotnet does for Environment.CommandLine outside of Windows, I'm not as familiar to process args there.

  23. mklement0 commented on Nov 17, 2022

    @mklement0
    Contributor

    I think the existing testexe -echoraw is sufficient on Unix platforms, but let me know if that's not the case.

    I assume you meant testexe -echoargs and, yes, that would be sufficient on Unix, given that there is no such thing as a raw command line there (only an array of verbatim arguments passed via the relevant system functions).

    However, is there really a need for a new, separate option?

    Can't the existing -echoargs option:

    • either: just also show the raw command line, on Windows only?

    • or - with an appropriate hint - on Unix show a hypothetical command line that would work from a POSIX-compatible shell?

      • However, note that [Environment]::CommandLine does not provide such a hypothetical command line robustly, because it doesn't \-escape $-prefixed tokens, neither in unquoted arguments nor in arguments with spaces, which it invariably encloses in "...", which are interpolating strings in POSIX-compatible shells.
      • For instance, a test.ps1 file that contains [Environment]::CommandLine invoked from Bash with
        pwsh -file test.ps1 'There is no place like $HOME' (note that $HOME is not expanded, due to use of '...') is echoed as follows, which does not roundtrip properly, because the hypothetical reconstruction results in an interpolating POSIX-shell string (that the pwsh executable is reported as a .dll is a separate problem - see Environment.CommandLine for published  dotnet/runtime#11305):
        • /usr/bin/pwsh.dll -nop -file test.ps1 "There is no place like $HOME"
  24. mklement0 commented on Nov 17, 2022

    @mklement0
    Contributor

    Dongbo Wang (@daxian-dbw)

    This makes perfect sense to me now, thank you!

    Glad to hear it.

    So, it makes sense to have sqlcmd in the legacy command list, and we are all good.

    I know I'm repeating myself, but I feel it bears repeating: such a legacy command list is a terrible idea; while it may selectively, obscurely help backward compatibility, it guarantees everlasting confusion going forward.

  25. daxian-dbw commented on Nov 17, 2022

    @daxian-dbw
    Member

    Michael Klement (@mklement0) testexe -echoargs is used in tests, and the tests only expect the individual arguments, not extra texts for the raw command line. The -echoraw flag is only for interactive use in cases like this one.

  26. mklement0 commented on Nov 17, 2022

    @mklement0
    Contributor

    Thanks for clarifying, Dongbo Wang (@daxian-dbw).
    On second thought re -echoraw, showing a hypothetical command line on Unix is probably not called for in a diagnostic utility, given that it would basically arbitrarily have to pick a shell whose syntax to conform to (even though there are only two logical candidates, POSIX-compatible shells and PowerShell) when reverse-engineering a (presumed) shell command line that the program itself never saw (and whose faithful reconstruction would be impossible).

  27. Scordo commented on Nov 24, 2022

    @Scordo

    Hi,

    we are facing the same problem with msbuild and property values containing a semicolon or comma.
    We have to support powershell 5.1 up to pwsh 7.3 and are using escaped quotes which are breaking in pwsh 7.3.

    We dont want to set $PSNativeCommandArgumentPassing script-wide because it may cause incompatibilities when including 3rd party modules/scripts which dont expect this.

    So we decided to go with local changes in scripts with the help of the following function:

    function Use-LegacyNativeCommandArgumentPassing {
      [CmdletBinding()]
      param (
        [Parameter(Mandatory=$true)]
        [scriptblock]
        $Script
      )
    
      if (!(test-path variable:PSNativeCommandArgumentPassing)) {
        & $Script
        return
      }
    
      $oldPSNativeCommandArgumentPassing = $PSNativeCommandArgumentPassing
    
      try {
        $PSNativeCommandArgumentPassing = 'Legacy'
        & $Script
      }
      finally {
        $PSNativeCommandArgumentPassing = $oldPSNativeCommandArgumentPassing
      }
    }
    

    Which we are using this way:

    Use-LegacyNativeCommandArgumentPassing {
      & 'msbuild.exe' @('test.target', "/p:Category=`"CI,Nightly`"")
    }
    

    Hope this helps someone else.

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

Metadata

Metadata

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions