Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
> If the user has asked that a new value be set for a reference, we use
> check_refname_format() to verify that the reference name satisfies all
> of the rules. But in other cases, at least check that refname_is_safe().
It isn't clear to me what "in other cases" exactly refers to. A
request to delete a ref would obviously one of those that do not
"ask that a new value be set", but are there other cases?
> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
> ---
> There are remaining problems in this area of the code. For example,
> check_refname_format() is *less* strict than refname_is_safe(). It
> allows almost arbitrary top-level reference names like "foo" and
> "refs". (The latter is especially fun because if you manage to create
> a reference called "refs", Git stops recognizing the directory as a
> Git repository.) On the other hand, some callers call
> check_refname_format() with incomplete reference names (e.g., branch
> names like "master"), so the functions can't be made stricter until
> those callers are changed.
>
> I'll address these problems separately if I find the time.
Thanks.
> refs.c | 5 +++--
> t/t1400-update-ref.sh | 2 +-
> t/t1430-bad-ref-name.sh | 2 +-
> 3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 858b6d7..41eb9e2 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -802,8 +802,9 @@ int ref_transaction_update(struct ref_transaction *transaction,
> if ((flags & REF_ISPRUNING) && !(flags & REF_NODEREF))
> die("BUG: REF_ISPRUNING set without REF_NODEREF");
>
> - if (new_sha1 && !is_null_sha1(new_sha1) &&
> - check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
> + if ((new_sha1 && !is_null_sha1(new_sha1)) ?
> + check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) :
> + !refname_is_safe(refname)) {
> strbuf_addf(err, "refusing to update ref with bad name '%s'",
> refname);
> return -1;
> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index 40b0cce..08bd8fd 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -23,7 +23,7 @@ test_expect_success setup '
> m=refs/heads/master
> n_dir=refs/heads/gu
> n=$n_dir/fixes
> -outside=foo
> +outside=refs/foo
>
> test_expect_success \
> "create $m" \
> diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
> index 25ddab4..8937e25 100755
> --- a/t/t1430-bad-ref-name.sh
> +++ b/t/t1430-bad-ref-name.sh
> @@ -285,7 +285,7 @@ test_expect_success 'update-ref -d cannot delete non-ref in .git dir' '
> echo precious >expect &&
> test_must_fail git update-ref -d my-private-file >output 2>error &&
> test_must_be_empty output &&
> - test_i18ngrep -e "cannot lock .*: unable to resolve reference" error &&
> + test_i18ngrep -e "refusing to update ref with bad name" error &&
> test_cmp expect .git/my-private-file
> '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html