Re: allow only a strictly defined set of references under .git/
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:15
Alex Riesen [off-list ref] writes:
The patch below reminded me of something:
+ /* We want to allow .git/description file and
+ * "description" branch to exist at the same time.
+ * "git-rev-parse description" should silently skip
+ * .git/description file as a candidate for
+ * get_sha1(). However, having garbage file anywhere
+ * under refs/ is not OK, and we would not have caught
+ * ambiguous heads and tags with the above test.
+ */
+ else if (**p && !access(pathname, F_OK)) {
+ /* Garbage exists under .git/refs */
+ return error("garbage ref found '%s'", pathname);
+ }
Maybe 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).I am glad somebody is paying attention. The ref-to-object-name loop in get_sha1_basic() gets what the user gave us, first checks it immediately under .git/ and then under .git/refs, .git/refs/tags, .git/refs/heads, in this order. Originally it grabbed the first match. For the last couple of weeks, it has been made "extra careful" to detect a case where both .git/refs/heads/foo and .git/refs/tags/foo exists and to reject unadorned "foo" with complaints. I have been nagged by a suspition that the whole disambiguation business should not be there [*1*]. I think the original semantics of getting the first match is as easy to explain, if not easier, as the current one, and I suspect that we do not have to worry about .git/refs/heads/HEAD if we did so. If somebody is curious enough to name a branch "HEAD", the only thing she should be aware of is this search order, and wherever "branchname" is called for she should be able to say "HEAD" to mean .git/refs/heads/HEAD, but places that call for an arbitrary object name, she needs to disambiguate by saying "heads/HEAD" or even "refs/heads/HEAD". I suspect that git barebone Porcelain-ish carelessly uses "git-rev-parse --verify $branchname" when it really means "git-rev-parse --verify refs/heads/$branchname" in some places [*2*], but if we fix them, we may not even need to have the "we do not like HEAD" patch by Johannes (which has already been merged). Probably post 1.0 --- I have not assessed the extent of confusion yet. [Footnote] *1* It was not in Linus version, and I consider anything I added recently is of suspecious design value. *2* That is, where we advertise that we get branchname as parameter to the command.