From e61e6e6fd9676ce3ff3971851755cdb7cf63dbf1 Mon Sep 17 00:00:00 2001
From: wm4 <wm4@nowhere>
Date: Fri, 29 Nov 2013 23:36:44 +0100
Subject: command: change the syntax and semantics of the "run" command

See the changes in input.rst for explanations.

Technically speaking, this also gets rid of some undefined behavior:
passing NULL as a vararg (execl()) is always a bug.
---
 DOCS/man/en/input.rst    | 21 ++++++++++++++++++---
 mpvcore/input/input.c    | 18 ++++++++++++++----
 mpvcore/player/command.c |  8 ++++++--
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/DOCS/man/en/input.rst b/DOCS/man/en/input.rst
index 917584428f..0e05be2745 100644
--- a/DOCS/man/en/input.rst
+++ b/DOCS/man/en/input.rst
@@ -198,9 +198,24 @@ List of Input Commands
     because index2 refers to the target entry, not the index the entry
     will have after moving.)
 
-``run "<command>"``
-    Run the given command with ``/bin/sh -c``. The string is expanded like in
-    `Property Expansion`_.
+``run "command" "arg1" "arg2" ...``
+    Run the given command. Unlike in MPlayer/mplayer2 and earlier versions of
+    mpv (0.2.x and older), this doesn't call the shell. Instead, the command
+    is run directly, with each argument passed separately. Each argument is
+    expanded like in `Property Expansion`_. Note that there is a static limit
+    of (as of this writing) 10 arguments (this limit could be raised on demand).
+
+    To get the old behavior, use ``/bin/sh`` and ``-c`` as the first two
+    arguments.
+
+    .. admonition:: Example
+
+        ``run "/bin/sh" "-c" "echo ${title} > /tmp/playing"``
+
+        This is not a particularly good example, because it doesn't handle
+        escaping, and a specially prepared file might allow an attacker to
+        execute arbitrary shell commands. It is recommended to write a small
+        shell script, and call that with ``run``.
 
 ``quit [<code>]``
     Exit the player using the given exit code.
diff --git a/mpvcore/input/input.c b/mpvcore/input/input.c
index 658f82f764..5d096fcd58 100644
--- a/mpvcore/input/input.c
+++ b/mpvcore/input/input.c
@@ -129,6 +129,7 @@ struct mp_cmd_def {
     const char *name;   // user-visible name (as used in input.conf)
     const struct m_option args[MP_CMD_MAX_ARGS];
     bool allow_auto_repeat; // react to repeated key events
+    bool vararg;        // last argument can be given 0 to multiple times
 };
 
 static const struct mp_cmd_def mp_cmds[] = {
@@ -214,7 +215,7 @@ static const struct mp_cmd_def mp_cmds[] = {
       ARG_CHOICE_OR_INT(0, INT_MAX, ({"current", -1})),
   }},
   { MP_CMD_PLAYLIST_MOVE, "playlist_move", { ARG_INT, ARG_INT } },
-  { MP_CMD_RUN, "run", { ARG_STRING } },
+  { MP_CMD_RUN, "run", { ARG_STRING, ARG_STRING }, .vararg = true },
 
   { MP_CMD_SET, "set", { ARG_STRING,  ARG_STRING } },
   { MP_CMD_GET_PROPERTY, "get_property", { ARG_STRING } },
@@ -989,6 +990,10 @@ static int parse_cmd(struct input_ctx *ictx, struct mp_cmd **dest, bstr str,
 
     for (int i = 0; i < MP_CMD_MAX_ARGS; i++) {
         const struct m_option *opt = &cmd_def->args[i];
+        bool is_vararg = cmd_def->vararg &&
+            (i + 1 >= MP_CMD_MAX_ARGS || !cmd_def->args[i + 1].type); // last arg
+        if (!opt->type && is_vararg && cmd->nargs > 0)
+            opt = cmd->args[cmd->nargs - 1].type;
         if (!opt->type)
             break;
 
@@ -1007,9 +1012,14 @@ static int parse_cmd(struct input_ctx *ictx, struct mp_cmd **dest, bstr str,
             }
         } else {
             bool got_token = read_token(str, &str, &arg);
-            // Explicitly select default for an optional argument
-            if (got_token && opt->defval && bstr_equals0(arg, "-"))
-                got_token = false;
+            if (is_vararg) {
+                if (!got_token)
+                    continue;
+            } else {
+                // Explicitly select default for an optional argument
+                if (got_token && opt->defval && bstr_equals0(arg, "-"))
+                    got_token = false;
+            }
             // Skip optional arguments
             if (!got_token && opt->defval) {
                 struct mp_cmd_arg *cmdarg = &cmd->args[cmd->nargs];
diff --git a/mpvcore/player/command.c b/mpvcore/player/command.c
index f6aa429b99..3bd618da8e 100644
--- a/mpvcore/player/command.c
+++ b/mpvcore/player/command.c
@@ -2942,14 +2942,18 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
         screenshot_to_file(mpctx, cmd->args[0].v.s, cmd->args[1].v.i, msg_osd);
         break;
 
-    case MP_CMD_RUN:
+    case MP_CMD_RUN: {
 #ifndef __MINGW32__
+        char *args[MP_CMD_MAX_ARGS + 1] = {0};
+        for (int n = 0; n < cmd->nargs; n++)
+            args[n] = cmd->args[n].v.s;
         if (!fork()) {
-            execl("/bin/sh", "sh", "-c", cmd->args[0].v.s, NULL);
+            execvp(args[0], args);
             exit(0);
         }
 #endif
         break;
+    }
 
     case MP_CMD_ENABLE_INPUT_SECTION:
         mp_input_enable_section(mpctx->input, cmd->args[0].v.s,
-- 
cgit v1.2.3