crystal icon indicating copy to clipboard operation
crystal copied to clipboard

Command prompt escaping in `Process.quote_windows`

Open HertzDevil opened this issue 1 year ago • 2 comments

From #13567 we know that Process.quote_windows, which only performs the opposite of CommandLineToArgvW, does not handle metacharacter escaping for the command prompt. We also need both kinds of escaping, and there isn't a one-size-fits-all solution. So I am proposing the following addition to the standard library:

class Process
  def self.quote_windows(args : Enumerable(String), *, c_runtime : Bool = true, shell : Bool = false)
    # if `c_runtime` is true, do what the method does right now; then
    # if `shell` is true, prepend a `^` to every metacharacter
    # (`(`, `)`, `%`, `!`, `^`, `"`, `<`, `>`, `&`, `|`), and also
    # raise if any `\n` appears
  end

  def self.quote_windows(args : String, *, c_runtime : Bool = true, shell : Bool = false)
    quote_windows({args}, c_runtime: c_runtime, shell: shell)
  end
end

Process.quote_windows(%q(foo <"bar">), c_runtime: false)              # => %q(foo <"bar">)
Process.quote_windows(%q(foo <"bar">), c_runtime: false, shell: true) # => %q(foo ^<^"bar^"^>)
Process.quote_windows(%q(foo <"bar">))                                # => %q("foo <\"bar\">")
Process.quote_windows(%q(foo <"bar">), shell: true)                   # => %q(^"foo ^<\^"bar\^"^>^")

Process.quote is not supposed to be used in platform-specific scenarios, so it doesn't have to expose those new parameters.

This still leaves some questions unanswered:

  • Should we continue to expect that the backtick is portable?
  • Should we continue to expect that Process.quote inside a backtick is portable?
  • Do these parameters' defaults for .quote_windows also hold for .quote?

HertzDevil avatar Feb 16 '24 17:02 HertzDevil

And the big question that I keep insisting on...

  • Should we assume that the "cmd" shell is the shell and deserves to be tied to a boolean shell: true

It might be cool to instead expand Process.run to accept an enum as the shell argument, which would then have implementations for sh, cmd, windows backed by corresponding quote implementations.

As for backticks, I don't have much hope of them ever doing anything sensible...

oprypin avatar Feb 17 '24 10:02 oprypin

Sometimes I've been thinking what the shell would be for a hypothetical ~~MinGW-based toolchain~~ MSYS2-based build environment. Crystal programs built with it can definitely expect the availability of sh.exe as a default POSIX shell. So maybe replacing shell: true with an enum isn't a bad idea after all...?

HertzDevil avatar Feb 17 '24 11:02 HertzDevil