Re: [PATCH] [COGITO] make cg-tag use git-check-ref-format

Martin Atukunda [off-list ref] writes:

The egrep pattern used by cg-tag is too restrictive. While it will prevent
control characters from being specified as a tag name, it will also reject
nearly anything written in a non-English language, as noted by -hpa
...
-(echo $name | egrep -qv '[^a-zA-Z0-9_.@!:-]') || \
+git-check-ref-format $name || \
 	die "name contains invalid characters"

Perhaps you meant to say:

	git-check-ref-format "$name"

instead; after all you are dealing with potentially garbage
input from the user here.

While you are at it, you might want to also quote $_git/refs/tags
immediately follows the part that you patched, and there is
another.

-- >8 --
[PATCH] cg-tag: shell variable quoting.

Scripts sometimes tend to be loose in variable quoting, and
often they are justifiable (e.g. the variables are already
validated before used unquoted); but when checking the input, we
should try to be strict.

Signed-off-by: Junio C Hamano <redacted>
---

diff --git a/cg-tag b/cg-tag
index da4f2d5..1efb50d 100755
--- a/cg-tag
+++ b/cg-tag

@@ -28,7 +28,7 @@
 
 USAGE="cg-tag [-d DESCRIPTION] [-s [-k KEYNAME]] TAG_NAME [OBJECT_ID]"
 
-. ${COGITO_LIB}cg-Xlib || exit 1
+. "${COGITO_LIB}cg-Xlib" || exit 1
 
 sign=
 keyname=

@@ -54,10 +54,10 @@ id=$(cg-object-id -n "$id") || exit 1
 type=$(git-cat-file -t "$id")
 id=${id% *}
 
-(echo $name | egrep -qv '[^a-zA-Z0-9_.@!:-]') || \
-	die "name contains invalid characters"
+git-check-ref-format "$name" ||
+	die "name $name contains invalid characters"
 
-mkdir -p $_git/refs/tags
+mkdir -p "$_git/refs/tags"
 
 [ -s "$_git/refs/tags/$name" ] && die "tag already exists ($name)"
 [ "$id" ] || id="$(cat "$_git/$(git-symbolic-ref HEAD)")"

@@ -83,7 +83,7 @@ if [ "$sign" ]; then
 	fi
 	cat "$tagdir/tag.asc" >>"$tagdir/tag"
 fi
-if ! git-mktag <"$tagdir/tag" >$_git/refs/tags/$name; then
+if ! git-mktag <"$tagdir/tag" >"$_git/refs/tags/$name"; then
 	rm -rf "$tagdir"
 	die "error creating tag"
 fi

On Tuesday 13 December 2005 14:13, Junio C Hamano wrote:

Martin Atukunda [off-list ref] writes:

quoted

The egrep pattern used by cg-tag is too restrictive. While it will
prevent control characters from being specified as a tag name, it will
also reject nearly anything written in a non-English language, as noted
by -hpa ...
-(echo $name | egrep -qv '[^a-zA-Z0-9_.@!:-]') || \
+git-check-ref-format $name || \
 	die "name contains invalid characters"

Perhaps you meant to say:

	git-check-ref-format "$name"

Yes. i've just finished preparing a new patch with this exact change. But your 
patch is much better.

- Martin -

-- 
Due to a shortage of devoted followers, the production of great leaders has 
been discontinued.

Dear diary, on Tue, Dec 13, 2005 at 12:13:12PM CET, I got a letter
where Junio C Hamano [off-list ref] said that...

Martin Atukunda [off-list ref] writes:

quoted

The egrep pattern used by cg-tag is too restrictive. While it will prevent
control characters from being specified as a tag name, it will also reject
nearly anything written in a non-English language, as noted by -hpa
...
-(echo $name | egrep -qv '[^a-zA-Z0-9_.@!:-]') || \
+git-check-ref-format $name || \
 	die "name contains invalid characters"

Perhaps you meant to say:

	git-check-ref-format "$name"

instead; after all you are dealing with potentially garbage
input from the user here.

While you are at it, you might want to also quote $_git/refs/tags
immediately follows the part that you patched, and there is
another.

Thank you both for the patch, but I'd be much more comfortable if at
least quotes (both ' and "), backslashes, ? and * would be prohibited in
the names as well. Any chance of also implementing this policy upstream?
Taken to the extreme, using such a names for tags might be perceived as
a possible security vulnerability wrt. the less shell-savy users. ;-)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
VI has two modes: the one in which it beeps and the one in which
it doesn't.