Re: Buggy handling of non-canonical ref names

["Carlos Martín Nieto" <cmn@elego.de>]

[-- Attachment #1: Type: text/plain, Size: 2445 bytes --]

On Wed, 2011-08-24 at 11:40 -0700, Junio C Hamano wrote:
> 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.

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



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]