t1402: test forbidden characters in refnames

From: Nikolaus Schuetz <[email protected]>

git-check-ref-format(1) documents that a refname cannot contain a space, tilde, caret, colon, question-mark, asterisk or open-bracket, and that it cannot be the single character "@". Of these, only "?" was tested as a character embedded in an otherwise-valid refname; "*" was checked only as a lone character or with --refspec-pattern.

Add the remaining forbidden characters in that embedded form, and check that "@" alone is rejected even with --allow-onelevel -- where "@" is otherwise a valid refname component, as "refs/@" confirms.

---
t1402: test forbidden characters in refnames

git-check-ref-format(1) documents the characters that a refname may not contain (space, tilde, caret, colon, question-mark, asterisk, open-bracket) and the rule that it may not be the single character "@". t1402 only exercised a few of these directly.

This adds the remaining forbidden characters in embedded form, and checks that "@" alone is rejected even with --allow-onelevel, where "@" is otherwise a valid refname component (as "refs/@" confirms).

Test-only; documents existing behaviour, in the spirit of 919eb8ace (t1402: check for refs ending with a dot).

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2203%2Fnikolauspschuetz%2Fns%2Ft1402-forbidden-characters-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2203/nikolauspschuetz/ns/t1402-forbidden-characters-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2203

 t/t1402-check-ref-format.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

index cabc516ae9..bc1e878a0f 100755
@@ -51,12 +51,20 @@ invalid_ref '.refs/foo'
 invalid_ref 'refs/heads/foo.'
 invalid_ref 'heads/foo..bar'
 invalid_ref 'heads/foo?bar'
+invalid_ref 'heads/foo~bar'
+invalid_ref 'heads/foo^bar'
+invalid_ref 'heads/foo:bar'
+invalid_ref 'heads/foo*bar'
+invalid_ref 'heads/foo[bar'
+invalid_ref 'heads/foo bar'
 valid_ref 'foo./bar'
 invalid_ref 'heads/foo.lock'
 invalid_ref 'heads///foo.lock'
 invalid_ref 'foo.lock/bar'
 invalid_ref 'foo.lock///bar'
 valid_ref 'heads/foo@bar'
+valid_ref 'refs/@'
+invalid_ref '@' --allow-onelevel
 invalid_ref 'heads/v@{ation'
 invalid_ref 'heads/foo\bar'
 invalid_ref "$(printf 'heads/foo\t')"

base-commit: 745601a9a94110d74769ab605ccd4f61339758d2

gitgitgadget

Okay.

    for c in '?' '~' '^' ':' '*' '[' ' '
    do
        invalid_ref "heads/foo${c}bar"
    done

"Why would we even care to check these insane cases?" was my first reaction, but I agree with you that these are to protect authors of new backends from stupid mistakes.

True. And c does not have to be a single byte. ".." can also be part of the repertoire.

Agreed and updated accordingly: forbidden chars are looped over, and I folded ".." in along with "\" (the same forbidden-char list). The other refname rules enforced by refs.c are well covered, so I kept the loop to the embedded forbidden tokens.

Not intentional -- the file already mixes them (e.g. 'refs/heads/foo.' vs 'heads/foo..bar'). check-ref-format validates each component regardless of a refs/ prefix, so it doesn't change what's tested; I kept 'heads/' to match the neighbours.

Thanks,
Nikolaus

From: Nikolaus Schuetz <[email protected]>

git-check-ref-format(1) documents that a refname cannot contain a space, tilde, caret, colon, question-mark, asterisk, open-bracket or backslash, nor the sequence "..", and cannot be the single character "@". Of these, only "?", "\" and ".." were tested embedded in an otherwise-valid refname; "*" was checked only as a lone character or with --refspec-pattern.

Test all of them in that embedded form with a single loop, and check that "@" alone is rejected even with --allow-onelevel -- where "@" is otherwise a valid refname component, as "refs/@" confirms.

---
t1402: test forbidden characters in refnames

git-check-ref-format(1) documents the characters that a refname may not contain (space, tilde, caret, colon, question-mark, asterisk, open-bracket) and the rule that it may not be the single character "@". t1402 only exercised a few of these directly.

This adds the remaining forbidden characters in embedded form, and checks that "@" alone is rejected even with --allow-onelevel, where "@" is otherwise a valid refname component (as "refs/@" confirms).

Test-only; documents existing behaviour, in the spirit of 919eb8ace (t1402: check for refs ending with a dot).

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2203%2Fnikolauspschuetz%2Fns%2Ft1402-forbidden-characters-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2203/nikolauspschuetz/ns/t1402-forbidden-characters-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2203

Range-diff vs v1:

