All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] t1402: test forbidden characters in refnames
@ 2026-08-13 20:43 Nikolaus Schuetz via GitGitGadget
  2026-08-19 11:20 ` Patrick Steinhardt
  2026-08-20 22:20 ` [PATCH v2] " Nikolaus Schuetz via GitGitGadget
  0 siblings, 2 replies; 5+ messages in thread
From: Nikolaus Schuetz via GitGitGadget @ 2026-08-13 20:43 UTC (permalink / raw)
  To: git; +Cc: Nikolaus Schuetz, Nikolaus Schuetz

From: Nikolaus Schuetz <nikolauspschuetz@gmail.com>

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.

Signed-off-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
---
    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(+)

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'
 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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] t1402: test forbidden characters in refnames
  2026-08-13 20:43 [PATCH] t1402: test forbidden characters in refnames Nikolaus Schuetz via GitGitGadget
@ 2026-08-19 11:20 ` Patrick Steinhardt
  2026-08-19 20:22   ` Junio C Hamano
  2026-08-20 22:20 ` [PATCH v2] " Nikolaus Schuetz via GitGitGadget
  1 sibling, 1 reply; 5+ messages in thread
From: Patrick Steinhardt @ 2026-08-19 11:20 UTC (permalink / raw)
  To: Nikolaus Schuetz via GitGitGadget; +Cc: git, Nikolaus Schuetz

On Thu, Aug 13, 2026 at 08:43:56PM +0000, Nikolaus Schuetz via GitGitGadget wrote:
> From: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
> 
> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] t1402: test forbidden characters in refnames
  2026-08-19 11:20 ` Patrick Steinhardt
@ 2026-08-19 20:22   ` Junio C Hamano
  2026-08-20 14:46     ` Nikolaus Schuetz
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2026-08-19 20:22 UTC (permalink / raw)
  To: Patrick Steinhardt
  Cc: Nikolaus Schuetz via GitGitGadget, git, Nikolaus Schuetz

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Aug 13, 2026 at 08:43:56PM +0000, Nikolaus Schuetz via GitGitGadget wrote:
>> From: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
>> 
>> 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.

"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.

> 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

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

> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] t1402: test forbidden characters in refnames
  2026-08-19 20:22   ` Junio C Hamano
@ 2026-08-20 14:46     ` Nikolaus Schuetz
  0 siblings, 0 replies; 5+ messages in thread
From: Nikolaus Schuetz @ 2026-08-20 14:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Patrick Steinhardt

> 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.

> By the way, one weird bit: is it intentional that all of these really
> use "heads/something" instead of "refs/heads/something"?

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] t1402: test forbidden characters in refnames
  2026-08-13 20:43 [PATCH] t1402: test forbidden characters in refnames Nikolaus Schuetz via GitGitGadget
  2026-08-19 11:20 ` Patrick Steinhardt
@ 2026-08-20 22:20 ` Nikolaus Schuetz via GitGitGadget
  1 sibling, 0 replies; 5+ messages in thread
From: Nikolaus Schuetz via GitGitGadget @ 2026-08-20 22:20 UTC (permalink / raw)
  To: git; +Cc: Patrick Steinhardt, Nikolaus Schuetz, Nikolaus Schuetz

From: Nikolaus Schuetz <nikolauspschuetz@gmail.com>

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.

Signed-off-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
---
    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.
      
          Signed-off-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com>
      
       ## 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(-)

diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index cabc516ae9..9dd64662b2 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-20 22:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 20:43 [PATCH] t1402: test forbidden characters in refnames Nikolaus Schuetz via GitGitGadget
2026-08-19 11:20 ` Patrick Steinhardt
2026-08-19 20:22   ` Junio C Hamano
2026-08-20 14:46     ` Nikolaus Schuetz
2026-08-20 22:20 ` [PATCH v2] " Nikolaus Schuetz via GitGitGadget

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.