On Fri, Sep 17, 2021 at 05:47:17PM -0400, Jeff King wrote:
> On Fri, Sep 17, 2021 at 01:53:52PM -0700, Junio C Hamano wrote:
> > > On Fri, Sep 17, 2021 at 7:12 AM <gitmailinglist.bentolor@xxxxxxxx> wrote:
> > >>
> > >> A SO commenter pointed out, that git-check-ref-format forbids @ and
> > >> maybe I should report this as a potential bug. Is it?
> > >
> > > a reference that is named "@" only is invalid, but refs/tags/@ is not.
> >
> > ;-)
> >
> > "git check-ref-format master ; echo $?" would show that any single
> > level name is "forbidden", so probably the SO commenter (whatever
> > that is) was confused---it is not about @ at all.
> >
> > In any case, a tag whose name is @ may be another source of
> > confusion in the modern world, after we added @ as a synonym to
> > HEAD. I do not know, for example, offhand which between the HEAD or
> > that tag "git show @" would choose. It makes sense to avoid it.
>
> In the past when we've had confusing names (like refs/heads/HEAD), we
> continue to allow them at the plumbing level (to retain backwards
> compatibility), but flag them at the porcelain level to prevent users
> shooting themselves in the foot. This seems like a good candidate for
> that (for both git-branch and git-tag).
I was leaning towards something like that plus a Documentation update, but
noticed that the current behaviour was inconsistent, and the confusion
pointed out by Junio seems to indicate it is better if fully restricted.
Carlo
----- >8 -----
Subject: [PATCH] refs: mark "@" as an invalid refspec
9ba89f484e (Add new @ shortcut for HEAD, 2013-09-02), adds "@" to
the list of invalid refspec, but does it only for full refspec and
not as a component.
Move the logic to validate it at the component level to prevent also
tags to be created with that name.
Reported-by: Benjamin <gitmailinglist.bentolor@xxxxxxxx>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx>
---
refs.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 8b9f7c3a80..2ca6995c0f 100644
--- a/refs.c
+++ b/refs.c
@@ -47,13 +47,14 @@ int ref_storage_backend_exists(const char *name)
* 4: A bad character: ASCII control characters, and
* ":", "?", "[", "\", "^", "~", SP, or TAB
* 5: *, reject unless REFNAME_REFSPEC_PATTERN is set
+ * 6: @, only valid if used around valid characters
*/
static unsigned char refname_disposition[256] = {
1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 2, 1,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 4,
- 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ 6, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 0, 4, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
@@ -137,6 +138,14 @@ static int check_refname_component(const char *refname, int *flags,
*/
*flags &= ~ REFNAME_REFSPEC_PATTERN;
break;
+ case 6:
+ if (last == '\0' && *(cp + 1) == '\0') {
+ if (sanitized)
+ sanitized->buf[sanitized->len] = '-';
+ else
+ return -1;
+ }
+ break;
}
last = ch;
}
@@ -167,14 +176,6 @@ static int check_or_sanitize_refname(const char *refname, int flags,
{
int component_len, component_count = 0;
- if (!strcmp(refname, "@")) {
- /* Refname is a single character '@'. */
- if (sanitized)
- strbuf_addch(sanitized, '-');
- else
- return -1;
- }
-
while (1) {
if (sanitized && sanitized->len)
strbuf_complete(sanitized, '/');
--
2.33.0.911.gbe391d4e11
