Add System.shell/2 (#10965)

Author: josevalim

lib/elixir/lib/system.ex

@@ -881,6 +881,55 @@ defmodule System do
     :init.stop(String.to_charlist(status))
   end
 
+  @doc ~S"""
+  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 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
+
+      iex> System.shell("echo hello")
+      {"hello\n", 0}
+
+  ## Options
+
+  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
+    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`.
 
@@ -964,7 +1013,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
@@ -983,11 +1032,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)
@@ -997,17 +1050,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)
@@ -1018,7 +1072,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)
@@ -1029,7 +1084,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

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

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