{"thread":{"id":"2860","subject":"allow only a strictly defined set of references under .git/","startedAt":"2005-12-16T08:24:35Z","lastAt":"2005-12-16T20:44:27Z","messageCount":3,"participants":["Alex Riesen","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"13721","messageId":"81b0412b0512160024q47afe0b9v1b7f217a6cecfe68@mail.gmail.com","threadId":"2860","inReplyTo":null,"subject":"allow only a strictly defined set of references under .git/","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-12-16T08:24:35Z","receivedAt":"2005-12-16T08:24:35Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"The patch below reminded me of something:\n\n  +               /* We want to allow .git/description file and\n  +                * \"description\" branch to exist at the same time.\n  +                * \"git-rev-parse description\" should silently skip\n  +                * .git/description file as a candidate for\n  +                * get_sha1().  However, having garbage file anywhere\n  +                * under refs/ is not OK, and we would not have caught\n  +                * ambiguous heads and tags with the above test.\n  +                */\n  +               else if (**p && !access(pathname, F_OK)) {\n  +                       /* Garbage exists under .git/refs */\n  +                       return error(\"garbage ref found '%s'\", pathname);\n  +               }\n\nMaybe we should only allow only a strictly defined set of refnames\nunder $GIT_DIR? The directory is used pretty much for anything else\n(temporary message files, config, description).\n\nSo, if we, say, allow only HEAD, ORIG_HEAD, and MERGE_HEAD to be\nreferences in .git/ and ignore everything else. Then there will be no\nambiguity anymore and no need to parse \"description\" or \"config\"\nreference (besides, I can very simply imagine a message file\ncontaining \"ref: refs/heads/foo\").\n\nDoes it make sense or have I missed some very informative discussion\nas to why we do allow references to be anywhere in .git?\n\n\nSubject: [PATCH] The patch below reminded me of something:\n\n  +               /* We want to allow .git/description file and\n  +                * \"description\" branch to exist at the same time.\n  +                * \"git-rev-parse description\" should silently skip\n  +                * .git/description file as a candidate for\n  +                * get_sha1().  However, having garbage file anywhere\n  +                * under refs/ is not OK, and we would not have caught\n  +                * ambiguous heads and tags with the above test.\n  +                */\n  +               else if (**p && !access(pathname, F_OK)) {\n  +                       /* Garbage exists under .git/refs */\n  +                       return error(\"garbage ref found '%s'\", pathname);\n  +               }\n\nMaybe we should only allow only a strictly defined set of refnames under $GIT_DIR? The directory is used pretty much for anything else (temporary message files, config, description).\n\nSo, if we, say, allow only HEAD, ORIG_HEAD, and MERGE_HEAD to be references in .git/ and ignore everything else. Then there will be no ambiguity anymore and no need to parse \"description\" or \"config\" reference (besides, I can very simply imagine a message file containing \"ref: refs/heads/foo\").\n\nDoes it make sense or have I missed some very informative discussion as to why we do allow references to be anywhere in .git?\n\n---\n\n sha1_name.c |   22 +++++++++++++++++++++-\n 1 files changed, 21 insertions(+), 1 deletions(-)\n\n4a0ef1ed775d2311da7125257cb734d561fd2ef1\ndiff --git a/sha1_name.c b/sha1_name.c\nindex bf8f0f0..dce1574 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -247,7 +247,27 @@ static int get_sha1_basic(const char *st\n \tif (ambiguous_path(str, len))\n \t\treturn -1;\n \n-\tfor (p = prefix; *p; p++) {\n+\tp = prefix;\n+\t/* Check references _without_ leading directories */\n+\tif (!memchr(str, '/', len)) {\n+\t\t/* only these references can be directly in $GIT_DIR... */\n+\t\tstatic const char *git_refs[] = {\n+\t\t\t\"HEAD\",\n+\t\t\t\"ORIG_HEAD\",\n+\t\t\t\"FETCH_HEAD\",\n+\t\t\t\"MERGE_HEAD\",\n+\t\t\tNULL\n+\t\t};\n+\t\tconst char **r;\n+\t\tfor (r = git_refs; *r; ++r)\n+\t\t\tif (strlen(*r) == len && strncmp(*r, str, len) == 0)\n+\t\t\t\tbreak;\n+\t\t/* ...all the others have to be somewhere under .git/refs/ */\n+\t\tif ( !*r )\n+\t\t\t++p;\n+\t}\n+\n+\tfor (; *p; p++) {\n \t\tchar *pathname = git_path(\"%s/%.*s\", *p, len, str);\n \n \t\tif (!read_ref(pathname, sha1)) {\n-- \n0.99.9.GIT\n"},{"id":"13731","messageId":"7v1x0cj4zz.fsf@assigned-by-dhcp.cox.net","threadId":"2860","inReplyTo":"81b0412b0512160024q47afe0b9v1b7f217a6cecfe68@mail.gmail.com","subject":"Re: allow only a strictly defined set of references under .git/","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-12-16T18:01:20Z","receivedAt":"2005-12-16T18:01:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> The patch below reminded me of something:\n>\n>   +               /* We want to allow .git/description file and\n>   +                * \"description\" branch to exist at the same time.\n>   +                * \"git-rev-parse description\" should silently skip\n>   +                * .git/description file as a candidate for\n>   +                * get_sha1().  However, having garbage file anywhere\n>   +                * under refs/ is not OK, and we would not have caught\n>   +                * ambiguous heads and tags with the above test.\n>   +                */\n>   +               else if (**p && !access(pathname, F_OK)) {\n>   +                       /* Garbage exists under .git/refs */\n>   +                       return error(\"garbage ref found '%s'\", pathname);\n>   +               }\n>\n> Maybe we should only allow only a strictly defined set of refnames\n> under $GIT_DIR? The directory is used pretty much for anything else\n> (temporary message files, config, description).\n\nI am glad somebody is paying attention.\n\nThe ref-to-object-name loop in get_sha1_basic() gets what the\nuser gave us, first checks it immediately under .git/ and then\nunder .git/refs, .git/refs/tags, .git/refs/heads, in this order.\nOriginally it grabbed the first match.  For the last couple of\nweeks, it has been made \"extra careful\" to detect a case where\nboth .git/refs/heads/foo and .git/refs/tags/foo exists and\nto reject unadorned \"foo\" with complaints.\n\nI have been nagged by a suspition that the whole disambiguation\nbusiness should not be there [*1*].  I think the original\nsemantics of getting the first match is as easy to explain, if\nnot easier, as the current one, and I suspect that we do not\nhave to worry about .git/refs/heads/HEAD if we did so.  If\nsomebody is curious enough to name a branch \"HEAD\", the only\nthing she should be aware of is this search order, and wherever\n\"branchname\" is called for she should be able to say \"HEAD\" to\nmean .git/refs/heads/HEAD, but places that call for an arbitrary\nobject name, she needs to disambiguate by saying \"heads/HEAD\" or\neven \"refs/heads/HEAD\".\n\nI suspect that git barebone Porcelain-ish carelessly uses\n\"git-rev-parse --verify $branchname\" when it really means\n\"git-rev-parse --verify refs/heads/$branchname\" in some places\n[*2*], but if we fix them, we may not even need to have the \"we\ndo not like HEAD\" patch by Johannes (which has already been\nmerged).\n\nProbably post 1.0 --- I have not assessed the extent of\nconfusion yet.\n\n\n[Footnote]\n\n*1* It was not in Linus version, and I consider anything I added\nrecently is of suspecious design value.\n\n*2* That is, where we advertise that we get branchname as\nparameter to the command.\n"},{"id":"13737","messageId":"20051216204427.GA4164@steel.home","threadId":"2860","inReplyTo":"7v1x0cj4zz.fsf@assigned-by-dhcp.cox.net","subject":"Re: allow only a strictly defined set of references under .git/","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2005-12-16T20:44:27Z","receivedAt":"2005-12-16T20:44:27Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Fri, Dec 16, 2005 19:01:20 +0100:\n> > Maybe we should only allow only a strictly defined set of refnames\n> > under $GIT_DIR? The directory is used pretty much for anything else\n> > (temporary message files, config, description).\n> \n> I am glad somebody is paying attention.\n> \n\nI actually run into it...\n\n> Probably post 1.0 --- I have not assessed the extent of\n> confusion yet.\n\nIt has good chances to continue to grow, as git gets more exposure.\n"}]}