Re: [PATCH] t1402: test forbidden characters in refnames
Patrick Steinhardt <[email protected]>
| Newsgroups | org.kernel.vger.git |
|---|---|
| Message-ID | <[email protected]> |
On Thu, Aug 13, 2026 at 08:43:56PM +0000, Nikolaus Schuetz via GitGitGadget wrote: > 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. Okay. > diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh > index cabc516ae9..bc1e878a0f 100755 > --- a/t/t1402-check-ref-format.sh > +++ b/t/t1402-check-ref-format.sh > @@ -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' This feels a tiny bit excessive, but I guess it does not hurt to enforce this property, especially now that it's so easy to add new backends. One thing I was briefly wondering is whether we could maybe have a simple loop here, as this feels quite repetitive. We could for example: for c in '?' '~' '^' ':' '*' '[' ' ' do invalid_ref "heads/foo${c}bar" done By the way, one weird bit: is it intentional that all of these really use "heads/something" instead of "refs/heads/something"? I guess it ultimately doesn't matter. > 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 This one certainly is a good addition, as these are quite a bit more subtle. Thanks! Patrick