@matheusbsbs / matheus-bagliano / commits / 16b3880dd7

rebase -i: check labels and refs when parsing todo list

Check that the argument to the "label" and "update-ref" commands is a valid refname when the todo list is parsed rather than waiting until the command is executed. This means that the user can deal with any errors at the beginning of the rebase rather than having it stop halfway through due to a typo in a label name. The "update-ref" command is changed to reject single level refs as it is all to easy to type "update-ref branch" which is incorrect rather than "update-ref refs/heads/branch" Note that it is not straight forward to check the arguments to "reset" and "merge" commands as they may be any revision, not just a refname and we do not have an equivalent of check_refname_format() for revisions. Helped-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Feb 20, 2023 at 14:19 UTC 16b3880dd7e617f909b2a3bc3672e29a897842bd
2 files changed +61 -1
sequencer.c
+38 -1
@@ -2487,6 +2487,34 @@ static int is_command(enum todo_command command, const char **bol)
2487 (*bol = p));
2488 }
2489
2490 +static int check_label_or_ref_arg(enum todo_command command, const char *arg)
2491 +{
2492 + switch (command) {
2493 + case TODO_LABEL:
2494 + /*
2495 + * '#' is not a valid label as the merge command uses it to
2496 + * separate merge parents from the commit subject.
2497 + */
2498 + if (!strcmp(arg, "#") ||
2499 + check_refname_format(arg, REFNAME_ALLOW_ONELEVEL))
2500 + return error(_("'%s' is not a valid label"), arg);
2501 + break;
2502 +
2503 + case TODO_UPDATE_REF:
2504 + if (check_refname_format(arg, REFNAME_ALLOW_ONELEVEL))
2505 + return error(_("'%s' is not a valid refname"), arg);
2506 + if (check_refname_format(arg, 0))
2507 + return error(_("update-ref requires a fully qualified "
2508 + "refname e.g. refs/heads/%s"), arg);
2509 + break;
2510 +
2511 + default:
2512 + BUG("unexpected todo_command");
2513 + }
2514 +
2515 + return 0;
2516 +}
2517 +
2518 static int parse_insn_line(struct repository *r, struct todo_item *item,
2519 const char *buf, const char *bol, char *eol)
2520 {
@@ -2535,10 +2563,19 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
2563
2564 if (item->command == TODO_EXEC || item->command == TODO_LABEL ||
2565 item->command == TODO_RESET || item->command == TODO_UPDATE_REF) {
2566 + int ret = 0;
2567 +
2568 item->commit = NULL;
2569 item->arg_offset = bol - buf;
2570 item->arg_len = (int)(eol - bol);
2541 - return 0;
2571 + if (item->command == TODO_LABEL ||
2572 + item->command == TODO_UPDATE_REF) {
2573 + saved = *eol;
2574 + *eol = '\0';
2575 + ret = check_label_or_ref_arg(item->command, bol);
2576 + *eol = saved;
2577 + }
2578 + return ret;
2579 }
2580
2581 if (item->command == TODO_FIXUP) {
t/t3404-rebase-interactive.sh
+23
@@ -2072,6 +2072,7 @@ test_expect_success '--update-refs: --edit-todo with no update-ref lines' '
2072 '
2073
2074 test_expect_success '--update-refs: check failed ref update' '
2075 + test_when_finished "test_might_fail git rebase --abort" &&
2076 git checkout -B update-refs-error no-conflict-branch &&
2077 git branch -f base HEAD~4 &&
2078 git branch -f first HEAD~3 &&
@@ -2123,6 +2124,28 @@ test_expect_success '--update-refs: check failed ref update' '
2124 test_cmp expect err.trimmed
2125 '
2126
2127 +test_expect_success 'bad labels and refs rejected when parsing todo list' '
2128 + test_when_finished "test_might_fail git rebase --abort" &&
2129 + cat >todo <<-\EOF &&
2130 + exec >execed
2131 + label #
2132 + label :invalid
2133 + update-ref :bad
2134 + update-ref topic
2135 + EOF
2136 + rm -f execed &&
2137 + (
2138 + set_replace_editor todo &&
2139 + test_must_fail git rebase -i HEAD 2>err
2140 + ) &&
2141 + grep "'\''#'\'' is not a valid label" err &&
2142 + grep "'\'':invalid'\'' is not a valid label" err &&
2143 + grep "'\'':bad'\'' is not a valid refname" err &&
2144 + grep "update-ref requires a fully qualified refname e.g. refs/heads/topic" \
2145 + err &&
2146 + test_path_is_missing execed
2147 +'
2148 +
2149 # This must be the last test in this file
2150 test_expect_success '$EDITOR and friends are unchanged' '
2151 test_editor_unchanged