On Sat, Jul 30, 2022 at 12:53 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> And "git symbolic-ref" is perfectly happy to take complete garbage
> input. There seems to be no advantage over using that silly "echo"
> model.
Side note: it looks like that patch may break a test.
And that test most definitely *should* be broken.
t3200-branch.sh does
git symbolic-ref refs/heads/dangling-symref nowhere
which really depends on that whole "git symbolic-ref does no sanity
checking at all".
In fact, it seems to depend particularly on the fact that for non-HEAD
refs, it does even *less* sanity checking, and doesn't even check that
the ref starts with a "refs/"
There is also t4202-log, which wants to test that "git log" reacts
well to a bad ref. But now that "git symbolic-ref" refuses to create
such a bad ref, that test fails.
Anyway, here's a slightly updated patch that just fixes that test that
depended on not just a dangling symref, but an *invalid* dangling
symref. And it changes t4202-log to use "echo" to create the bad ref
instead. Which is what the previous test did too, to create the bogus
hash.
Again - this is such a low-level plumbing thing that maybe nobody
cares, but it just struck me as a bad idea to have these kinds of
maintenance commands that can be used to just mess up your repository.
And if you have a bare repo, this really does look like the command
that *should* be used to change HEAD, since it's not about "git
checkout"
Linus
builtin/symbolic-ref.c | 2 ++
t/t3200-branch.sh | 4 ++--
t/t4202-log.sh | 2 +-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index e547a08d6c..5354cfb4f1 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -71,6 +71,8 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
if (!strcmp(argv[0], "HEAD") &&
!starts_with(argv[1], "refs/"))
die("Refusing to point HEAD outside of refs/");
+ if (check_refname_format(argv[1], 0) < 0)
+ die("Refusing to set '%s' to invalid ref '%s'", argv[0], argv[1]);
ret = !!create_symref(argv[0], argv[1], msg);
break;
default:
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 9723c2827c..b194c1b09b 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -723,9 +723,9 @@ test_expect_success 'deleting a symref' '
'
test_expect_success 'deleting a dangling symref' '
- git symbolic-ref refs/heads/dangling-symref nowhere &&
+ git symbolic-ref refs/heads/dangling-symref refs/heads/nowhere &&
test_path_is_file .git/refs/heads/dangling-symref &&
- echo "Deleted branch dangling-symref (was nowhere)." >expect &&
+ echo "Deleted branch dangling-symref (was refs/heads/nowhere)." >expect &&
git branch -d dangling-symref >actual &&
test_path_is_missing .git/refs/heads/dangling-symref &&
test_cmp expect actual
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 6e66352558..427b06442d 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -2114,7 +2114,7 @@ test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
test_expect_success 'log diagnoses bogus HEAD symref' '
git init empty &&
- git --git-dir empty/.git symbolic-ref HEAD refs/heads/invalid.lock &&
+ echo "ref: refs/heads/invalid.lock" > empty/.git/HEAD &&
test_must_fail git -C empty log 2>stderr &&
test_i18ngrep broken stderr &&
test_must_fail git -C empty log --default totally-bogus 2>stderr &&
