Re: Buggy handling of non-canonical ref names
- From
Carlos Martín Nieto <cmn@elego.de>
- Date
- Aug 24, 2011, 21:32 UTC
- Message-ID
- <1314221553.4325.13.camel@centaur.lab.cmartin.tk>
- In-Reply-To
- <7vaaayps9z.fsf@alter.siamese.dyndns.org>
On Wed, 2011-08-24 at 11:40 -0700, Junio C Hamano wrote:
Show 16 quoted lines
> Michael Haggerty <mhagger@alum.mit.edu> writes: > > > What is the policy about reference names and their canonicalization? > > The overall policy has been that we care about well-formed input, and > everything else is "undefined", even though as you found out some of them > try to work sensibly. > > > $ git check-ref-format /foo/bar ; echo $? > > 0 > > > > $ git check-ref-format --print /foo/bar > > /foo/bar > > I think these are bogus. Patches welcome. >
The rules in the manpage don't forbit it, as we assume that $GIT_DIR/ is going to be put in front. This makes /foo/bar mean the same as foo/bar (both become would cause git to look at $GIT_DIR/foo/bar), but I agree that it can be quite confusing.
Show 11 quoted lines
> > However, creating a reference with such a name is equivalent to creating > > a reference without the leading slash: > > > > $ git update-ref /foo/bar HEAD > > $ cat .git/foo/bar > > ef6cf90ba11dd6205f8b974692d795ea0b1c0bdd > > $ git branch /bar/baz > > $ git for-each-ref | grep baz > > ef6cf90ba11dd6205f8b974692d795ea0b1c0bdd commit refs/heads/bar/baz > > These are just examples of "undefined being nice to the user as a bonus".
Or not, the user might have expected `git branch /bar/baz` to create a reference in $GIT_DIR/bar/baz with the SHA of the current branch (yes, it's probably unlikely, but this is what one might think when giving references as absolute paths).
Be as it may, allowing /foo/bar is not a good thing, how about this patch? It doesn't modify the manpage, as I wasn't sure whether this should become an explicit rule.
-- >8 -- Subject: [PATCH] Don't allow reference names to start with '/'
Being able to name references using absolute filenames is confusing at best and might give people wrong ideas.
Signed-off-by: Carlos Martín Nieto <cmn@elego.de> --- refs.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c index 6f313a9..ab549e4 100644 --- a/refs.c +++ b/refs.c @@ -879,6 +879,10 @@ int check_ref_format(const char *ref) int ret = CHECK_REF_FORMAT_OK; const char *cp = ref; + /* we don't want refs expressed as absolute paths */ + if (*cp == '/') + return CHECK_REF_FORMAT_ERROR; + level = 0; while (1) { while ((ch = *cp++) == '/')
-- 1.7.5.2.354.g349bf