Re: [PATCH 2/2] symbolic-ref: check format of given refname
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 16, 2012, 17:12 UTC
- Message-ID
- <7v394r4old.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1342440781-18816-3-git-send-email-mschub@elegosoft.com>
Michael Schubert <mschub@elegosoft.com> writes:
Show 37 quoted lines
> Currently, it's possible to update HEAD with a nonsense reference since
> no strict validation ist performed. Example:
>
> $ git symbolic-ref HEAD 'refs/heads/master
> >
> >
> > '
>
> Fix this by checking the given reference with check_refname_format().
>
> Signed-off-by: Michael Schubert <mschub@elegosoft.com>
> ---
> builtin/symbolic-ref.c | 4 +++-
> t/t1401-symbolic-ref.sh | 10 ++++++++++
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
> index 801d62e..a529541 100644
> --- a/builtin/symbolic-ref.c
> +++ b/builtin/symbolic-ref.c
> @@ -44,13 +44,15 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
> git_config(git_default_config, NULL);
> argc = parse_options(argc, argv, prefix, options,
> git_symbolic_ref_usage, 0);
> - if (msg &&!*msg)
> + if (msg && !*msg)
> die("Refusing to perform update with empty message");
> switch (argc) {
> case 1:
> check_symref(argv[0], quiet);
> break;
> case 2:
> + if (check_refname_format(argv[1], 0))
> + die("No valid reference format: '%s'", argv[1]);
> if (!strcmp(argv[0], "HEAD") &&
> prefixcmp(argv[1], "refs/"))
> die("Refusing to point HEAD outside of refs/");The existing context lines above may give a clue why this patch is not such a good idea. We only limit HEAD to point under refs/ but allow advanced users and scripts creative uses of other kinds of symrefs. Shouldn't the patch apply the new restriction only to HEAD as well?
By the way, should "git symbolic-ref _ HEAD" work?
Show 21 quoted lines
> diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh > index 2c96551..b1cd508 100755 > --- a/t/t1401-symbolic-ref.sh > +++ b/t/t1401-symbolic-ref.sh > @@ -27,6 +27,16 @@ test_expect_success 'symbolic-ref refuses non-ref for HEAD' ' > ' > reset_to_sane > > +test_expect_success 'symbolic-ref refuses ref with leading dot' ' > + test_must_fail git symbolic-ref HEAD refs/heads/.foo > +' > +reset_to_sane > + > +test_expect_success 'symbolic-ref refuses ref with leading dash' ' > + test_must_fail git symbolic-ref HEAD refs/heads/-foo > +' > +reset_to_sane > + > test_expect_success 'symbolic-ref refuses bare sha1' ' > echo content >file && git add file && git commit -m one && > test_must_fail git symbolic-ref HEAD `git rev-parse HEAD`