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

Process.parse_arguments for Windows #12193

Closed
HertzDevil opened this issue Jul 4, 2022 · 1 comment · Fixed by #12278
Closed

Process.parse_arguments for Windows #12193

HertzDevil opened this issue Jul 4, 2022 · 1 comment · Fixed by #12278
Labels
kind:feature platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:system

Comments

@HertzDevil
Copy link
Contributor

HertzDevil commented Jul 4, 2022

Process.parse_arguments uses the POSIX parsing rules, so it can fail to parse Windows command-line arguments (that use Microsoft's C runtime, which is virtually everything). A common example is unquoted arguments containing backslashes, the preferred path separator on Windows:

lib LibC
  fun CommandLineToArgvW(lpCmdLine : LPWSTR, pNumArgs : Int*) : LPWSTR*
  fun LocalFree(hMem : Void*) : Void*
end

class Process
  def self.parse_arguments_native(line : String) : Array(String)
    # `CommandLineToArgvW` always takes a program name, possibly empty,
    # and the program name doesn't obey the same escaping rules, so
    # we have to insert a space before *line* and then ignore the empty program name
    arg_ptrs = LibC.CommandLineToArgvW((' ' + line).to_utf16, out arg_count)
    raise RuntimeError.from_winerror("CommandLineToArgvW") unless arg_ptrs

    Array.new(arg_count - 1) do |i|
      str, _ = String.from_utf16(arg_ptrs[i + 1])
      str
    end
  ensure
    LibC.LocalFree(arg_ptrs) if arg_ptrs
  end
end

Process.parse_arguments %q(/LIBPATH:C:\crystal\lib)        # => ["/LIBPATH:C:crystallib"]
Process.parse_arguments_native %q(/LIBPATH:C:\crystal\lib) # => ["/LIBPATH:C:\\crystal\\lib"]

This was found while trying to get the interpreter compiling on Windows, where the compiler builds the list of linker arguments for passing to a Crystal::Loader. It is not possible to split the arguments beforehand, because we need to do this again anyway when there is any backtick expansion.

The standard library should provide a portable implementation of the above method. I also suggest renaming the method so that it matches .quote's API:

class Process
  # this naming suggests it is the inverse of `.quote`
  def self.unquote(line : String) : Array(String)
    {% if flag?(:win32) %}
      unquote_windows(args)
    {% else %}
      unquote_posix(args)
    {% end %}
  end

  def self.unquote_posix(line : String) : Array(String); end

  # equivalent to `CommandLineToArgvW`
  # note that *line* refers to the one for functions like `CreateProcess`,
  # _not_ the CMD shell arguments, nor PowerShell for that matter
  def self.unquote_windows(line : String) : Array(String); end

  @[Deprecated("Use `.unquote_posix` instead")]
  def self.parse_arguments(line : String) : Array(String); end
end

Deprecation of the existing method is likely, because directly putting unquote's body in it would cause a breaking change for Windows.

@HertzDevil HertzDevil added kind:feature topic:stdlib:system platform:windows Windows support based on the MSVC toolchain / Win32 API labels Jul 4, 2022
@straight-shoota
Copy link
Member

straight-shoota commented Jul 4, 2022

I like parse_arguments better than unquote. This method does not only remove quotes (which could just return a single string), but it also parses the individual arguments.

Breaking changes on Windows should be fine because it's still not fully supported yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants