From 69dc6ab9cd4da86734b14b0c6b0b228f5e491057 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 4 May 2021 23:10:17 +0200 Subject: [PATCH 1/4] Add System.shell/2 --- lib/elixir/lib/system.ex | 69 ++++++++++++++++++++++++++++++----- lib/mix/lib/mix/shell.ex | 77 ++++++++++------------------------------ 2 files changed, 79 insertions(+), 67 deletions(-) diff --git a/lib/elixir/lib/system.ex b/lib/elixir/lib/system.ex index 12bd840cc2d..23f229f9158 100644 --- a/lib/elixir/lib/system.ex +++ b/lib/elixir/lib/system.ex @@ -880,6 +880,52 @@ defmodule System do :init.stop(String.to_charlist(status)) end + @doc ~S""" + Executes the given `command` in the current OS shell. + + **Important**: Use this function with care. In particular, **never + pass user input to this function**, as the user would be able to + execute any code directly on the machine. Generally speaking, + prefer to use `cmd/3` over this function. + + ## Examples + + iex> System.shell("echo hello") + {"hello\n", 0} + + ## Options + + It accepts the same options as `cmd/3`. + """ + @spec shell(binary, keyword) :: {Collectable.t(), exit_status :: non_neg_integer} + def shell(command, opts) when is_binary(command) do + assert_no_null_byte!(command, "System.shell/2") + + # Finding shell command logic from :os.cmd in OTP + # https://github.com/erlang/otp/blob/8deb96fb1d017307e22d2ab88968b9ef9f1b71d0/lib/kernel/src/os.erl#L184 + command = + case :os.type() do + {:unix, _} -> + command = + command + |> String.replace("\"", "\\\"") + |> String.to_charlist() + + 'sh -c "' ++ command ++ '"' + + {:win32, osname} -> + command = '"' ++ String.to_charlist(command) ++ '"' + + case {System.get_env("COMSPEC"), osname} do + {nil, :windows} -> 'command.com /s /c ' ++ command + {nil, _} -> 'cmd /s /c ' ++ command + {cmd, _} -> '#{cmd} /s /c ' ++ command + end + end + + do_cmd({:spawn, command}, [], opts) + end + @doc ~S""" Executes the given `command` with `args`. @@ -963,7 +1009,7 @@ defmodule System do ## Shell commands If you desire to execute a trusted command inside a shell, with pipes, - redirecting and so on, please check `:os.cmd/1`. + redirecting and so on, please check `shell/2`. """ @spec cmd(binary, [binary], keyword) :: {Collectable.t(), exit_status :: non_neg_integer} def cmd(command, args, opts \\ []) when is_binary(command) and is_list(args) do @@ -982,11 +1028,15 @@ defmodule System do :os.find_executable(cmd) || :erlang.error(:enoent, [command, args, opts]) end - {into, opts} = cmd_opts(opts, [:use_stdio, :exit_status, :binary, :hide, args: args], "") + do_cmd({:spawn_executable, cmd}, [args: args], opts) + end + + defp do_cmd(port_init, base_opts, opts) do + {into, opts} = cmd_opts(opts, [:use_stdio, :exit_status, :binary, :hide] ++ base_opts, "") {initial, fun} = Collectable.into(into) try do - do_cmd(Port.open({:spawn_executable, cmd}, opts), initial, fun) + do_port(Port.open(port_init, opts), initial, fun) catch kind, reason -> fun.(initial, :halt) @@ -996,17 +1046,18 @@ defmodule System do end end - defp do_cmd(port, acc, fun) do + defp do_port(port, acc, fun) do receive do {^port, {:data, data}} -> - do_cmd(port, fun.(acc, {:cont, data}), fun) + do_port(port, fun.(acc, {:cont, data}), fun) {^port, {:exit_status, status}} -> {acc, status} end end - defp cmd_opts([{:into, any} | t], opts, _into), do: cmd_opts(t, opts, any) + defp cmd_opts([{:into, any} | t], opts, _into), + do: cmd_opts(t, opts, any) defp cmd_opts([{:cd, bin} | t], opts, into) when is_binary(bin), do: cmd_opts(t, [{:cd, bin} | opts], into) @@ -1017,7 +1068,8 @@ defmodule System do defp cmd_opts([{:stderr_to_stdout, true} | t], opts, into), do: cmd_opts(t, [:stderr_to_stdout | opts], into) - defp cmd_opts([{:stderr_to_stdout, false} | t], opts, into), do: cmd_opts(t, opts, into) + defp cmd_opts([{:stderr_to_stdout, false} | t], opts, into), + do: cmd_opts(t, opts, into) defp cmd_opts([{:parallelism, bool} | t], opts, into) when is_boolean(bool), do: cmd_opts(t, [{:parallelism, bool} | opts], into) @@ -1028,7 +1080,8 @@ defmodule System do defp cmd_opts([{key, val} | _], _opts, _into), do: raise(ArgumentError, "invalid option #{inspect(key)} with value #{inspect(val)}") - defp cmd_opts([], opts, into), do: {into, opts} + defp cmd_opts([], opts, into), + do: {into, opts} defp validate_env(enum) do Enum.map(enum, fn diff --git a/lib/mix/lib/mix/shell.ex b/lib/mix/lib/mix/shell.ex index b8ffcf07eaa..164745d6639 100644 --- a/lib/mix/lib/mix/shell.ex +++ b/lib/mix/lib/mix/shell.ex @@ -3,6 +3,9 @@ defmodule Mix.Shell do Defines `Mix.Shell` contract. """ + @doc false + defstruct [:callback] + @doc """ Prints the given ANSI message to the shell. """ @@ -87,73 +90,29 @@ defmodule Mix.Shell do """ def cmd(command, options \\ [], callback) when is_function(callback, 1) do callback = - if Keyword.get(options, :quiet, false) do + if options[:quiet] do fn x -> x end else callback end - env = validate_env(Keyword.get(options, :env, [])) - - args = - if Keyword.get(options, :stderr_to_stdout, true) do - [:stderr_to_stdout] - else - [] - end + options = + options + |> Keyword.take([:cd, :stderr_to_stdout, :env]) + |> Keyword.put(:into, %Mix.Shell{callback: callback}) + |> Keyword.put_new(:stderr_to_stdout, true) - opts = - [:stream, :binary, :exit_status, :hide, :use_stdio, {:env, env}] ++ - args ++ Keyword.take(options, [:cd]) - - port = Port.open({:spawn, shell_command(command)}, opts) - port_read(port, callback) + {_, status} = System.shell(command, options) + status end - defp port_read(port, callback) do - receive do - {^port, {:data, data}} -> - _ = callback.(data) - port_read(port, callback) - - {^port, {:exit_status, status}} -> - status + defimpl Collectable do + def into(%Mix.Shell{callback: fun}) do + {:ok, + fn + _, {:cont, data} -> fun.(data) + _, _ -> :ok + end} end end - - # Finding shell command logic from :os.cmd in OTP - # https://github.com/erlang/otp/blob/8deb96fb1d017307e22d2ab88968b9ef9f1b71d0/lib/kernel/src/os.erl#L184 - defp shell_command(command) do - case :os.type() do - {:unix, _} -> - command = - command - |> String.replace("\"", "\\\"") - |> String.to_charlist() - - 'sh -c "' ++ command ++ '"' - - {:win32, osname} -> - command = '"' ++ String.to_charlist(command) ++ '"' - - case {System.get_env("COMSPEC"), osname} do - {nil, :windows} -> 'command.com /s /c ' ++ command - {nil, _} -> 'cmd /s /c ' ++ command - {cmd, _} -> '#{cmd} /s /c ' ++ command - end - end - end - - defp validate_env(enum) do - Enum.map(enum, fn - {k, nil} -> - {String.to_charlist(k), false} - - {k, v} -> - {String.to_charlist(k), String.to_charlist(v)} - - other -> - raise ArgumentError, "invalid environment key-value #{inspect(other)}" - end) - end end From d604efeb95d3d5211c4cc08aabca65639926fe11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 5 May 2021 08:09:34 +0200 Subject: [PATCH 2/4] Update system.ex --- lib/elixir/lib/system.ex | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/elixir/lib/system.ex b/lib/elixir/lib/system.ex index 23f229f9158..7f41a2dccea 100644 --- a/lib/elixir/lib/system.ex +++ b/lib/elixir/lib/system.ex @@ -881,12 +881,15 @@ defmodule System do end @doc ~S""" - Executes the given `command` in the current OS shell. + Executes the given `command` in the OS shell. + + It uses `sh` for Unix-like systems and `cmd` for Windows. **Important**: Use this function with care. In particular, **never - pass user input to this function**, as the user would be able to - execute any code directly on the machine. Generally speaking, - prefer to use `cmd/3` over this function. + pass untrusted user input to this function**, as the user would be + able to perform "command injection attacks" by executing any code + directly on the machine. Generally speaking, prefer to use `cmd/3` + over this function. ## Examples From 0676b82089dd8224091aa9ca6d03f9d48bf3f9fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 5 May 2021 09:06:36 +0200 Subject: [PATCH 3/4] Update lib/elixir/lib/system.ex --- lib/elixir/lib/system.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/lib/system.ex b/lib/elixir/lib/system.ex index 7f41a2dccea..0e68645297c 100644 --- a/lib/elixir/lib/system.ex +++ b/lib/elixir/lib/system.ex @@ -917,7 +917,7 @@ defmodule System do 'sh -c "' ++ command ++ '"' {:win32, osname} -> - command = '"' ++ String.to_charlist(command) ++ '"' + command = String.to_charlist(command) case {System.get_env("COMSPEC"), osname} do {nil, :windows} -> 'command.com /s /c ' ++ command From c49a22ab8486fc16a7912eb8d76ad85647cb0027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 5 May 2021 09:24:56 +0200 Subject: [PATCH 4/4] Shell tests --- lib/elixir/lib/system.ex | 4 +- lib/elixir/test/elixir/system_test.exs | 56 ++++++++++++++++++++------ 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/lib/elixir/lib/system.ex b/lib/elixir/lib/system.ex index 0e68645297c..200ea01dcaa 100644 --- a/lib/elixir/lib/system.ex +++ b/lib/elixir/lib/system.ex @@ -898,10 +898,10 @@ defmodule System do ## Options - It accepts the same options as `cmd/3`. + It accepts the same options as `cmd/3`, except for `arg0`. """ @spec shell(binary, keyword) :: {Collectable.t(), exit_status :: non_neg_integer} - def shell(command, opts) when is_binary(command) do + def shell(command, opts \\ []) when is_binary(command) do assert_no_null_byte!(command, "System.shell/2") # Finding shell command logic from :os.cmd in OTP diff --git a/lib/elixir/test/elixir/system_test.exs b/lib/elixir/test/elixir/system_test.exs index 8d591556f6b..d31a46fe92f 100644 --- a/lib/elixir/test/elixir/system_test.exs +++ b/lib/elixir/test/elixir/system_test.exs @@ -96,22 +96,21 @@ defmodule SystemTest do end test "cmd/3 (with options)" do - assert {["hello\r\n"], 0} = - System.cmd( - "cmd", - ~w[/c echo hello], - into: [], - cd: File.cwd!(), - env: %{"foo" => "bar", "baz" => nil}, - arg0: "echo", - stderr_to_stdout: true, - parallelism: true - ) + opts = [ + into: [], + cd: File.cwd!(), + env: %{"foo" => "bar", "baz" => nil}, + arg0: "echo", + stderr_to_stdout: true, + parallelism: true + ] + + assert {["hello\r\n"], 0} = System.cmd("cmd", ~w[/c echo hello], opts) end @echo "echo-elixir-test" @tag :tmp_dir - test "cmd/2 with absolute and relative paths", config do + test "cmd/3 with absolute and relative paths", config do echo = Path.join(config.tmp_dir, @echo) File.mkdir_p!(Path.dirname(echo)) File.cp!(System.find_executable("cmd"), echo) @@ -128,6 +127,22 @@ defmodule SystemTest do System.cmd(Path.join(File.cwd!(), @echo), ~w[/c echo hello], [{:arg0, "echo"}]) end) end + + test "shell/1" do + assert {"hello\r\n", 0} = System.shell("echo hello") + end + + test "shell/2 (with options)" do + opts = [ + into: [], + cd: File.cwd!(), + env: %{"foo" => "bar", "baz" => nil}, + stderr_to_stdout: true, + parallelism: true + ] + + assert {["bar\r\n"], 0} = System.shell("echo %foo%", opts) + end end describe "Unix" do @@ -152,7 +167,7 @@ defmodule SystemTest do @echo "echo-elixir-test" @tag :tmp_dir - test "cmd/2 with absolute and relative paths", config do + test "cmd/3 with absolute and relative paths", config do echo = Path.join(config.tmp_dir, @echo) File.mkdir_p!(Path.dirname(echo)) File.cp!(System.find_executable("echo"), echo) @@ -169,6 +184,21 @@ defmodule SystemTest do System.cmd(Path.join(File.cwd!(), @echo), ["hello"], [{:arg0, "echo"}]) end) end + + test "shell/1" do + assert {"hello\n", 0} = System.shell("echo hello") + end + + test "shell/2 (with options)" do + opts = [ + into: [], + cd: File.cwd!(), + env: %{"foo" => "bar", "baz" => nil}, + stderr_to_stdout: true + ] + + assert {["bar\n"], 0} = System.shell("echo $foo", opts) + end end @tag :unix