Re: Troubles when directory is replaced by symlink

[Kjetil Barvik <barvik@broadpark.no>]

From: Kjetil Barvik <barvik@broadpark.no>
Date: Sun, 14 Jun 2009 15:08:28 +0200
Subject: [PATCH] lstat_cache: guard against full match of length of 'name' parameter

longest_path_match() in symlinks.c does exactly what it's name says,
but in some cases that match can be too long, since the
has_*_leading_path() functions assumes that the match will newer be as
long as the name string given to the function.

fix this by adding an extra if test which checks if the match length
is equal to the 'len' parameter.
---

  OK, this is a first version of a patch for this bug.  I have not added
  a "Signed-off-by:"-tag yet, but I intend to do that when I am more
  shure that this is the real fix for the bug.  Will also add a test
  case in a later version.

  The given instructions(1) to repeat the failure, gave me the following
  4 calls to has_symlink_leading_path() for the failed 'git checkout
  master' command, and I made the following the table:

   'name' parameter       Kjetil      Linus      tested
  -------------------------------------------------------
   "beta/alpha/file"        2           1           2
   "beta/alpha"             2C          0           0
   "alpha/file"             0           0           0
   "beta/alpha/file"        0           1C          0

  The "Kjetil" column is what the h_s_l_p() function returns as of
  commit 92604b46 by me.  The "Linus" column is what the h_s_l_p()
  function returns as of commit c40641b7 by Linus.  The "tested" column
  is what will be returned when the cache is clear'ed each time.  An
  added "C" indicate that the result was cached/taken from the cache.

  My bug is from the incorrect cached value from line 2, and since I
  noticed that it take the symbolic link result, "beta/alpha", which was
  the full length of the 'name' parameter and not the leading part, I
  have based the included patch on this fact.

  The function is named has_symlink_leading_path(), so I guess it is a
  reason for that "leading" part of the name.  So, is this a reasonable
  fix?  Other comments?

  -- kjetil

  1) the following commands:

# make a repo
mkdir repo && cd repo && git init

# content in alpha
mkdir alpha && echo content >alpha/file && git add . && git commit -m one
# and duplicate content inside beta
mkdir beta && cp -R alpha beta && git add . && git commit -m two

# now replace the duplicate with a symlink
git checkout -b branch
rm -rf beta/alpha && git add -u && git commit -m deleted
ln -s ../alpha beta/alpha && git add . && git commit -m symlink

# now checkout master again; alpha/file will be missing, even
# though it wasn't touched at all
git checkout master



 symlinks.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/symlinks.c b/symlinks.c
index 8dcd632..934abc6 100644
--- a/symlinks.c
+++ b/symlinks.c
@@ -97,6 +97,10 @@ static int lstat_cache(const char *name, int len,
 			longest_path_match(name, len, cache.path, cache.len,
 					   &previous_slash);
 		match_flags = cache.flags & track_flags & (FL_NOENT|FL_SYMLINK);
+
+		if (!(track_flags & FL_FULLPATH) && match_len == len)
+			match_len = last_slash = previous_slash;
+
 		if (match_flags && match_len == cache.len)
 			return match_flags;
 		/*
-- 
1.6.3.2.277.gd10543