diff --git a/spec/std/process_spec.cr b/spec/std/process_spec.cr index b786f7e89c94..edbfee68fefd 100644 --- a/spec/std/process_spec.cr +++ b/spec/std/process_spec.cr @@ -63,6 +63,10 @@ private def newline {% end %} end +private def to_ary(tuple) + [tuple[0]].concat(tuple[1]) +end + # interpreted code doesn't receive SIGCHLD for `#wait` to work (#12241) {% if flag?(:interpreted) && !flag?(:win32) %} pending Process @@ -70,7 +74,78 @@ end {% end %} describe Process do - describe ".new" do + describe ".new (args)" do + it "raises if args is empty" do + expect_raises(File::NotFoundError, "Error executing process: No command") do + Process.new([] of String) + end + end + + it "raises if args[0] is empty" do + expect_raises(IO::Error, /Error executing process: '("")?'/) do + Process.new([""] of String) + end + end + + it "raises if command doesn't exist" do + expect_raises(File::NotFoundError, "Error executing process: 'foobarbaz'") do + Process.new(["foobarbaz"]) + end + end + + it "raises for long path" do + expect_raises(File::NotFoundError, "Error executing process: 'aaaaaaa") do + Process.new(["a" * 1000]) + end + end + + it "accepts nilable string for `chdir` (#13767)" do + expect_raises(File::NotFoundError, "Error executing process: 'foobarbaz'") do + Process.new(["foobarbaz"], chdir: nil.as(String?)) + end + end + + it "raises if command is not executable" do + with_tempfile("crystal-spec-run") do |path| + File.touch path + expect_raises({% if flag?(:win32) %} File::BadExecutableError {% else %} File::AccessDeniedError {% end %}, "Error executing process: '#{path.inspect_unquoted}'") do + Process.new([path]) + end + end + end + + it "raises if command is not executable" do + with_tempfile("crystal-spec-run") do |path| + Dir.mkdir path + expect_raises(File::AccessDeniedError, "Error executing process: '#{path.inspect_unquoted}'") do + Process.new([path]) + end + end + end + + it "raises if command could not be executed" do + with_tempfile("crystal-spec-run") do |path| + File.touch path + command = File.join(path, "foo") + expect_raises(IO::Error, "Error executing process: '#{command.inspect_unquoted}'") do + Process.new([command]) + end + end + end + + it "doesn't break if process is collected before completion", tags: %w[slow] do + 200.times { Process.new(to_ary(exit_code_command(0))) } + + # run the GC multiple times to unmap as much memory as possible + 10.times { GC.collect } + + # the processes above have now been queued after completion; if this last + # one finishes at all, nothing was broken by the GC + Process.run(*exit_code_command(0)) + end + end + + describe ".new (command + args)" do it "raises if command doesn't exist" do expect_raises(File::NotFoundError, "Error executing process: 'foobarbaz'") do Process.new("foobarbaz") diff --git a/src/crystal/system/process.cr b/src/crystal/system/process.cr index e917c6663777..46ca5cbf02c7 100644 --- a/src/crystal/system/process.cr +++ b/src/crystal/system/process.cr @@ -73,8 +73,11 @@ struct Crystal::System::Process # def self.fork : ProcessInformation # def self.fork(&) + # def prepare_args(command : String, args : Enumerable(String)?, shell : Bool) : Args + # def prepare_args(args : Enumerable(String)) : Args + # Launches a child process with the command + args. - # def self.spawn(command : String, args : Enumerable(String)?, shell : Bool, env : Env?, clear_env : Bool, input : Stdio, output : Stdio, error : Stdio, chdir : Path | String?) : ProcessInformation + # def self.spawn(prepared_args : Args, shell : Bool, env : Env?, clear_env : Bool, input : Stdio, output : Stdio, error : Stdio, chdir : Path | String?) : ProcessInformation # Replaces the current process with a new one. # def self.replace(command : String, args : Enumerable(String)?, shell : Bool, env : Env?, clear_env : Bool, input : Stdio, output : Stdio, error : Stdio, chdir : Path | String?) : NoReturn diff --git a/src/crystal/system/unix/process.cr b/src/crystal/system/unix/process.cr index d464704605b1..2de805da7a00 100644 --- a/src/crystal/system/unix/process.cr +++ b/src/crystal/system/unix/process.cr @@ -292,23 +292,25 @@ struct Crystal::System::Process def self.prepare_args(command : String, args : Enumerable(String)?, shell : Bool) : {String, LibC::Char**} if shell command = %(#{command} "${@}") unless command.includes?(' ') - argv_ary = ["/bin/sh", "-c", command, "sh"] + command_args = ["/bin/sh", "-c", command, "sh"] if args unless command.includes?(%("${@}")) raise ArgumentError.new(%(Can't specify arguments in both command and args without including "${@}" into your command)) end end - - pathname = "/bin/sh" else - argv_ary = [command] - pathname = command + command_args = [command] end - argv_ary.concat(args) if args + command_args.concat(args) if args + + prepare_args(command_args) + end - argv = argv_ary.map(&.check_no_null_byte.to_unsafe) + def self.prepare_args(args : Enumerable(String)) : {String, LibC::Char**} + pathname = args.first + argv = args.map(&.check_no_null_byte.to_unsafe) {pathname, argv.to_unsafe} end diff --git a/src/crystal/system/unix/spawn.cr b/src/crystal/system/unix/spawn.cr index ccd6c6a07a81..ef4f4b9069f7 100644 --- a/src/crystal/system/unix/spawn.cr +++ b/src/crystal/system/unix/spawn.cr @@ -2,9 +2,7 @@ require "c/signal" require "c/unistd" struct Crystal::System::Process - def self.spawn(command, args, shell, env, clear_env, input, output, error, chdir) - prepared_args = prepare_args(command, args, shell) - + def self.spawn(prepared_args, shell, env, clear_env, input, output, error, chdir) r, w = FileDescriptor.system_pipe envp = Env.make_envp(env, clear_env) diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index 15d1c2b0c9f5..de1e44930c0f 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -286,7 +286,7 @@ struct Crystal::System::Process {new_handle, dup_handle} end - def self.spawn(command, args, shell, env, clear_env, input, output, error, chdir) + def self.spawn(prepared_args, shell, env, clear_env, input, output, error, chdir) startup_info = LibC::STARTUPINFOW.new startup_info.cb = sizeof(LibC::STARTUPINFOW) startup_info.dwFlags = LibC::STARTF_USESTDHANDLES @@ -297,7 +297,6 @@ struct Crystal::System::Process process_info = LibC::PROCESS_INFORMATION.new - prepared_args = prepare_args(command, args, shell) prepared_args = ::Process.quote_windows(prepared_args) unless prepared_args.is_a?(String) if LibC.CreateProcessW( @@ -331,20 +330,25 @@ struct Crystal::System::Process end command else - # Disable implicit execution of batch files (https://github.com/crystal-lang/crystal/issues/14536) - # - # > `CreateProcessW()` implicitly spawns `cmd.exe` when executing batch files (`.bat`, `.cmd`, etc.), even if the application didn’t specify them in the command line. - # > The problem is that the `cmd.exe` has complicated parsing rules for the command arguments, and programming language runtimes fail to escape the command arguments properly. - # > Because of this, it’s possible to inject commands if someone can control the part of command arguments of the batch file. - # https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/ - if command.rstrip(". ").byte_slice?(-4, 4).try(&.downcase).in?(".bat", ".cmd") - raise ::File::Error.from_os_error("Error executing process", WinError::ERROR_BAD_EXE_FORMAT, file: command) - end + command_args = [command] + command_args.concat(args) if args + prepare_args(command_args) + end + end - prepared_args = [command] - prepared_args.concat(args) if args - prepared_args + def self.prepare_args(args : Enumerable(String)) + # Disable implicit execution of batch files (https://github.com/crystal-lang/crystal/issues/14536) + # + # > `CreateProcessW()` implicitly spawns `cmd.exe` when executing batch files (`.bat`, `.cmd`, etc.), even if the application didn’t specify them in the command line. + # > The problem is that the `cmd.exe` has complicated parsing rules for the command arguments, and programming language runtimes fail to escape the command arguments properly. + # > Because of this, it’s possible to inject commands if someone can control the part of command arguments of the batch file. + # https://flatt.tech/research/posts/batbadbut-you-cant-securely-execute-commands-on-windows/ + command = args.first + if command.rstrip(". ").byte_slice?(-4, 4).try(&.downcase).in?(".bat", ".cmd") + raise ::File::Error.from_os_error("Error executing process", WinError::ERROR_BAD_EXE_FORMAT, file: command) end + + args end private def self.try_replace(command, prepared_args, env, clear_env, input, output, error, chdir) diff --git a/src/process.cr b/src/process.cr index b743167333f6..2e67e5605fb9 100644 --- a/src/process.cr +++ b/src/process.cr @@ -286,6 +286,67 @@ class Process @process_info : Crystal::System::Process @wait_count = 0 + # Creates and executes a child process. + # + # This starts a new process for the command given in *args[0]*. + # + # The command is either a path to the executable to run, or the name of an + # executable which is then looked up by the operating system. + # The lookup uses the `PATH` variable of the current process environment + # (i.e. `ENV["PATH"]). + # In order to resolve to a specific executable, provide a path instead of + # only a command name. `Process.find_executable` can help with looking up a + # command in a custom `PATH`. + # + # The following arguments in *args* are passed as arguments to the child process. + # + # Raises `IO::Error` if executing *args[0]* fails, for example because the + # executable doesn't exist or is not executable. + # + # *env* provides a mapping of environment variables for the child process. + # If *clear_env* is `true`, only these explicit variables are used; if `false`, + # the child inherits the parent's environment with *env* merged. + # + # *input*, *output*, *error* configure the child process's standard streams. + # * `Redirect::Close` passes the null device + # * `Redirect::Pipe` creates a pipe that's accessible via `#input`, `#output` + # or `#error`. + # * `Redirect::Inherit` to share the parent's streams (`STDIN`, `STDOUT`, `STDERR`). + # * An `IO` instance creates a pipe that reads/writes into the given IO. + # + # *chdir* changes the working directory of the child process. If `nil`, uses + # the current working directory of the parent process. + # + # Example: + # + # ``` + # process = Process.new(["echo", "Hello"], output: Process::Redirect::Pipe) + # process.output.gets_to_end # => "Hello\n" + # process.wait # => Process::Status[0] + # ``` + # + # Similar methods: + # + # * `Process.run` is a convenient short cut if you just want to run a command + # and wait for it to finish. + # * `Process.exec` replaces the current process. + def initialize(args : Enumerable(String), *, env : Env = nil, clear_env : Bool = false, + input : Stdio = Redirect::Close, output : Stdio = Redirect::Close, error : Stdio = Redirect::Close, chdir : Path | String? = nil) + raise File::NotFoundError.new("Error executing process: No command", file: "") if args.empty? + + fork_input = stdio_to_fd(input, for: STDIN) + fork_output = stdio_to_fd(output, for: STDOUT) + fork_error = stdio_to_fd(error, for: STDERR) + + prepared_args = Crystal::System::Process.prepare_args(args) + pid = Crystal::System::Process.spawn(prepared_args, false, env, clear_env, fork_input, fork_output, fork_error, chdir.try &.to_s) + @process_info = Crystal::System::Process.new(pid) + + fork_input.close unless fork_input.in?(input, STDIN) + fork_output.close unless fork_output.in?(output, STDOUT) + fork_error.close unless fork_error.in?(error, STDERR) + end + # Creates and executes a child process. # # This starts a new process for `command`. @@ -359,7 +420,8 @@ class Process fork_output = stdio_to_fd(output, for: STDOUT) fork_error = stdio_to_fd(error, for: STDERR) - pid = Crystal::System::Process.spawn(command, args, shell, env, clear_env, fork_input, fork_output, fork_error, chdir.try &.to_s) + prepared_args = Crystal::System::Process.prepare_args(command, args, shell) + pid = Crystal::System::Process.spawn(prepared_args, shell, env, clear_env, fork_input, fork_output, fork_error, chdir.try &.to_s) @process_info = Crystal::System::Process.new(pid) fork_input.close unless fork_input.in?(input, STDIN) @@ -436,10 +498,12 @@ class Process fork_io end - # :nodoc: - def initialize(pid : LibC::PidT) - @process_info = Crystal::System::Process.new(pid) - end + {% unless flag?(:interpreted) %} + # :nodoc: + def initialize(pid : LibC::PidT) + @process_info = Crystal::System::Process.new(pid) + end + {% end %} # Sends *signal* to this process. #