dart:io | Ensure that Directory.list terminates even when symbolic links create loops in the file system.

BUG=

Review URL: https://codereview.chromium.org//13818010

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@21199 260f80e4-7a28-3924-810f-c04153c831b5

runtime/bin/directory_android.cc

@@ -54,10 +54,20 @@ class PathBuffer {
 };
 
 
+// A linked list of symbolic links, with their unique file system identifiers.
+// These are scanned to detect loops while doing a recursive directory listing.
+struct LinkList {
+  dev_t dev;
+  ino_t ino;
+  LinkList* next;
+};
+
+
 // Forward declarations.
 static bool ListRecursively(PathBuffer* path,
                             bool recursive,
                             bool follow_links,
+                            LinkList* seen,
                             DirectoryListing* listing);
 static bool DeleteRecursively(PathBuffer* path);
 
@@ -72,6 +82,7 @@ static bool HandleDir(char* dir_name,
                       PathBuffer* path,
                       bool recursive,
                       bool follow_links,
+                      LinkList* seen,
                       DirectoryListing *listing) {
   if (strcmp(dir_name, ".") == 0) return true;
   if (strcmp(dir_name, "..") == 0) return true;
@@ -80,7 +91,8 @@ static bool HandleDir(char* dir_name,
     return false;
   }
   return listing->HandleDirectory(path->data) &&
-      (!recursive || ListRecursively(path, recursive, follow_links, listing));
+      (!recursive ||
+       ListRecursively(path, recursive, follow_links, seen, listing));
 }
 
 
