Add detection for aliasing loops in cases where one of the aliases
re-invokes git as a shell command. This catches cases like:

    [alias]
    foo = !git bar
    bar = !git foo

Before this change running "git {foo,bar}" would create a
forkbomb. Now using the aliasing loop detection and call history
reporting added in 82f71d9a5a ("alias: show the call history when an
alias is looping", 2018-09-16) and c6d75bc17a ("alias: add support for
aliases of an alias", 2018-09-16) we'll instead report:

    fatal: alias loop detected: expansion of 'foo' does not terminate:
      foo <==
      bar ==>

Since the implementation carries the call history in an environment
variable, using the same sort of trick as used for -c (see
2b64fc894d ("pass "git -c foo=bar" params through environment",
2010-08-23) ). For example:

    [alias]
    one = two
    two = !git three
    three = four
    four = !git five
    five = two

Will, on "git one" report:

    fatal: alias loop detected: expansion of 'one' does not terminate:
      one
      two <==
      three
      four
      five ==>

Signed-off-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
---

Implements what I suggested in
https://public-inbox.org/git/87o9dar9qc....@evledraar.gmail.com/

 cache.h          |  1 +
 git.c            | 36 ++++++++++++++++++++++++++++++++++--
 t/t0001-init.sh  |  1 +
 t/t0014-alias.sh | 15 ++++++---------
 4 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/cache.h b/cache.h
index d508f3d4f8..00cbd25f1c 100644
--- a/cache.h
+++ b/cache.h
@@ -478,6 +478,7 @@ static inline enum object_type object_type(unsigned int 
mode)
 #define TEMPLATE_DIR_ENVIRONMENT "GIT_TEMPLATE_DIR"
 #define CONFIG_ENVIRONMENT "GIT_CONFIG"
 #define CONFIG_DATA_ENVIRONMENT "GIT_CONFIG_PARAMETERS"
+#define COMMAND_HISTORY_ENVIRONMENT "GIT_COMMAND_HISTORY"
 #define EXEC_PATH_ENVIRONMENT "GIT_EXEC_PATH"
 #define CEILING_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES"
 #define NO_REPLACE_OBJECTS_ENVIRONMENT "GIT_NO_REPLACE_OBJECTS"
diff --git a/git.c b/git.c
index 5920f8019b..cba242836c 100644
--- a/git.c
+++ b/git.c
@@ -672,12 +672,43 @@ static void execv_dashed_external(const char **argv)
                exit(128);
 }
 
+static void init_cmd_history(struct strbuf *env, struct string_list *cmd_list)
+{
+       const char *old = getenv(COMMAND_HISTORY_ENVIRONMENT);
+       struct strbuf **cmd_history, **ptr;
+
+       if (!old || !*old)
+               return;
+
+       strbuf_addstr(env, old);
+       strbuf_rtrim(env);
+
+       cmd_history = strbuf_split_buf(old, strlen(old), ' ', 0);
+       for (ptr = cmd_history; *ptr; ptr++) {
+               strbuf_rtrim(*ptr);
+               string_list_append(cmd_list, (*ptr)->buf);
+       }
+       strbuf_list_free(cmd_history);
+}
+
+static void add_cmd_history(struct strbuf *env, struct string_list *cmd_list,
+                           const char *cmd)
+{
+       string_list_append(cmd_list, cmd);
+       if (env->len)
+               strbuf_addch(env, ' ');
+       strbuf_addstr(env, cmd);
+       setenv(COMMAND_HISTORY_ENVIRONMENT, env->buf, 1);
+}
+
 static int run_argv(int *argcp, const char ***argv)
 {
        int done_alias = 0;
-       struct string_list cmd_list = STRING_LIST_INIT_NODUP;
+       struct string_list cmd_list = STRING_LIST_INIT_DUP;
        struct string_list_item *seen;
+       struct strbuf env = STRBUF_INIT;
 
+       init_cmd_history(&env, &cmd_list);
        while (1) {
                /*
                 * If we tried alias and futzed with our environment,
@@ -711,7 +742,7 @@ static int run_argv(int *argcp, const char ***argv)
                              " not terminate:%s"), cmd_list.items[0].string, 
sb.buf);
                }
 
-               string_list_append(&cmd_list, *argv[0]);
+               add_cmd_history(&env, &cmd_list, *argv[0]);
 
                /*
                 * It could be an alias -- this works around the insanity
@@ -724,6 +755,7 @@ static int run_argv(int *argcp, const char ***argv)
        }
 
        string_list_clear(&cmd_list, 0);
+       strbuf_release(&env);
 
        return done_alias;
 }
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 182da069f1..eb2ca8a172 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -93,6 +93,7 @@ test_expect_success 'No extra GIT_* on alias scripts' '
                sed -n \
                        -e "/^GIT_PREFIX=/d" \
                        -e "/^GIT_TEXTDOMAINDIR=/d" \
+                       -e "/^GIT_COMMAND_HISTORY=/d" \
                        -e "/^GIT_/s/=.*//p" |
                sort
        EOF
diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
index a070e645d7..9ed03a4a4f 100755
--- a/t/t0014-alias.sh
+++ b/t/t0014-alias.sh
@@ -27,14 +27,11 @@ test_expect_success 'looping aliases - internal execution' '
        test_i18ngrep "^fatal: alias loop detected: expansion of" output
 '
 
-# This test is disabled until external loops are fixed, because would block
-# the test suite for a full minute.
-#
-#test_expect_failure 'looping aliases - mixed execution' '
-#      git config alias.loop-mixed-1 loop-mixed-2 &&
-#      git config alias.loop-mixed-2 "!git loop-mixed-1" &&
-#      test_must_fail git loop-mixed-1 2>output &&
-#      test_i18ngrep "^fatal: alias loop detected: expansion of" output
-#'
+test_expect_success 'looping aliases - mixed execution' '
+       git config alias.loop-mixed-1 loop-mixed-2 &&
+       git config alias.loop-mixed-2 "!git loop-mixed-1" &&
+       test_must_fail git loop-mixed-1 2>output &&
+       test_i18ngrep "^fatal: alias loop detected: expansion of" output
+'
 
 test_done
-- 
2.19.1.568.g152ad8e336

Reply via email to