Not going beyond symbolic links

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

Not going beyond symbolic links

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:06

Junio C Hamano [off-list ref] writes:
A symlink to us is just a different kind of blob, and by definition a blob
is the leaf level of a tree structure that represents the working tree in
the index. There won't be anything hanging below it, and when adding
things to the index we should not dereference the symlink to see where it
leads to.

Traditionally we have been loose about this check, and the normal "git
add" and "git update-index" codepath is still forever broken, and we
allow:

      $ mkdir dir
      $ >dir/file
      $ ln -s dir symlink
      $ git add symlink/file

but some codepaths that matter more (because they do larger damage
unattended, as opposed to the above command sequence that can be avoided
by user education a bit more easily), such as "git apply" and "git
read-tree", have been corrected using has_symlink_leading_path() since mid
2007.  We would need to follow through c40641b (Optimize symlink/directory
detection, 2008-05-09) and further fix "git add" and "git update-index"
codepaths to forbid the above command sequence.
I started to revisit this issue and patched "git update-index --add"
and "git add" so far.  Patches follow.

I think we should also check the following and fix them if any of them is
broken.  Under the same "dir/file exists but symlink points at dir"
scenario:

 * "git rm symlink/file" --- should fail without removing dir/file;

 * "git ls-tree $commit -- symlink/file" should *not* fail if $commit does
   have "symlink/file" in it (iow, we cannot add the logic to get_pathspec());

 * "git ls-files --exclude-standard -o -- symlink/file" should not talk
   about "symlink/file".

If we reword the paragraph I quoted at the beginning of this message
slightly:

	A gitlink to is is just a leaf level of a tree structure that
	represents the working tree in the index.  There won't be anything
	hanging below it.

We would need a similar check to stop at module boundary, just like the
helper function we use for these patches, has_symlink_leading_path(),
stops at a symlink.

So without further ado...

Re: Not going beyond symbolic links

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:45:06


On Mon, 4 Aug 2008, Junio C Hamano wrote:
I started to revisit this issue and patched "git update-index --add"
and "git add" so far.  Patches follow.
Patches look good to me, but did you check the performance impact?

The rewritten 'has_symlink_leading_path()' should do ok, but it migth 
still be a huge performance downside to check all the paths for things 
like "git add -u".

I didn't test, though.

		Linus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help