@@ -109,6 +121,7 @@ static bool HandleLink(char* link_name,
 static bool ListRecursively(PathBuffer* path,
                             bool recursive,
                             bool follow_links,
+                            LinkList* seen,
                             DirectoryListing *listing) {
   if (!path->Add(File::PathSeparator())) {
     PostError(listing, path->data);
@@ -167,19 +180,56 @@ static bool ListRecursively(PathBuffer* path,
           break;
         }
         int stat_success;
-        if (follow_links) {
-          stat_success = TEMP_FAILURE_RETRY(stat(path->data, &entry_info));
-          if (stat_success == -1) {
-            stat_success = TEMP_FAILURE_RETRY(lstat(path->data, &entry_info));
-          }
-        } else {
-          stat_success = TEMP_FAILURE_RETRY(lstat(path->data, &entry_info));
-        }
+        stat_success = TEMP_FAILURE_RETRY(lstat(path->data, &entry_info));
         if (stat_success == -1) {
           success = false;
           PostError(listing, path->data);
           break;
         }
+        if (follow_links && S_ISLNK(entry_info.st_mode)) {
+          // Check to see if we are in a loop created by a symbolic link.
+          LinkList current_link = { entry_info.st_dev,
+                                    entry_info.st_ino,
+                                    seen };
+          LinkList* previous = seen;
+          bool looping_link = false;
+          while (previous != NULL) {
+            if (previous->dev == current_link.dev &&
+                previous->ino == current_link.ino) {
+              // Report the looping link as a link, rather than following it.
+              path->Reset(path_length);
+              success = HandleLink(entry.d_name,
+                                   path,
+                                   listing) && success;
+              looping_link = true;
+              break;
+            }
+            previous = previous->next;
+          }
+          if (looping_link) break;
+          stat_success = TEMP_FAILURE_RETRY(stat(path->data, &entry_info));
+          if (stat_success == -1) {
+            // Report a broken link as a link, even if follow_links is true.
+            path->Reset(path_length);
+            success = HandleLink(entry.d_name,
+                                 path,
+                                 listing) && success;
+            break;
+          }
+          if (S_ISDIR(entry_info.st_mode)) {
+            // Recurse into the subdirectory with current_link added to the
+            // linked list of seen file system links.
+            path->Reset(path_length);
+            success = HandleDir(entry.d_name,
+                                path,
+                                recursive,
+                                follow_links,
+                                seen,
+                                &current_link,
+                                listing) && success;
+            break;
+          }
+        }
         path->Reset(path_length);
         if (S_ISDIR(entry_info.st_mode)) {
           success = HandleDir(entry.d_name,
@@ -327,7 +377,7 @@ bool Directory::List(const char* dir_name,
     PostError(listing, dir_name);
     return false;
   }
-  return ListRecursively(&path, recursive, follow_links, listing);
+  return ListRecursively(&path, recursive, follow_links, NULL, listing);
 }
 
 

runtime/bin/directory_linux.cc

@@ -54,10 +54,20 @@ class PathBuffer {
 };
 
 
+// A linked list of symbolic links, with their unique file system identifiers.
+// These are scanned to detect loops while doing a recursive directory listing.
+struct LinkList {
+  dev_t dev;
+  ino_t ino;
+  LinkList* next;
+};
+
+
 // Forward declarations.
 static bool ListRecursively(PathBuffer* path,
                             bool recursive,
                             bool follow_links,
+                            LinkList* seen,
                             DirectoryListing* listing);
 static bool DeleteRecursively(PathBuffer* path);
 
@@ -72,6 +82,7 @@ static bool HandleDir(char* dir_name,
                       PathBuffer* path,
                       bool recursive,
                       bool follow_links,
+                      LinkList* seen,
                       DirectoryListing *listing) {
   if (strcmp(dir_name, ".") == 0) return true;
   if (strcmp(dir_name, "..") == 0) return true;
@@ -80,7 +91,8 @@ static bool HandleDir(char* dir_name,
     return false;
   }
   return listing->HandleDirectory(path->data) &&
-      (!recursive || ListRecursively(path, recursive, follow_links, listing));
+      (!recursive ||
+       ListRecursively(path, recursive, follow_links, seen, listing));
 }
 
 
@@ -109,6 +121,7 @@ static bool HandleLink(char* link_name,
 static bool ListRecursively(PathBuffer* path,
                             bool recursive,
                             bool follow_links,
+                            LinkList* seen,
                             DirectoryListing *listing) {
   if (!path->Add(File::PathSeparator())) {
     PostError(listing, path->data);
@@ -140,6 +153,7 @@ static bool ListRecursively(PathBuffer* path,
                             path,
                             recursive,
                             follow_links,
+                            seen,
                             listing) && success;
         break;
       case DT_REG:
@@ -167,25 +181,62 @@ static bool ListRecursively(PathBuffer* path,
           break;
         }
         int stat_success;
-        if (follow_links) {
-          stat_success = TEMP_FAILURE_RETRY(stat(path->data, &entry_info));
-          if (stat_success == -1) {
-            stat_success = TEMP_FAILURE_RETRY(lstat(path->data, &entry_info));
-          }
-        } else {
-          stat_success = TEMP_FAILURE_RETRY(lstat(path->data, &entry_info));
-        }
+        stat_success = TEMP_FAILURE_RETRY(lstat(path->data, &entry_info));
         if (stat_success == -1) {
           success = false;
           PostError(listing, path->data);
           break;
         }
+        if (follow_links && S_ISLNK(entry_info.st_mode)) {
+          // Check to see if we are in a loop created by a symbolic link.
+          LinkList current_link = { entry_info.st_dev,
+                                    entry_info.st_ino,
+                                    seen };
+          LinkList* previous = seen;
+          bool looping_link = false;
+          while (previous != NULL) {
+            if (previous->dev == current_link.dev &&
+                previous->ino == current_link.ino) {
+              // Report the looping link as a link, rather than following it.
+              path->Reset(path_length);
+              success = HandleLink(entry.d_name,
+                                   path,
+                                   listing) && success;
+              looping_link = true;
+              break;
+            }
+            previous = previous->next;
+          }
+          if (looping_link) break;
+          stat_success = TEMP_FAILURE_RETRY(stat(path->data, &entry_info));
+          if (stat_success == -1) {
+            // Report a broken link as a link, even if follow_links is true.
+            path->Reset(path_length);
+            success = HandleLink(entry.d_name,
+                                 path,
+                                 listing) && success;
+            break;
+          }
+          if (S_ISDIR(entry_info.st_mode)) {
+            // Recurse into the subdirectory with current_link added to the
+            // linked list of seen file system links.
+            path->Reset(path_length);
+            success = HandleDir(entry.d_name,
+                                path,
+                                recursive,
+                                follow_links,
+                                &current_link,
+                                listing) && success;
+            break;
+          }
+        }
         path->Reset(path_length);
         if (S_ISDIR(entry_info.st_mode)) {
           success = HandleDir(entry.d_name,
                               path,
                               recursive,
                               follow_links,
+                              seen,
                               listing) && success;
         } else if (S_ISREG(entry_info.st_mode)) {
           success = HandleFile(entry.d_name,
@@ -327,7 +378,7 @@ bool Directory::List(const char* dir_name,
     PostError(listing, dir_name);
     return false;
   }
-  return ListRecursively(&path, recursive, follow_links, listing);
+  return ListRecursively(&path, recursive, follow_links, NULL, listing);
 }
 
 

runtime/bin/directory_macos.cc

@@ -54,10 +54,20 @@ class PathBuffer {
 };
 
 
+// A linked list of symbolic links, with their unique file system identifiers.
+// These are scanned to detect loops while doing a recursive directory listing.
+struct LinkList {
+  dev_t dev;
+  ino_t ino;
+  LinkList* next;
+};
+
+
 // Forward declarations.
 static bool ListRecursively(PathBuffer* path,
                             bool recursive,
                             bool follow_links,
+                            LinkList* seen,
                             DirectoryListing* listing);
 static bool DeleteRecursively(PathBuffer* path);
 
@@ -72,6 +82,7 @@ static bool HandleDir(char* dir_name,
                       PathBuffer* path,
                       bool recursive,
                       bool follow_links,
+                      LinkList* seen,
                       DirectoryListing *listing) {
   if (strcmp(dir_name, ".") == 0) return true;
   if (strcmp(dir_name, "..") == 0) return true;
@@ -80,7 +91,8 @@ static bool HandleDir(char* dir_name,
     return false;
   }
   return listing->HandleDirectory(path->data) &&
-      (!recursive || ListRecursively(path, recursive, follow_links, listing));
+      (!recursive ||
+       ListRecursively(path, recursive, follow_links, seen, listing));
 }
 
 
@@ -109,6 +121,7 @@ static bool HandleLink(char* link_name,
 static bool ListRecursively(PathBuffer* path,
                             bool recursive,
                             bool follow_links,
+                            LinkList* seen,
                             DirectoryListing *listing) {
   if (!path->Add(File::PathSeparator())) {
     PostError(listing, path->data);
@@ -167,25 +180,62 @@ static bool ListRecursively(PathBuffer* path,
           break;
         }
         int stat_success;
-        if (follow_links) {
-          stat_success = TEMP_FAILURE_RETRY(stat(path->data, &entry_info));
-          if (stat_success == -1) {
-            stat_success = TEMP_FAILURE_RETRY(lstat(path->data, &entry_info));
-          }
-        } else {
-          stat_success = TEMP_FAILURE_RETRY(lstat(path->data, &entry_info));
-        }
+        stat_success = TEMP_FAILURE_RETRY(lstat(path->data, &entry_info));
         if (stat_success == -1) {
           success = false;
           PostError(listing, path->data);
           break;
         }
+        if (follow_links && S_ISLNK(entry_info.st_mode)) {
+          // Check to see if we are in a loop created by a symbolic link.
+          LinkList current_link = { entry_info.st_dev,
+                                    entry_info.st_ino,
+                                    seen };
+          LinkList* previous = seen;
+          bool looping_link = false;
+          while (previous != NULL) {
+            if (previous->dev == current_link.dev &&
+                previous->ino == current_link.ino) {
+              // Report the looping link as a link, rather than following it.
+              path->Reset(path_length);
+              success = HandleLink(entry.d_name,
+                                   path,
+                                   listing) && success;
+              looping_link = true;
+              break;
+            }
+            previous = previous->next;
+          }
+          if (looping_link) break;
+          stat_success = TEMP_FAILURE_RETRY(stat(path->data, &entry_info));
+          if (stat_success == -1) {
+            // Report a broken link as a link, even if follow_links is true.
+            path->Reset(path_length);
+            success = HandleLink(entry.d_name,
+                                 path,
+                                 listing) && success;
+            break;
+          }
+          if (S_ISDIR(entry_info.st_mode)) {
+            // Recurse into the subdirectory with current_link added to the
+            // linked list of seen file system links.
+            path->Reset(path_length);
+            success = HandleDir(entry.d_name,
+                                path,
+                                recursive,
+                                follow_links,
+                                &current_link,
+                                listing) && success;
+            break;
+          }
+        }
         path->Reset(path_length);
         if (S_ISDIR(entry_info.st_mode)) {
           success = HandleDir(entry.d_name,
                               path,
                               recursive,
                               follow_links,
+                              seen,
                               listing) && success;
         } else if (S_ISREG(entry_info.st_mode)) {
           success = HandleFile(entry.d_name,
@@ -327,7 +377,7 @@ bool Directory::List(const char* dir_name,
     PostError(listing, dir_name);
     return false;
   }
-  return ListRecursively(&path, recursive, follow_links, listing);
+  return ListRecursively(&path, recursive, follow_links, NULL, listing);
 }
 
 

runtime/bin/directory_win.cc

@@ -68,11 +68,20 @@ static bool IsBrokenLink(const wchar_t* link_name) {
   }
 }
 
+// A linked list structure holding a link target's unique file system ID.
+// Used to detect loops in the file system when listing recursively.
+struct LinkList {
+  DWORD volume;
+  DWORD id_low;
+  DWORD id_high;
+  LinkList* next;
+};
 
 // Forward declarations.
 static bool ListRecursively(PathBuffer* path,
                             bool recursive,
                             bool follow_links,
+                            LinkList* seen,
                             DirectoryListing* listing);
 static bool DeleteRecursively(PathBuffer* path);
 
@@ -89,6 +98,7 @@ static bool HandleDir(wchar_t* dir_name,
                       PathBuffer* path,
                       bool recursive,
                       bool follow_links,
+                      LinkList* seen,
                       DirectoryListing* listing) {
   if (wcscmp(dir_name, L".") == 0) return true;
   if (wcscmp(dir_name, L"..") == 0) return true;
@@ -100,7 +110,8 @@ static bool HandleDir(wchar_t* dir_name,
   bool ok = listing->HandleDirectory(utf8_path);
   free(utf8_path);
   return ok &&
-      (!recursive || ListRecursively(path, recursive, follow_links, listing));
+      (!recursive ||
+       ListRecursively(path, recursive, follow_links, seen, listing));
 }
 
 
@@ -136,6 +147,7 @@ static bool HandleEntry(LPWIN32_FIND_DATAW find_file_data,
                         PathBuffer* path,
                         bool recursive,
                         bool follow_links,
+                        LinkList* seen,
                         DirectoryListing* listing) {
   DWORD attributes = find_file_data->dwFileAttributes;
   if ((attributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) {
@@ -144,18 +156,61 @@ static bool HandleEntry(LPWIN32_FIND_DATAW find_file_data,
     }
     int path_length = path->length;
     if (!path->Add(find_file_data->cFileName)) return false;
-    bool broken = IsBrokenLink(path->data);
+    HANDLE handle = CreateFileW(
+        path->data,
+        0,
+        FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+        NULL,
+        OPEN_EXISTING,
+        FILE_FLAG_BACKUP_SEMANTICS,
+        NULL);
     path->Reset(path_length);
-    if (broken) {
+    if (handle == INVALID_HANDLE_VALUE) {
       // Report as (broken) link.
       return HandleLink(find_file_data->cFileName, path, listing);
     }
+    if ((attributes & FILE_ATTRIBUTE_DIRECTORY) != 0) {
+      // Check the seen link targets to see if we are in a file system loop.
+      LinkList current_link;
+      BY_HANDLE_FILE_INFORMATION info;
+      // Get info
+      if (!GetFileInformationByHandle(handle, &info)) {
+        DWORD error = GetLastError();
+        CloseHandle(handle);
+        SetLastError(error);
+        PostError(listing, path->data);
+        return false;
+      }
+      CloseHandle(handle);
+      current_link.volume = info.dwVolumeSerialNumber;
+      current_link.id_low = info.nFileIndexLow;
+      current_link.id_high = info.nFileIndexHigh;
+      current_link.next = seen;
+      LinkList* previous = seen;
+      while (previous != NULL) {
+        if (previous->volume == current_link.volume &&
+            previous->id_low == current_link.id_low &&
+            previous->id_high == current_link.id_high) {
+          // Report the looping link as a link, rather than following it.
+          return HandleLink(find_file_data->cFileName, path, listing);
+        }
+        previous = previous->next;
+      }
+      // Recurse into the directory, adding current link to the seen links list.
+      return HandleDir(find_file_data->cFileName,
+                       path,
+                       recursive,
+                       follow_links,
+                       &current_link,
+                       listing);
+    }
   }
   if ((attributes & FILE_ATTRIBUTE_DIRECTORY) != 0) {
     return HandleDir(find_file_data->cFileName,
                      path,
                      recursive,
                      follow_links,
+                     seen,
                      listing);
   } else {
     return HandleFile(find_file_data->cFileName, path, listing);
@@ -166,6 +221,7 @@ static bool HandleEntry(LPWIN32_FIND_DATAW find_file_data,
 static bool ListRecursively(PathBuffer* path,
                             bool recursive,
                             bool follow_links,
+                            LinkList* seen,
                             DirectoryListing* listing) {
   if (!path->Add(L"\\*")) {
     PostError(listing, path->data);
@@ -188,6 +244,7 @@ static bool ListRecursively(PathBuffer* path,
                              path,
                              recursive,
                              follow_links,
+                             seen,
                              listing);
 
   while ((FindNextFileW(find_handle, &find_file_data) != 0)) {
@@ -196,6 +253,7 @@ static bool ListRecursively(PathBuffer* path,
                           path,
                           recursive,
                           follow_links,
+                          seen,
                           listing) && success;
   }
 
@@ -320,7 +378,7 @@ bool Directory::List(const char* dir_name,
     return false;
   }
   free(const_cast<wchar_t*>(system_name));
-  return ListRecursively(&path, recursive, follow_links, listing);
+  return ListRecursively(&path, recursive, follow_links, NULL, listing);
 }
 
 

sdk/lib/io/directory.dart

@@ -152,7 +152,11 @@ abstract class Directory extends FileSystemEntity {
    * If [followLinks] is true, then working links are reported as
    * directories or files, depending on
    * their type, and links to directories are recursed into.
-   * Broken links are reported as [Link] objects,
+   * Broken links are reported as [Link] objects.
+   * If a symbolic link makes a loop in the file system, then a recursive
+   * listing will not follow a link twice in the
+   * same recursive descent, but will report it as a [Link]
+   * the second time it is seen.
    *
    * The result is a stream of [FileSystemEntity] objects
    * for the directories, files, and links.
@@ -171,7 +175,11 @@ abstract class Directory extends FileSystemEntity {
    * If [followLinks] is true, then working links are reported as
    * directories or files, depending on
    * their type, and links to directories are recursed into.
-   * Broken links are reported as [Link] objects,
+   * Broken links are reported as [Link] objects.
+   * If a link makes a loop in the file system, then a recursive
+   * listing will not follow a link twice in the
+   * same recursive descent, but will report it as a [Link]
+   * the second time it is seen.
    *
    * Returns a [List] containing [FileSystemEntity] objects for the
    * directories, files, and links.

tests/standalone/io/link_test.dart

@@ -137,7 +137,34 @@ testCreateSync() {
   });
 }
 
+testCreateLoopingLink() {
+  Path base = new Path(new Directory('').createTempSync().path);
+  new Directory.fromPath(base.append('a/b/c')).create(recursive: true)
+  .then((_) =>
+    new Link.fromPath(base.append('a/b/c/d'))
+        .create(base.append('a/b').toNativePath()))
+  .then((_) =>
+    new Link.fromPath(base.append('a/b/c/e'))
+        .create(base.append('a').toNativePath()))
+  .then((_) =>
+    new Directory.fromPath(base.append('a'))
+        .list(recursive: true, followLinks: false)
+        .last)
+  .then((_) =>
+    // This directory listing must terminate, even though it contains loops.
+    new Directory.fromPath(base.append('a'))
+        .list(recursive: true, followLinks: true)
+        .last)
+  .then((_) =>
+    // This directory listing must terminate, even though it contains loops.
+    new Directory.fromPath(base.append('a/b/c'))
+        .list(recursive: true, followLinks: true)
+        .last)
+  .then((_) =>
+    new Directory.fromPath(base).delete(recursive: true));
+}
 
 main() {
   testCreateSync();
+  testCreateLoopingLink();
 }