{"thread":{"id":"2820","subject":"[PATCH] [COGITO] make cg-tag use git-check-ref-format","startedAt":"2005-12-13T10:54:51Z","lastAt":"2005-12-16T09:17:07Z","messageCount":9,"participants":["Martin Atukunda","Junio C Hamano","Petr Baudis","Alex Riesen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"13563","messageId":"11344712912199-git-send-email-matlads@dsmagic.com","threadId":"2820","inReplyTo":null,"subject":"[PATCH] [COGITO] make cg-tag use git-check-ref-format","fromName":"Martin Atukunda","fromEmail":"matlads@dsmagic.com","sentAt":"2005-12-13T10:54:51Z","receivedAt":"2005-12-13T10:54:51Z","isPatch":true,"sender":{"key":"matlads@dsmagic.com","avatar":null},"body":"The egrep pattern used by cg-tag is too restrictive. While it will prevent\ncontrol characters from being specified as a tag name, it will also reject\nnearly anything written in a non-English language, as noted by -hpa\n\nSigned-off-by: Martin Atukunda \n\n---\n\n cg-tag | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\n187670279068e7177149765a7f736cc565a1fe19\ndiff --git a/cg-tag b/cg-tag\nindex da4f2d5..73616b8 100755\n--- a/cg-tag\n+++ b/cg-tag\n@@ -54,7 +54,7 @@ id=$(cg-object-id -n \"$id\") || exit 1\n type=$(git-cat-file -t \"$id\")\n id=${id% *}\n \n-(echo $name | egrep -qv '[^a-zA-Z0-9_.@!:-]') || \\\n+git-check-ref-format $name || \\\n \tdie \"name contains invalid characters\"\n \n mkdir -p $_git/refs/tags\n-- \n0.99.9.GIT\n"},{"id":"13564","messageId":"7vy82p9rnb.fsf@assigned-by-dhcp.cox.net","threadId":"2820","inReplyTo":"11344712912199-git-send-email-matlads@dsmagic.com","subject":"Re: [PATCH] [COGITO] make cg-tag use git-check-ref-format","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-13T11:13:12Z","receivedAt":"2005-12-13T11:13:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Atukunda writes:\n\n> The egrep pattern used by cg-tag is too restrictive. While it will prevent\n> control characters from being specified as a tag name, it will also reject\n> nearly anything written in a non-English language, as noted by -hpa\n>...\n> -(echo $name | egrep -qv '[^a-zA-Z0-9_.@!:-]') || \\\n> +git-check-ref-format $name || \\\n> \tdie \"name contains invalid characters\"\n\nPerhaps you meant to say:\n\n\tgit-check-ref-format \"$name\"\n\ninstead; after all you are dealing with potentially garbage\ninput from the user here.\n\nWhile you are at it, you might want to also quote $_git/refs/tags\nimmediately follows the part that you patched, and there is\nanother.\n\n-- >8 --\n[PATCH] cg-tag: shell variable quoting.\n\nScripts sometimes tend to be loose in variable quoting, and\noften they are justifiable (e.g. the variables are already\nvalidated before used unquoted); but when checking the input, we\nshould try to be strict.\n\nSigned-off-by: Junio C Hamano \n---\n\ndiff --git a/cg-tag b/cg-tag\nindex da4f2d5..1efb50d 100755\n--- a/cg-tag\n+++ b/cg-tag\n@@ -28,7 +28,7 @@\n \n USAGE=\"cg-tag [-d DESCRIPTION] [-s [-k KEYNAME]] TAG_NAME [OBJECT_ID]\"\n \n-. ${COGITO_LIB}cg-Xlib || exit 1\n+. \"${COGITO_LIB}cg-Xlib\" || exit 1\n \n sign=\n keyname=\n@@ -54,10 +54,10 @@ id=$(cg-object-id -n \"$id\") || exit 1\n type=$(git-cat-file -t \"$id\")\n id=${id% *}\n \n-(echo $name | egrep -qv '[^a-zA-Z0-9_.@!:-]') || \\\n-\tdie \"name contains invalid characters\"\n+git-check-ref-format \"$name\" ||\n+\tdie \"name $name contains invalid characters\"\n \n-mkdir -p $_git/refs/tags\n+mkdir -p \"$_git/refs/tags\"\n \n [ -s \"$_git/refs/tags/$name\" ] && die \"tag already exists ($name)\"\n [ \"$id\" ] || id=\"$(cat \"$_git/$(git-symbolic-ref HEAD)\")\"\n@@ -83,7 +83,7 @@ if [ \"$sign\" ]; then\n \tfi\n \tcat \"$tagdir/tag.asc\" >>\"$tagdir/tag\"\n fi\n-if ! git-mktag <\"$tagdir/tag\" >$_git/refs/tags/$name; then\n+if ! git-mktag <\"$tagdir/tag\" >\"$_git/refs/tags/$name\"; then\n \trm -rf \"$tagdir\"\n \tdie \"error creating tag\"\n fi\n"},{"id":"13565","messageId":"200512131428.45273.matlads@dsmagic.com","threadId":"2820","inReplyTo":"7vy82p9rnb.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] [COGITO] make cg-tag use git-check-ref-format","fromName":"Martin Atukunda","fromEmail":"matlads@dsmagic.com","sentAt":"2005-12-13T11:28:45Z","receivedAt":"2005-12-13T11:28:45Z","isPatch":true,"sender":{"key":"matlads@dsmagic.com","avatar":null},"body":"On Tuesday 13 December 2005 14:13, Junio C Hamano wrote:\n> Martin Atukunda writes:\n> > The egrep pattern used by cg-tag is too restrictive. While it will\n> > prevent control characters from being specified as a tag name, it will\n> > also reject nearly anything written in a non-English language, as noted\n> > by -hpa ...\n> > -(echo $name | egrep -qv '[^a-zA-Z0-9_.@!:-]') || \\\n> > +git-check-ref-format $name || \\\n> > \tdie \"name contains invalid characters\"\n>\n> Perhaps you meant to say:\n>\n> \tgit-check-ref-format \"$name\"\n>\nYes. i've just finished preparing a new patch with this exact change. But your \npatch is much better.\n\n- Martin -\n\n-- \nDue to a shortage of devoted followers, the production of great leaders has \nbeen discontinued.\n"},{"id":"13569","messageId":"20051213170015.GD22159@pasky.or.cz","threadId":"2820","inReplyTo":"7vy82p9rnb.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] [COGITO] make cg-tag use git-check-ref-format","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2005-12-13T17:00:15Z","receivedAt":"2005-12-13T17:00:15Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Tue, Dec 13, 2005 at 12:13:12PM CET, I got a letter\nwhere Junio C Hamano said that...\n> Martin Atukunda writes:\n> \n> > The egrep pattern used by cg-tag is too restrictive. While it will prevent\n> > control characters from being specified as a tag name, it will also reject\n> > nearly anything written in a non-English language, as noted by -hpa\n> >...\n> > -(echo $name | egrep -qv '[^a-zA-Z0-9_.@!:-]') || \\\n> > +git-check-ref-format $name || \\\n> > \tdie \"name contains invalid characters\"\n> \n> Perhaps you meant to say:\n> \n> \tgit-check-ref-format \"$name\"\n> \n> instead; after all you are dealing with potentially garbage\n> input from the user here.\n> \n> While you are at it, you might want to also quote $_git/refs/tags\n> immediately follows the part that you patched, and there is\n> another.\n\nThank you both for the patch, but I'd be much more comfortable if at\nleast quotes (both ' and \"), backslashes, ? and * would be prohibited in\nthe names as well. Any chance of also implementing this policy upstream?\nTaken to the extreme, using such a names for tags might be perceived as\na possible security vulnerability wrt. the less shell-savy users. ;-)\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nVI has two modes: the one in which it beeps and the one in which\nit doesn't.\n"},{"id":"13573","messageId":"7vu0dcalgo.fsf@assigned-by-dhcp.cox.net","threadId":"2820","inReplyTo":"20051213170015.GD22159@pasky.or.cz","subject":"Re: [PATCH] [COGITO] make cg-tag use git-check-ref-format","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-13T18:41:27Z","receivedAt":"2005-12-13T18:41:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis writes:\n\n> Thank you both for the patch, but I'd be much more comfortable if at\n> least quotes (both ' and \"), backslashes, ? and * would be prohibited in\n> the names as well.\n\nI second that, and thanks for pointing it out. Any objections?\n"},{"id":"13701","messageId":"20051215222424.GA3094@steel.home","threadId":"2820","inReplyTo":"7vu0dcalgo.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] [COGITO] make cg-tag use git-check-ref-format","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-12-15T22:24:24Z","receivedAt":"2005-12-15T22:24:24Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Tue, Dec 13, 2005 19:41:27 +0100:\n> \n> > Thank you both for the patch, but I'd be much more comfortable if at\n> > least quotes (both ' and \"), backslashes, ? and * would be prohibited in\n> > the names as well.\n> \n> I second that, and thanks for pointing it out. Any objections?\n\nJust as a warning, perhaps? It's not like git is anywhere limited in\nthis respect...\n"},{"id":"13709","messageId":"7vacf2lyn4.fsf@assigned-by-dhcp.cox.net","threadId":"2820","inReplyTo":"20051215222424.GA3094@steel.home","subject":"Re: [PATCH] [COGITO] make cg-tag use git-check-ref-format","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-15T23:38:07Z","receivedAt":"2005-12-15T23:38:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> Junio C Hamano, Tue, Dec 13, 2005 19:41:27 +0100:\n>> \n>> > Thank you both for the patch, but I'd be much more comfortable if at\n>> > least quotes (both ' and \"), backslashes, ? and * would be prohibited in\n>> > the names as well.\n>> \n>> I second that, and thanks for pointing it out. Any objections?\n>\n> Just as a warning, perhaps? It's not like git is anywhere limited in\n> this respect...\n\nYeah, after thinking about it a bit more, I changed my mind.\n\nThe wildcard letters like ? and * I understand and sympathetic\nabout somewhat. Something like this:\n\n name=\"*.sh\" ;# this also comes from the end user\n echo $name\n\nends up showing every shell script in the current directory,\nand not literal '*.sh'.\n\nHowever, I do not think covering five characters '\"\\?* gives us\nanything, and sends a strong message that we do not know our\nshell programming to whoever is reading our code. For one\nthing, the user can still say \"foo[a-z]bar\" to confuse you, so\nyou also need to forbid [].\n\nThe thing is, if you start to care about single and double\nquotes, then what you are doing carelessly is not something\nsimple like this:\n\n\tname='frotz'\\''nitfol\"filfre\\xyzzy' ;# this comes from the end user.\n\techo $name ;# and this prints just fine.\n\nFor quotes to matter, you must be doing an \"eval\" carelessly,\nand \"eval\" and careless should never go together.\n\n # do not try this in your repository without echo\n\tname=\"foo; echo rm -fr .\"\n eval \"git-rev-parse $name\" \n\nYou end up needing to forbid a lot more than the quoting and\nwildcard, if you want to keep your shell scripts loose and lazy;\nwhich may be a worthy goal in itself but pretty much defeats the\ninitial discussion of \"why do we allow only these characters in\ntags\".\n\nSo in short, I am somewhat negative about the idea of adding\nmore \"forbidden letters\". Let's make sure our scripts are\ncareful where safety matters.\n\nNote that this does not forbid Porcelains to enforce additional\nrestrictions on their own.\n"},{"id":"13717","messageId":"7virtplr9s.fsf@assigned-by-dhcp.cox.net","threadId":"2820","inReplyTo":"7vacf2lyn4.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] [COGITO] make cg-tag use git-check-ref-format","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-16T02:17:19Z","receivedAt":"2005-12-16T02:17:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> The wildcard letters like ? and * I understand and sympathetic\n> about somewhat. Something like this:\n>\n> name=\"*.sh\" ;# this also comes from the end user\n> echo $name\n>\n> ends up showing every shell script in the current directory,\n> and not literal '*.sh'.\n\nSo why don't we do this?\n\n-- >8 --\nSubject: [PATCH] Forbid pattern maching characters in refnames.\n\nby marking '?', '*', and '[' as bad_ref_char().\n\nSigned-off-by: Junio C Hamano \n\n---\n\n Documentation/git-check-ref-format.txt | 8 +++++---\n refs.c | 4 +++-\n 2 files changed, 8 insertions(+), 4 deletions(-)\n\nd04ec25a77249095b6d2af5a08fe131351f2d86d\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex 636e951..f7f84c6 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -26,13 +26,15 @@ imposes the following rules on how refs \n \n . It cannot have ASCII control character (i.e. bytes whose\n values are lower than \\040, or \\177 `DEL`), space, tilde `~`,\n- caret `{caret}`, or colon `:` anywhere;\n+ caret `{caret}`, colon `:`, question-mark `?`, asterisk `*`,\n+ or open bracket `[` anywhere;\n \n . It cannot end with a slash `/`.\n \n These rules makes it easy for shell script based tools to parse\n-refnames, and also avoids ambiguities in certain refname\n-expressions (see gitlink:git-rev-parse[1]). Namely:\n+refnames, pathname expansion by the shell when a refname is used\n+unquoted (by mistake), and also avoids ambiguities in certain\n+refname expressions (see gitlink:git-rev-parse[1]). Namely:\n \n . double-dot `..` are often used as in `ref1..ref2`, and in some\n context this notation means `{caret}ref1 ref2` (i.e. not in\ndiff --git a/refs.c b/refs.c\nindex b8fcb98..0d63c1f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -313,7 +313,9 @@ int write_ref_sha1(const char *ref, int \n static inline int bad_ref_char(int ch)\n {\n \treturn (((unsigned) ch) <= ' ' ||\n-\t\tch == '~' || ch == '^' || ch == ':');\n+\t\tch == '~' || ch == '^' || ch == ':' ||\n+\t\t/* 2.13 Pattern Matching Notation */\n+\t\tch == '?' || ch == '*' || ch == '[');\n }\n \n int check_ref_format(const char *ref)\n-- \n0.99.9.GIT\n"},{"id":"13722","messageId":"20051216091707.GR22159@pasky.or.cz","threadId":"2820","inReplyTo":"7virtplr9s.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] [COGITO] make cg-tag use git-check-ref-format","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2005-12-16T09:17:07Z","receivedAt":"2005-12-16T09:17:07Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Fri, Dec 16, 2005 at 03:17:19AM CET, I got a letter\nwhere Junio C Hamano said that...\n> Junio C Hamano writes:\n> \n> > The wildcard letters like ? and * I understand and sympathetic\n> > about somewhat. Something like this:\n> >\n> > name=\"*.sh\" ;# this also comes from the end user\n> > echo $name\n> >\n> > ends up showing every shell script in the current directory,\n> > and not literal '*.sh'.\n> \n> So why don't we do this?\n\nI'm all for it, and now I also agree that forbidding \\'\" is pointless.\n\nThanks,\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nVI has two modes: the one in which it beeps and the one in which\nit doesn't.\n"}]}