allow only a strictly defined set of references under .git/

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

allow only a strictly defined set of references under .git/

From: Alex Riesen <hidden>
Date: 2016-06-15 22:42:15

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

So, 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").

Does it make sense or have I missed some very informative discussion
as to why we do allow references to be anywhere in .git?

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.

Re: allow only a strictly defined set of references under .git/

From: Alex Riesen <hidden>
Date: 2016-06-15 22:42:15

Junio C Hamano, Fri, Dec 16, 2005 19:01:20 +0100:
quoted
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.
I actually run into it...
Probably post 1.0 --- I have not assessed the extent of
confusion yet.
It has good chances to continue to grow, as git gets more exposure.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help