refs.c: allow listing and deleting badly named refs

We currently do not handle badly named refs well:

  $ cp .git/refs/heads/master .git/refs/heads/master.....@\*@\\.
  $ git branch
    fatal: Reference has invalid format: 'refs/heads/master.....@*@\.'
  $ git branch -D master.....@\*@\\.
    error: branch 'master.....@*@\.' not found.

Users cannot recover from a badly named ref without manually finding
and deleting the loose ref file or appropriate line in packed-refs.
Making that easier will make it easier to tweak the ref naming rules
in the future, for example to forbid shell metacharacters like '`'
and '"', without putting people in a state that is hard to get out of.

So allow "branch --list" to show these refs and allow "branch -d/-D"
and "update-ref -d" to delete them.  Other commands (for example to
rename refs) will continue to not handle these refs but can be changed
in later patches.

Details:

In resolving functions, refuse to resolve refs that don't pass the
git-check-ref-format(1) check unless the new RESOLVE_REF_ALLOW_BAD_NAME
flag is passed.  Even with RESOLVE_REF_ALLOW_BAD_NAME, refuse to
resolve refs that escape the refs/ directory and do not match the
pattern [A-Z_]* (think "HEAD" and "MERGE_HEAD").

In locking functions, refuse to act on badly named refs unless they
are being deleted and either are in the refs/ directory or match [A-Z_]*.

Just like other invalid refs, flag resolved, badly named refs with the
REF_ISBROKEN flag, treat them as resolving to null_sha1, and skip them
in all iteration functions except for for_each_rawref.

Flag badly named refs (but not symrefs pointing to badly named refs)
with a REF_BAD_NAME flag to make it easier for future callers to
notice and handle them specially.  For example, in a later patch
for-each-ref will use this flag to detect refs whose names can confuse
callers parsing for-each-ref output.

In the transaction API, refuse to create or update badly named refs,
but allow deleting them (unless they try to escape refs/ and don't match
[A-Z_]*).

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 124 insertions, 1 deletions

diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index c7b19b0548..468e85621a 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -4,7 +4,8 @@ test_description='Test handling of ref names that check-ref-format rejects'
 . ./test-lib.sh
 
 test_expect_success setup '
-	test_commit one
+	test_commit one &&
+	test_commit two
 '
 
 test_expect_success 'fast-import: fail on invalid branch name ".badbranchname"' '
@@ -37,6 +38,107 @@ test_expect_success 'fast-import: fail on invalid branch name "bad[branch]name"'
 	test_must_fail git fast-import <input
 '
 
+test_expect_success 'git branch shows badly named ref' '
+	cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	git branch >output &&
+	grep -e "broken\.\.\.ref" output
+'
+
+test_expect_success 'branch -d can delete badly named ref' '
+	cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	git branch -d broken...ref &&
+	git branch >output &&
+	! grep -e "broken\.\.\.ref" output
+'
+
+test_expect_success 'branch -D can delete badly named ref' '
+	cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	git branch -D broken...ref &&
+	git branch >output &&
+	! grep -e "broken\.\.\.ref" output
+'
+
+test_expect_success 'branch -D cannot delete non-ref in .git dir' '
+	echo precious >.git/my-private-file &&
+	echo precious >expect &&
+	test_must_fail git branch -D ../../my-private-file &&
+	test_cmp expect .git/my-private-file
+'
+
+test_expect_success 'branch -D cannot delete absolute path' '
+	git branch -f extra &&
+	test_must_fail git branch -D "$(pwd)/.git/refs/heads/extra" &&
+	test_cmp_rev HEAD extra
+'
+
+test_expect_success 'git branch cannot create a badly named ref' '
+	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test_must_fail git branch broken...ref &&
+	git branch >output &&
+	! grep -e "broken\.\.\.ref" output
+'
+
+test_expect_success 'branch -m cannot rename to a bad ref name' '
+	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test_might_fail git branch -D goodref &&
+	git branch goodref &&
+	test_must_fail git branch -m goodref broken...ref &&
+	test_cmp_rev master goodref &&
+	git branch >output &&
+	! grep -e "broken\.\.\.ref" output
+'
+
+test_expect_failure 'branch -m can rename from a bad ref name' '
+	cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	git branch -m broken...ref renamed &&
+	test_cmp_rev master renamed &&
+	git branch >output &&
+	! grep -e "broken\.\.\.ref" output
+'
+
+test_expect_success 'push cannot create a badly named ref' '
+	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	test_must_fail git push "file://$(pwd)" HEAD:refs/heads/broken...ref &&
+	git branch >output &&
+	! grep -e "broken\.\.\.ref" output
+'
+
+test_expect_failure 'push --mirror can delete badly named ref' '
+	top=$(pwd) &&
+	git init src &&
+	git init dest &&
+
+	(
+		cd src &&
+		test_commit one
+	) &&
+	(
+		cd dest &&
+		test_commit two &&
+		git checkout --detach &&
+		cp .git/refs/heads/master .git/refs/heads/broken...ref
+	) &&
+	git -C src push --mirror "file://$top/dest" &&
+	git -C dest branch >output &&
+	! grep -e "broken\.\.\.ref" output
+'
+
+test_expect_success 'rev-parse skips symref pointing to broken name' '
+	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	git branch shadow one &&
+	cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+	git symbolic-ref refs/tags/shadow refs/heads/broken...ref &&
+
+	git rev-parse --verify one >expect &&
+	git rev-parse --verify shadow >actual 2>err &&
+	test_cmp expect actual &&
+	test_i18ngrep "ignoring.*refs/tags/shadow" err
+'
+
 test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '
 	git symbolic-ref refs/heads/badname refs/heads/broken...ref &&
 	test_when_finished "rm -f .git/refs/heads/badname" &&
@@ -45,6 +147,27 @@ test_expect_success 'update-ref --no-deref -d can delete reference to broken nam
 	test_path_is_missing .git/refs/heads/badname
 '
 
+test_expect_success 'update-ref -d can delete broken name' '
+	cp .git/refs/heads/master .git/refs/heads/broken...ref &&
+	test_when_finished "rm -f .git/refs/heads/broken...ref" &&
+	git update-ref -d refs/heads/broken...ref &&
+	git branch >output &&
+	! grep -e "broken\.\.\.ref" output
+'
+
+test_expect_success 'update-ref -d cannot delete non-ref in .git dir' '
+	echo precious >.git/my-private-file &&
+	echo precious >expect &&
+	test_must_fail git update-ref -d my-private-file &&
+	test_cmp expect .git/my-private-file
+'
+
+test_expect_success 'update-ref -d cannot delete absolute path' '
+	git branch -f extra &&
+	test_must_fail git update-ref -d "$(pwd)/.git/refs/heads/extra" &&
+	test_cmp_rev HEAD extra
+'
+
 test_expect_success 'update-ref --stdin fails create with bad ref name' '
 	echo "create ~a refs/heads/master" >stdin &&
 	test_must_fail git update-ref --stdin <stdin 2>err &&