Allow FileEnumerator to expose enumeration errors

Adds an ErrorPolicy setting to base::FileEnumerator. By default
(ErrorPolicy::IGNORE_ERRORS) all FileEnumerators behave as they
always have. If set to ErrorPolicy::STOP_ENUMERATION however, a
FileEnumerator will cease enumeration as soon as it encounters
any error, and the corresponding error code can be retrieved from
the FileEnumerator.

The motivation for this change is that some existing logic in
third_party/leveldb/env_chromium.cc duplicates most of
FileEnumerator's behavior just to work around the lack of error
reporting. This change will allow the code to be de-duplicated
without disturbing error reporting histograms.

Bug: 1052045
Change-Id: I0639029403fe42d1baef12075898e3cc9ec7f6a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2082559
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746988}

base/files/file_enumerator.h

@@ -12,8 +12,10 @@
 
 #include "base/base_export.h"
 #include "base/containers/stack.h"
+#include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 
@@ -97,6 +99,20 @@ class BASE_EXPORT FileEnumerator {
     ALL,
   };
 
+  // Determines how a FileEnumerator handles errors encountered during
+  // enumeration. When no ErrorPolicy is explicitly set, FileEnumerator defaults
+  // to IGNORE_ERRORS.
+  enum class ErrorPolicy {
+    // Errors are ignored if possible and FileEnumerator returns as many files
+    // as it is able to enumerate.
+    IGNORE_ERRORS,
+
+    // Any error encountered during enumeration will terminate the enumeration
+    // immediately. An error code indicating the nature of a failure can be
+    // retrieved from |GetError()|.
+    STOP_ENUMERATION,
+  };
+
   // |root_path| is the starting directory to search for. It may or may not end
   // in a slash.
   //
