threads / patch / 176

patchgittrack.sh accepts invalid branch names

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

## tl;dr

4 messages between Apr 20, 2005 and Apr 21, 2005. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Pavel Roskin· Apr 20, 2005, 19:48 UTC · lore
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>
Show changes to gittrack.sh +1 −1
--- 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
Petr Baudis· Apr 20, 2005, 23:21 UTC · re: Pavel Roskin · lore

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

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

Show 10 quoted lines
> --- 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
Pavel Roskin· Apr 21, 2005, 01:28 UTC · re: Petr Baudis · lore

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

Hi, Petr!
On Thu, 2005-04-21 at 01:21 +0200, Petr Baudis wrote:
Show 17 quoted lines
> 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:
Show changes to gittrack.sh +4 −2
--- 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
Paul Jackson· Apr 20, 2005, 23:22 UTC · re: Pavel Roskin · lore

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

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

← back to recent threads