Skip to content

Commit 8efe3d6

Browse files
committed
check_refname_format(): add FULLY_QUALIFIED flag
Before operating on a refname we get from a user, we usually check that it's syntactically valid. As a general rule, refs should be in the "refs/" namespace, the exception being root refs like HEAD, REBASE_HEAD, and so on (including pseudorefs like FETCH_HEAD). Those root refs should consist only of all-caps and underscore, but the syntactic rules are not enforced by check_refname_format(). So for example we will happily operate on a ref "foo/bar" that will use the file ".git/foo/bar" under the hood (when using the files backend, of course). Making things even more complicated, refname_is_safe() does enforce these syntax restrictions! When that function was added in d0f810f (refs.c: allow listing and deleting badly named refs, 2014-09-03), we would have refused to work with such refs entirely. But we stopped calling it in every code path in 03afcbe (read_packed_refs: avoid double-checking sane refs, 2015-04-16). The rationale given in that commit is that check_refname_format() is supposed to contain a superset of the checks of refname_is_safe(). So the idea is that we usually would rely on the more-strict check_refname_format(), but for certain operations (e.g., deleting a ref) we want to allow invalid names as long as they are not unsafe (e.g., not escaping the on-disk "refs/" hierarchy). But the root ref handling violates that logic; check_refname_format() is more lenient than refname_is_safe(). So you can create "foo/bar" and read it, but you cannot delete it: $ git update-ref foo/bar HEAD $ git rev-parse foo/bar 747a29934757b7e695781e13e2511c43b951da2 $ git update-ref -d foo/bar error: refusing to update ref with bad name 'foo/bar' So we probably want check_ref_format() to learn the same syntactic restrictions that refname_is_safe() uses (namely insisting that anything outside of "refs/" matches the root ref syntax). The most obvious way to do that is simply to call refname_is_safe(). But the point of 03afcbe is that doing so is expensive. Without the syntactic restrictions of check_refname_format(), refname_is_safe() has to actually normalize the pathname to make sure it does not escape "refs/". That's redundant for us in check_refname_format(); we just need to make sure it either starts with "refs/" or uses the all-caps root-ref syntax. But wait, it gets more complicated! We also allow some special worktree refnames: "worktrees/foo/$X" and "main-worktree/$X". In that case we should only be checking "$X" (which should be either a root ref or start with "refs/"). We can use parse_worktree_ref(), which fairly efficiently gives us the "bare" refname (even for a non-worktree ref, where it returns the original name). And now when should this new logic kick in? Unfortunately we can't just do it all the time, because many callers pass in partial ref components to check_ref_format(). E.g., if they are thinking about making "refs/heads/foo", they'll pass us "foo". This is usually accompanied by the ALLOW_ONELEVEL flag. But we likewise can't take the absence of ALLOW_ONELEVEL as a hint that the name is fully qualified, because that flag is also used to indicate that root refs should be allowed! We need a new flag to tell these two situations apart. So let's add a FULLY_QUALIFIED flag that callers can use to ask us to enforce these syntactic rules. There are no callers yet, but we should be able to examine users of ALLOW_ONELEVEL, figure out which semantics they wanted, and convert as needed. Signed-off-by: Jeff King <peff@peff.net>
1 parent 48a8b45 commit 8efe3d6

2 files changed

Lines changed: 14 additions & 1 deletion

File tree

refs.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,15 @@ static int check_or_sanitize_refname(const char *refname, int flags,
280280
{
281281
int component_len, component_count = 0;
282282

283+
if ((flags & REFNAME_FULLY_QUALIFIED)) {
284+
const char *bare_ref;
285+
286+
parse_worktree_ref(refname, NULL, NULL, &bare_ref);
287+
if (!starts_with(bare_ref, "refs/") &&
288+
!is_root_ref_syntax(bare_ref))
289+
return -1;
290+
}
291+
283292
if (!strcmp(refname, "@")) {
284293
/* Refname is a single character '@'. */
285294
if (sanitized)
@@ -314,8 +323,11 @@ static int check_or_sanitize_refname(const char *refname, int flags,
314323
else
315324
return -1;
316325
}
317-
if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
326+
327+
if (!(flags & (REFNAME_ALLOW_ONELEVEL | REFNAME_FULLY_QUALIFIED)) &&
328+
component_count < 2)
318329
return -1; /* Refname has only one component. */
330+
319331
return 0;
320332
}
321333

refs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,7 @@ int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_dat
685685

686686
#define REFNAME_ALLOW_ONELEVEL 1
687687
#define REFNAME_REFSPEC_PATTERN 2
688+
#define REFNAME_FULLY_QUALIFIED 4
688689

689690
/*
690691
* Return 0 iff refname has the correct format for a refname according

0 commit comments

Comments
 (0)