@@ -114,9 +130,7 @@ class BASE_EXPORT FileEnumerator {
   // since the underlying code uses OS-specific matching routines.  In general,
   // Windows matching is less featureful than others, so test there first.
   // If unspecified, this will match all files.
-  FileEnumerator(const FilePath& root_path,
-                 bool recursive,
-                 int file_type);
+  FileEnumerator(const FilePath& root_path, bool recursive, int file_type);
   FileEnumerator(const FilePath& root_path,
                  bool recursive,
                  int file_type,
@@ -126,6 +140,12 @@ class BASE_EXPORT FileEnumerator {
                  int file_type,
                  const FilePath::StringType& pattern,
                  FolderSearchPolicy folder_search_policy);
+  FileEnumerator(const FilePath& root_path,
+                 bool recursive,
+                 int file_type,
+                 const FilePath::StringType& pattern,
+                 FolderSearchPolicy folder_search_policy,
+                 ErrorPolicy error_policy);
   ~FileEnumerator();
 
   // Returns the next file or an empty string if there are no more results.
@@ -138,6 +158,12 @@ class BASE_EXPORT FileEnumerator {
   // Write the file info into |info|.
   FileInfo GetInfo() const;
 
+  // Once |Next()| returns an empty path, enumeration has been terminated. If
+  // termination was normal (i.e. no more results to enumerate) or ErrorPolicy
+  // is set to IGNORE_ERRORS, this returns FILE_OK. Otherwise it returns an
+  // error code reflecting why enumeration was stopped early.
+  File::Error GetError() const { return error_; }
+
  private:
   // Returns true if the given path should be skipped in enumeration.
   bool ShouldSkip(const FilePath& path);
@@ -167,6 +193,8 @@ class BASE_EXPORT FileEnumerator {
   const int file_type_;
   FilePath::StringType pattern_;
   const FolderSearchPolicy folder_search_policy_;
+  const ErrorPolicy error_policy_;
+  File::Error error_ = File::FILE_OK;
 
   // A stack that keeps track of which subdirectories we still need to
   // enumerate in the breadth-first search.

base/files/file_enumerator_posix.cc

@@ -100,12 +100,26 @@ FileEnumerator::FileEnumerator(const FilePath& root_path,
                                int file_type,
                                const FilePath::StringType& pattern,
                                FolderSearchPolicy folder_search_policy)
+    : FileEnumerator(root_path,
+                     recursive,
+                     file_type,
+                     pattern,
+                     folder_search_policy,
+                     ErrorPolicy::IGNORE_ERRORS) {}
+
+FileEnumerator::FileEnumerator(const FilePath& root_path,
+                               bool recursive,
+                               int file_type,
+                               const FilePath::StringType& pattern,
+                               FolderSearchPolicy folder_search_policy,
+                               ErrorPolicy error_policy)
     : current_directory_entry_(0),
       root_path_(root_path),
       recursive_(recursive),
       file_type_(file_type),
       pattern_(pattern),
-      folder_search_policy_(folder_search_policy) {
+      folder_search_policy_(folder_search_policy),
+      error_policy_(error_policy) {
   // INCLUDE_DOT_DOT must not be specified if recursive.
   DCHECK(!(recursive && (INCLUDE_DOT_DOT & file_type_)));
 
@@ -135,8 +149,12 @@ FilePath FileEnumerator::Next() {
     pending_paths_.pop();
 
     DIR* dir = opendir(root_path_.value().c_str());
-    if (!dir)
-      continue;
+    if (!dir) {
+      if (errno == 0 || error_policy_ == ErrorPolicy::IGNORE_ERRORS)
+        continue;
+      error_ = File::OSErrorToFileError(errno);
+      return FilePath();
+    }
 
     directory_entries_.clear();
 
@@ -157,7 +175,11 @@ FilePath FileEnumerator::Next() {
 
     current_directory_entry_ = 0;
     struct dirent* dent;
-    while ((dent = readdir(dir))) {
+    // NOTE: Per the readdir() documentation, when the end of the directory is
+    // reached with no errors, null is returned and errno is not changed.
+    // Therefore we must reset errno to zero before calling readdir() if we
+    // wish to know whether a null result indicates an error condition.
+    while (errno = 0, dent = readdir(dir)) {
       FileInfo info;
       info.filename_ = FilePath(dent->d_name);
 
@@ -195,7 +217,12 @@ FilePath FileEnumerator::Next() {
       if (is_pattern_matched && IsTypeMatched(is_dir))
         directory_entries_.push_back(std::move(info));
     }
+    int readdir_errno = errno;
     closedir(dir);
+    if (readdir_errno != 0 && error_policy_ != ErrorPolicy::IGNORE_ERRORS) {
+      error_ = File::OSErrorToFileError(readdir_errno);
+      return FilePath();
+    }
 
     // MATCH_ONLY policy enumerates files in matched subfolders by "*" pattern.
     // ALL policy enumerates files in all subfolders by origin pattern.

base/files/file_enumerator_unittest.cc

@@ -36,7 +36,8 @@ circular_deque<FilePath> RunEnumerator(
     FileEnumerator::FolderSearchPolicy folder_search_policy) {
   circular_deque<FilePath> rv;
   FileEnumerator enumerator(root_path, recursive, file_type, pattern,
-                            folder_search_policy);
+                            folder_search_policy,
+                            FileEnumerator::ErrorPolicy::IGNORE_ERRORS);
   for (auto file = enumerator.Next(); !file.empty(); file = enumerator.Next())
     rv.emplace_back(std::move(file));
   return rv;
@@ -310,6 +311,29 @@ TEST(FileEnumerator, FilesInSubfoldersWithFiltering) {
   EXPECT_THAT(files, UnorderedElementsAre(subdir_foo, foo_foo, bar_foo));
 }
 
+TEST(FileEnumerator, InvalidDirectory) {
+  ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  const FilePath test_file = temp_dir.GetPath().AppendASCII("test_file");
+  ASSERT_TRUE(CreateDummyFile(test_file));
+
+  // Attempt to enumerate entries at a regular file path.
+  FileEnumerator enumerator(test_file, /*recursive=*/true,
+                            FileEnumerator::FILES, kEmptyPattern,
+                            FileEnumerator::FolderSearchPolicy::ALL,
+                            FileEnumerator::ErrorPolicy::STOP_ENUMERATION);
+  FilePath path = enumerator.Next();
+  EXPECT_TRUE(path.empty());
+
+  // Slightly different outcomes between Windows and POSIX.
+#if defined(OS_WIN)
+  EXPECT_EQ(File::Error::FILE_ERROR_FAILED, enumerator.GetError());
+#else
+  EXPECT_EQ(File::Error::FILE_ERROR_NOT_A_DIRECTORY, enumerator.GetError());
+#endif
+}
+
 #if defined(OS_POSIX)
 TEST(FileEnumerator, SymLinkLoops) {
   ScopedTempDir temp_dir;

base/files/file_enumerator_win.cc

@@ -86,10 +86,24 @@ FileEnumerator::FileEnumerator(const FilePath& root_path,
                                int file_type,
                                const FilePath::StringType& pattern,
                                FolderSearchPolicy folder_search_policy)
+    : FileEnumerator(root_path,
+                     recursive,
+                     file_type,
+                     pattern,
+                     folder_search_policy,
+                     ErrorPolicy::IGNORE_ERRORS) {}
+
+FileEnumerator::FileEnumerator(const FilePath& root_path,
+                               bool recursive,
+                               int file_type,
+                               const FilePath::StringType& pattern,
+                               FolderSearchPolicy folder_search_policy,
+                               ErrorPolicy error_policy)
     : recursive_(recursive),
       file_type_(file_type),
       pattern_(!pattern.empty() ? pattern : FILE_PATH_LITERAL("*")),
-      folder_search_policy_(folder_search_policy) {
+      folder_search_policy_(folder_search_policy),
+      error_policy_(error_policy) {
   // INCLUDE_DOT_DOT must not be specified if recursive.
   DCHECK(!(recursive && (INCLUDE_DOT_DOT & file_type_)));
   memset(&find_data_, 0, sizeof(find_data_));
@@ -136,6 +150,7 @@ FilePath FileEnumerator::Next() {
       }
     }
 
+    DWORD last_error = GetLastError();
     if (INVALID_HANDLE_VALUE == find_handle_) {
       has_find_data_ = false;
 
@@ -150,7 +165,13 @@ FilePath FileEnumerator::Next() {
         pattern_ = FILE_PATH_LITERAL("*");
       }
 
-      continue;
+      if (last_error == ERROR_NO_MORE_FILES ||
+          error_policy_ == ErrorPolicy::IGNORE_ERRORS) {
+        continue;
+      }
+
+      error_ = File::OSErrorToFileError(last_error);
+      return FilePath();
     }
 
     const FilePath filename(find_data_.cFileName);