[PATCH v2] rebase -x: sanity check command

[Phillip Wood <phillip.wood@talktalk.net>]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

If the user gives an empty argument to --exec then git creates a todo
list that it cannot parse. The rebase starts to run before erroring out
with

  error: missing arguments for exec
  error: invalid line 2: exec
  You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'.
  Or you can abort the rebase with 'git rebase --abort'.

Instead check for empty commands before starting the rebase.

Also check that the command does not contain any newlines as the
todo-list format is unable to cope with multiline commands. Note that
this changes the behavior, before this change one could do

git rebase --exec='echo one
exec echo two'

and it would insert two exec lines in the todo list, now it will error
out.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---

Apologies for the resend but I messed up the from address on the
previous send and I don't think it made it to the mailing list.

Thanks for the comments. I've updated the code as suggested by Dscho
and tried to make it clearer in the commit message that this is fixing
git creating a todo-list that it cannot parse.

Range-diff against v1:
1:  9626549aec ! 1:  726b84c300 rebase -x: sanity check command
    @@ -2,8 +2,9 @@
     
         rebase -x: sanity check command
     
    -    If the user gives an empty argument to --exec then the rebase starts to
    -    run before erroring out with
    +    If the user gives an empty argument to --exec then git creates a todo
    +    list that it cannot parse. The rebase starts to run before erroring out
    +    with
     
           error: missing arguments for exec
           error: invalid line 2: exec
    @@ -33,21 +34,16 @@
      
     +static int check_exec_cmd(const char *cmd)
     +{
    -+	int non_blank = 0;
    ++	if (strchr(cmd, '\n'))
    ++		return error(_("exec commands cannot contain newlines"));
     +
    -+	while (*cmd) {
    -+		if (*cmd == '\n')
    -+			return error(_("exec commands cannot contain newlines"));
    -+		if (!isspace(*cmd))
    -+			non_blank = 1;
    -+		cmd++;
    -+	}
    ++	/* Does the command consist purely of whitespace? */
    ++	if (!cmd[strspn(cmd, " \t\r\f\v")])
    ++		return error(_("empty exec command"));
     +
    -+	if (non_blank)
    -+		return 0;
    -+
    -+	return error(_("empty exec command"));
    ++	return 0;
     +}
    ++
     +
      int cmd_rebase(int argc, const char **argv, const char *prefix)
      {

 builtin/rebase.c              | 17 +++++++++++++++++
 t/t3404-rebase-interactive.sh | 19 +++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 00de70365e..f75bd79b45 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -793,6 +793,19 @@ static void set_reflog_action(struct rebase_options *options)
 	strbuf_release(&buf);
 }
 
+static int check_exec_cmd(const char *cmd)
+{
+	if (strchr(cmd, '\n'))
+		return error(_("exec commands cannot contain newlines"));
+
+	/* Does the command consist purely of whitespace? */
+	if (!cmd[strspn(cmd, " \t\r\f\v")])
+		return error(_("empty exec command"));
+
+	return 0;
+}
+
+
 int cmd_rebase(int argc, const char **argv, const char *prefix)
 {
 	struct rebase_options options = {
@@ -1130,6 +1143,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 		}
 	}
 
+	for (i = 0; i < exec.nr; i++)
+		if (check_exec_cmd(exec.items[i].string))
+			exit(1);
+
 	if (!(options.flags & REBASE_NO_QUIET))
 		argv_array_push(&options.git_am_opts, "-q");
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 7a440e08d8..c98f64eb2d 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -147,6 +147,25 @@ test_expect_success 'rebase -i with the exec command checks tree cleanness' '
 	git rebase --continue
 '
 
+test_expect_success 'rebase -x with empty command fails' '
+	test_when_finished "git rebase --abort ||:" &&
+	test_must_fail git rebase -x "" @ 2>actual &&
+	test_write_lines "error: empty exec command" >expected &&
+	test_i18ncmp expected actual &&
+	test_must_fail git rebase -x " " @ 2>actual &&
+	test_i18ncmp expected actual
+'
+
+LF='
+'
+test_expect_success 'rebase -x with newline in command fails' '
+	test_when_finished "git rebase --abort ||:" &&
+	test_must_fail git rebase -x "a${LF}b" @ 2>actual &&
+	test_write_lines "error: exec commands cannot contain newlines" \
+			 >expected &&
+	test_i18ncmp expected actual
+'
+
 test_expect_success 'rebase -i with exec of inexistent command' '
 	git checkout master &&
 	test_when_finished "git rebase --abort" &&
-- 
2.20.1