Add file size to DirectoryEntry

envoy/filesystem/filesystem.h

@@ -10,6 +10,7 @@
 #include "envoy/common/pure.h"
 
 #include "absl/strings/string_view.h"
+#include "absl/types/optional.h"
 
 namespace Envoy {
 namespace Filesystem {
@@ -169,15 +170,19 @@ struct DirectoryEntry {
   // target. For example, if name_ is a symlink to a directory, its file type will be Directory.
   FileType type_;
 
+  // The file size in bytes for regular files. nullopt for FileType::Directory and FileType::Other,
+  // and, on Windows, also nullopt for symlinks, and on Linux nullopt for broken symlinks.
+  absl::optional<uint64_t> size_bytes_;
+
   bool operator==(const DirectoryEntry& rhs) const {
-    return name_ == rhs.name_ && type_ == rhs.type_;
+    return name_ == rhs.name_ && type_ == rhs.type_ && size_bytes_ == rhs.size_bytes_;
   }
 };
 
 class DirectoryIteratorImpl;
 class DirectoryIterator {
 public:
-  DirectoryIterator() : entry_({"", FileType::Other}) {}
+  DirectoryIterator() : entry_({"", FileType::Other, absl::nullopt}) {}
   virtual ~DirectoryIterator() = default;
 
   const DirectoryEntry& operator*() const { return entry_; }

source/common/filesystem/posix/directory_iterator_impl.cc

@@ -42,39 +42,38 @@ void DirectoryIteratorImpl::nextEntry() {
   }
 
   if (entry == nullptr) {
-    entry_ = {"", FileType::Other};
+    entry_ = {"", FileType::Other, absl::nullopt};
   } else {
-    const std::string current_path(entry->d_name);
-    const std::string full_path(directory_path_ + "/" + current_path);
-    entry_ = {current_path, fileType(full_path, os_sys_calls_)};
+    entry_ = makeEntry(entry->d_name);
   }
 }
 
-FileType DirectoryIteratorImpl::fileType(const std::string& full_path,
-                                         Api::OsSysCallsImpl& os_sys_calls) {
+DirectoryEntry DirectoryIteratorImpl::makeEntry(absl::string_view filename) const {
+  const std::string full_path = absl::StrCat(directory_path_, "/", filename);
   struct stat stat_buf;
-
-  const Api::SysCallIntResult result = os_sys_calls.stat(full_path.c_str(), &stat_buf);
+  const Api::SysCallIntResult result = os_sys_calls_.stat(full_path.c_str(), &stat_buf);
   if (result.return_value_ != 0) {
-    if (errno == ENOENT) {
+    if (result.errno_ == ENOENT) {
       // Special case. This directory entity is likely to be a symlink,
       // but the reference is broken as the target could not be stat()'ed.
       // If we confirm this with an lstat, treat this file entity as
       // a regular file, which may be unlink()'ed.
       if (::lstat(full_path.c_str(), &stat_buf) == 0 && S_ISLNK(stat_buf.st_mode)) {
-        return FileType::Regular;
+        return DirectoryEntry{std::string{filename}, FileType::Regular, absl::nullopt};
       }
     }
+    // TODO: throwing an exception here makes this dangerous to use in worker threads,
+    // and in general since it's not clear to the user of Directory that an exception
+    // may be thrown. Perhaps make this return StatusOr and handle failures gracefully.
     throw EnvoyException(fmt::format("unable to stat file: '{}' ({})", full_path, errno));
-  }
-
-  if (S_ISDIR(stat_buf.st_mode)) {
-    return FileType::Directory;
+  } else if (S_ISDIR(stat_buf.st_mode)) {
+    return DirectoryEntry{std::string{filename}, FileType::Directory, absl::nullopt};
   } else if (S_ISREG(stat_buf.st_mode)) {
-    return FileType::Regular;
+    return DirectoryEntry{std::string{filename}, FileType::Regular,
+                          static_cast<uint64_t>(stat_buf.st_size)};
+  } else {
+    return DirectoryEntry{std::string{filename}, FileType::Other, absl::nullopt};
   }
-
-  return FileType::Other;
 }
 
 } // namespace Filesystem

source/common/filesystem/posix/directory_iterator_impl.h

@@ -25,15 +25,17 @@ class DirectoryIteratorImpl : public DirectoryIterator {
   DirectoryIteratorImpl(const DirectoryIteratorImpl&) = delete;
   DirectoryIteratorImpl(DirectoryIteratorImpl&&) = default;
 
-  static FileType fileType(const std::string& name, Api::OsSysCallsImpl& os_sys_calls);
-
 private:
   void nextEntry();
   void openDirectory();
 
+  DirectoryEntry makeEntry(absl::string_view filename) const;
+
   std::string directory_path_;
   DIR* dir_{nullptr};
   Api::OsSysCallsImpl& os_sys_calls_;
+
+  friend class DirectoryTest_MakeEntryThrowsOnStatFailure_Test;
 };
 
 } // namespace Filesystem

source/common/filesystem/win32/directory_iterator_impl.cc

@@ -16,7 +16,7 @@ DirectoryIteratorImpl::DirectoryIteratorImpl(const std::string& directory_path)
         fmt::format("unable to open directory {}: {}", directory_path, ::GetLastError()));
   }
 
-  entry_ = {std::string(find_data.cFileName), fileType(find_data)};
+  entry_ = makeEntry(find_data);
 }
 
 DirectoryIteratorImpl::~DirectoryIteratorImpl() {
@@ -34,27 +34,30 @@ DirectoryIteratorImpl& DirectoryIteratorImpl::operator++() {
   }
 
   if (ret == 0) {
-    entry_ = {"", FileType::Other};
+    entry_ = {"", FileType::Other, absl::nullopt};
   } else {
-    entry_ = {std::string(find_data.cFileName), fileType(find_data)};
+    entry_ = makeEntry(find_data);
   }
 
   return *this;
 }
 
-FileType DirectoryIteratorImpl::fileType(const WIN32_FIND_DATA& find_data) const {
+DirectoryEntry DirectoryIteratorImpl::makeEntry(const WIN32_FIND_DATA& find_data) {
+  ULARGE_INTEGER file_size;
+  file_size.LowPart = find_data.nFileSizeLow;
+  file_size.HighPart = find_data.nFileSizeHigh;
+  uint64_t size = static_cast<uint64_t>(file_size.QuadPart);
   if ((find_data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) &&
       !(find_data.dwReserved0 & IO_REPARSE_TAG_SYMLINK)) {
     // The file is reparse point and not a symlink, so it can't be
     // a regular file or a directory
-    return FileType::Other;
-  }
-
-  if (find_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
-    return FileType::Directory;
+    return {std::string(find_data.cFileName), FileType::Other, absl::nullopt};
+  } else if (find_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
+    return {std::string(find_data.cFileName), FileType::Directory, absl::nullopt};
+  } else {
+    return {std::string(find_data.cFileName), FileType::Regular,
+            (find_data.dwReserved0 & IO_REPARSE_TAG_SYMLINK) ? absl::nullopt : size};
   }
-
-  return FileType::Regular;
 }
 
 } // namespace Filesystem

source/common/filesystem/win32/directory_iterator_impl.h

@@ -22,7 +22,7 @@ class DirectoryIteratorImpl : public DirectoryIterator {
   DirectoryIteratorImpl& operator=(DirectoryIteratorImpl&&) = default;
 
 private:
-  FileType fileType(const WIN32_FIND_DATA& find_data) const;
+  static DirectoryEntry makeEntry(const WIN32_FIND_DATA& find_data);
 
   HANDLE find_handle_;
 };