From: Jim Meyering <jim@meyering.net>
Date: Mon, 24 Sep 2007 11:39:37 +0000 (+0200)
Subject: canonicalize: Avoid a false-positive cycle failure.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cbbc0892dc8607978ec55acdd8ef3a124df1ca1f;p=pspp

canonicalize: Avoid a false-positive cycle failure.

* modules/canonicalize (Depends-on): Add file-set and hash-triple.  Sort.
Remove cycle-check.
* lib/canonicalize.c: Include file-set.h and hash-triple.h, not cycle-check.h.
(seen_triple): New function.
(canonicalize_filename_mode): Use it instead of cycle-check.
* tests/test-canonicalize.c: Add a test for this bug.
* tests/test-canonicalize.sh: Set up and run the test.
---

diff --git a/ChangeLog b/ChangeLog
index 7b96aaf313..8206a24d0b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2007-09-27  Jim Meyering  <jim@meyering.net>
 
+	canonicalize: Avoid a false-positive cycle failure.
+	* modules/canonicalize (Depends-on): Add file-set and hash-triple.
+	Sort.  Remove cycle-check.
+	* lib/canonicalize.c: Include file-set.h and hash-triple.h,
+	not cycle-check.h.
+	(seen_triple): New function.
+	(canonicalize_filename_mode): Use it instead of cycle-check.
+	* tests/test-canonicalize.c: Add a test for this bug.
+	* tests/test-canonicalize.sh: Set up and run the test.
+
 	New module, file-set.
 	* modules/file-set: Define it.
 	* lib/file-set.c, lib/file-set.h: Implement.
diff --git a/lib/canonicalize.c b/lib/canonicalize.c
index fa2c1ab944..af2703cf7a 100644
--- a/lib/canonicalize.c
+++ b/lib/canonicalize.c
@@ -34,8 +34,9 @@
 #include <errno.h>
 #include <stddef.h>
 
-#include "cycle-check.h"
+#include "file-set.h"
 #include "filenamecat.h"
+#include "hash-triple.h"
 #include "xalloc.h"
 #include "xgetcwd.h"
 
@@ -123,6 +124,30 @@ canonicalize_file_name (const char *name)
 }
 #endif /* !HAVE_CANONICALIZE_FILE_NAME */
 
+/* Return true if we've already seen the triple, <FILENAME, dev, ino>.
+   If *HT is not initialized, initialize it.  */
+static bool
+seen_triple (Hash_table **ht, char const *filename, struct stat const *st)
+{
+  if (*ht == NULL)
+    {
+      size_t initial_capacity = 7;
+      *ht = hash_initialize (initial_capacity,
+			    NULL,
+			    triple_hash,
+			    triple_compare,
+			    triple_free);
+      if (*ht == NULL)
+	xalloc_die ();
+    }
+
+  if (seen_file (*ht, filename, st))
+    return true;
+
+  record_file (*ht, filename, st);
+  return false;
+}
+
 /* Return the canonical absolute name of file NAME.  A canonical name
    does not contain any `.', `..' components nor any repeated file name
    separators ('/') or symlinks.  Whether components must exist
@@ -136,7 +161,7 @@ canonicalize_filename_mode (const char *name, canonicalize_mode_t can_mode)
   char const *end;
   char const *rname_limit;
   size_t extra_len = 0;
-  struct cycle_check_state cycle_state;
+  Hash_table *ht = NULL;
 
   if (name == NULL)
     {
@@ -176,7 +201,6 @@ canonicalize_filename_mode (const char *name, canonicalize_mode_t can_mode)
       dest = rname + 1;
     }
 
-  cycle_check_init (&cycle_state);
   for (start = end = name; *start; start = end)
     {
       /* Skip sequence of multiple file name separators.  */
@@ -237,7 +261,11 @@ canonicalize_filename_mode (const char *name, canonicalize_mode_t can_mode)
 	      char *buf;
 	      size_t n, len;
 
-	      if (cycle_check (&cycle_state, &st))
+	      /* Detect loops.  We cannot use the cycle-check module here,
+		 since it's actually possible to encounter the same symlink
+		 more than once in a given traversal.  However, encountering
+		 the same symlink,NAME pair twice does indicate a loop.  */
+	      if (seen_triple (&ht, name, &st))
 		{
 		  __set_errno (ELOOP);
 		  if (can_mode == CAN_MISSING)
@@ -298,10 +326,14 @@ canonicalize_filename_mode (const char *name, canonicalize_mode_t can_mode)
   *dest = '\0';
 
   free (extra_buf);
+  if (ht)
+    hash_free (ht);
   return rname;
 
 error:
   free (extra_buf);
   free (rname);
+  if (ht)
+    hash_free (ht);
   return NULL;
 }
diff --git a/modules/canonicalize b/modules/canonicalize
index 52e80fbcbe..4c6916d857 100644
--- a/modules/canonicalize
+++ b/modules/canonicalize
@@ -8,12 +8,13 @@ lib/pathmax.h
 m4/canonicalize.m4
 
 Depends-on:
-cycle-check
+areadlink-with-size
+file-set
 filenamecat
+hash-triple
 sys_stat
 xalloc
 xgetcwd
-areadlink-with-size
 
 configure.ac:
 AC_FUNC_CANONICALIZE_FILE_NAME
diff --git a/tests/test-canonicalize.c b/tests/test-canonicalize.c
index 8003743320..aa20a7c7cb 100644
--- a/tests/test-canonicalize.c
+++ b/tests/test-canonicalize.c
@@ -127,5 +127,17 @@ main ()
     free (result2);
   }
 
+  /* Ensure that the following is resolved properly.
+     Before 2007-09-27, it would mistakenly report a loop.  */
+  {
+    char *result1 = canonicalize_filename_mode ("t-can.tmp", CAN_EXISTING);
+    char *result2 = canonicalize_filename_mode ("t-can.tmp/p/1", CAN_EXISTING);
+    ASSERT (result1 != NULL);
+    ASSERT (result2 != NULL);
+    ASSERT (strcmp (result2 + strlen (result1), "/d/2") == 0);
+    free (result1);
+    free (result2);
+  }
+
   return 0;
 }
diff --git a/tests/test-canonicalize.sh b/tests/test-canonicalize.sh
index 2992e5faf6..0ef91f308e 100755
--- a/tests/test-canonicalize.sh
+++ b/tests/test-canonicalize.sh
@@ -20,6 +20,19 @@ ln -s t-can.tmp/ket ise \
  && mkdir lum
 ) || exit 1
 
+# Trigger a bug that would make the function mistakenly report a loop.
+# To trigger it, we have to construct a name/situation during the
+# resolution of which the code dereferences the same symlink (S)
+# two different times with no actual loop.  In addition, the
+# second and fourth calls to readlink must operate on S.
+(cd t-can.tmp \
+ && ln -s s p \
+ && ln -s d s \
+ && mkdir d \
+ && echo > d/2 \
+ && ln -s ../s/2 d/1
+) || exit 1
+
 ./test-canonicalize${EXEEXT}
 result=$?