[PATCH] gittrack.sh accepts invalid branch names

[PATCH] gittrack.sh accepts invalid branch names

[Pavel Roskin]

Hello, Petr and everybody!

gittrack.sh allows abbreviated branch names, e.g. it's possible to run
"git track lin" when there is a branch called "linus".

I believe it's a bug, not a feature.  Please look at this line from
gittrack.sh:

grep -q $(echo -e "^$name\t" | sed 's/\./\\./g') .git/remotes

The result of command expansion is subjected to word splitting, which
means the trailing tab is removed as a space.  So grep doesn't see the
tab.

The way to avoid word splitting would be to quote "$()", but it would
make the shell code too hairy.  I'm not even sure all shells would
interpret "$("$name")" correctly.

So I decided to use tab directly in the sed expression.  I cannot think
of any portable way to avoid grep completely ("q" is a GNU sed
extension, and we want to support BSD, I think), so it's still there,
looking for any output from sed.

Signed-off-by: Pavel Roskin <proski@gnu.org>

--- a/gittrack.sh
+++ b/gittrack.sh
@@ -35,7 +35,7 @@ die () {
 mkdir -p .git/heads
 
 if [ "$name" ]; then
-	grep -q $(echo -e "^$name\t" | sed 's/\./\\./g') .git/remotes || \
+	sed -ne "/^$name\t/p" .git/remotes | grep -q . || \
 		[ -s ".git/heads/$name" ] || \
 		die "unknown branch \"$name\""
 

-- 
Regards,
Pavel Roskin

Re: [PATCH] gittrack.sh accepts invalid branch names

[Petr Baudis]

Dear diary, on Wed, Apr 20, 2005 at 09:48:30PM CEST, I got a letter
where Pavel Roskin <proski@gnu.org> told me that...
> --- a/gittrack.sh
> +++ b/gittrack.sh
> @@ -35,7 +35,7 @@ die () {
>  mkdir -p .git/heads
>  
>  if [ "$name" ]; then
> -	grep -q $(echo -e "^$name\t" | sed 's/\./\\./g') .git/remotes || \
> +	sed -ne "/^$name\t/p" .git/remotes | grep -q . || \
>  		[ -s ".git/heads/$name" ] || \
>  		die "unknown branch \"$name\""

This fixes the acceptance, but not the choice.

What does the grep -q . exactly do? Just sets error code based on
whether the sed output is non-empty? What about [] instead?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

Re: [PATCH] gittrack.sh accepts invalid branch names

[Paul Jackson]

Pavel wrote:
> 	sed -ne "/^$name\t/p" .git/remotes | grep -q .

Consider using the following to look for a match of $name with
the first tab separated field of the remotes file (and to avoid
using 'grep -q', which is not in all grep's, so far as I know):

	cut -f1 .git/remotes | grep -Fx "$name" >/dev/null

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [PATCH] gittrack.sh accepts invalid branch names

[Pavel Roskin]

Hi, Petr!

On Thu, 2005-04-21 at 01:21 +0200, Petr Baudis wrote:
> Dear diary, on Wed, Apr 20, 2005 at 09:48:30PM CEST, I got a letter
> where Pavel Roskin <proski@gnu.org> told me that...
> > --- a/gittrack.sh
> > +++ b/gittrack.sh
> > @@ -35,7 +35,7 @@ die () {
> >  mkdir -p .git/heads
> >  
> >  if [ "$name" ]; then
> > -	grep -q $(echo -e "^$name\t" | sed 's/\./\\./g') .git/remotes || \
> > +	sed -ne "/^$name\t/p" .git/remotes | grep -q . || \
> >  		[ -s ".git/heads/$name" ] || \
> >  		die "unknown branch \"$name\""
> 
> This fixes the acceptance, but not the choice.
> 
> What does the grep -q . exactly do? Just sets error code based on
> whether the sed output is non-empty?

Yes.

>  What about [] instead?

You'll need another pair of quotes for that:

[ "$(sed -ne "/^$name\t/p" .git/remotes)" ]; echo $?

If I remember correctly from my Autoconf hacking experience, not all
shells like mixing quotes and command substitution, and even bash
treated this differently in different versions.  I can do more research,
but it seems just too fragile to me.

Another thing I remember is that "case" would not need quotes.  For some
historic reasons, the expression between "case" and "in" is subjected to
command substitution, but not word expansion.

So the patch becomes:

--- a/gittrack.sh
+++ b/gittrack.sh
@@ -35,9 +35,11 @@ die () {
 mkdir -p .git/heads
 
 if [ "$name" ]; then
-	grep -q $(echo -e "^$name\t" | sed 's/\./\\./g') .git/remotes || \
+	case x$(sed -ne "/^$name\t/p" .git/remotes) in
+	x)
 		[ -s ".git/heads/$name" ] || \
-		die "unknown branch \"$name\""
+		die "unknown branch \"$name\"" ;;
+	esac
 
 	echo $name >.git/tracking
 
Looks rather ugly for my taste, but just in case:
Signed-off-by: Pavel Roskin <proski@gnu.org>

By the way, please check all references to .git/remotes - this bug is
not specific to gittrack.sh.


-- 
Regards,
Pavel Roskin