1: f254db5b09 ! 1: cc013499f9 t1402: test forbidden characters in refnames
@@ Commit message
t1402: test forbidden characters in refnames

git-check-ref-format(1) documents that a refname cannot contain a
- space, tilde, caret, colon, question-mark, asterisk or open-bracket,
- and that it cannot be the single character "@". Of these, only "?"
- was tested as a character embedded in an otherwise-valid refname;
- "*" was checked only as a lone character or with --refspec-pattern.
+ space, tilde, caret, colon, question-mark, asterisk, open-bracket or
+ backslash, nor the sequence "..", and cannot be the single character
+ "@". Of these, only "?", "\" and ".." were tested embedded in an
+ otherwise-valid refname; "*" was checked only as a lone character or
+ with --refspec-pattern.

- Add the remaining forbidden characters in that embedded form, and
- check that "@" alone is rejected even with --allow-onelevel -- where
- "@" is otherwise a valid refname component, as "refs/@" confirms.
+ Test all of them in that embedded form with a single loop, and check
+ that "@" alone is rejected even with --allow-onelevel -- where "@" is
+ otherwise a valid refname component, as "refs/@" confirms.

## t/t1402-check-ref-format.sh ##
-@@ t/t1402-check-ref-format.sh: invalid_ref '.refs/foo' +@@ t/t1402-check-ref-format.sh: invalid_ref 'foo/./bar'
+ invalid_ref 'foo/bar/.'
+ invalid_ref '.refs/foo'
invalid_ref 'refs/heads/foo.'
- invalid_ref 'heads/foo..bar'
- invalid_ref 'heads/foo?bar'
-+invalid_ref 'heads/foo~bar'
-+invalid_ref 'heads/foo^bar'
-+invalid_ref 'heads/foo:bar'
-+invalid_ref 'heads/foo*bar'
-+invalid_ref 'heads/foo[bar'
-+invalid_ref 'heads/foo bar'
+-invalid_ref 'heads/foo..bar'
+-invalid_ref 'heads/foo?bar'
++for c in '?' '~' '^' ':' '*' '[' ' ' '\' '..' ++do
++ invalid_ref "heads/foo${c}bar"
++done
valid_ref 'foo./bar'
invalid_ref 'heads/foo.lock'
invalid_ref 'heads///foo.lock'
@@ t/t1402-check-ref-format.sh: invalid_ref '.refs/foo'
+valid_ref 'refs/@'
+invalid_ref '@' --allow-onelevel
invalid_ref 'heads/v@{ation'
- invalid_ref 'heads/foo\bar'
+-invalid_ref 'heads/foo\bar'
invalid_ref "$(printf 'heads/foo\t')"
+ invalid_ref "$(printf 'heads/foo\177')"
+ valid_ref "$(printf 'heads/fu\303\237')"

 t/t1402-check-ref-format.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index cabc516ae9..9dd64662b2 100755
@@ -49,16 +49,19 @@ invalid_ref 'foo/./bar'
 invalid_ref 'foo/bar/.'
 invalid_ref '.refs/foo'
 invalid_ref 'refs/heads/foo.'
-invalid_ref 'heads/foo..bar'
-invalid_ref 'heads/foo?bar'
+for c in '?' '~' '^' ':' '*' '[' ' ' '\' '..'
+do
+	invalid_ref "heads/foo${c}bar"
+done
 valid_ref 'foo./bar'
 invalid_ref 'heads/foo.lock'
 invalid_ref 'heads///foo.lock'
 invalid_ref 'foo.lock/bar'
 invalid_ref 'foo.lock///bar'
 valid_ref 'heads/foo@bar'
+valid_ref 'refs/@'
+invalid_ref '@' --allow-onelevel
 invalid_ref 'heads/v@{ation'
-invalid_ref 'heads/foo\bar'
 invalid_ref "$(printf 'heads/foo\t')"
 invalid_ref "$(printf 'heads/foo\177')"
 valid_ref "$(printf 'heads/fu\303\237')"

base-commit: 745601a9a94110d74769ab605ccd4f61339758d2

gitgitgadget

Many funny characters are not allowed between 'foo' and 'bar', but are there characters other than dot that are not allowed at the beginning or at the end (e.g., "refs/heads/foo." and "foo.lock")?

IOW are we testing exhaustive now?

No -- dot is the only character with position-specific rules, and every other character in the forbidden set is rejected anywhere in a component.

The file already exercises each of those rules on its own -- the leading/trailing-dot, ".lock", empty-component, single-level and --normalize cases are all present. So this isn't reaching for exhaustiveness; the coverage was already broad, and this just fills the untested gap -- of the "forbidden anywhere" characters, only "?", "\" and ".." were tested embedded, so I folded the rest into the loop.

Thanks,
Nikolaus