filesystem: making fileReadToEnd take absl::status

contrib/client_ssl_auth/filters/network/test/client_ssl_auth_test.cc

@@ -176,8 +176,11 @@ TEST_F(ClientSslAuthFilterTest, Ssl) {
   EXPECT_CALL(*interval_timer_, enableTimer(_, _));
   Http::ResponseMessagePtr message(new Http::ResponseMessageImpl(
       Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}}));
-  message->body().add(api_->fileSystem().fileReadToEnd(TestEnvironment::runfilesPath(
-      "contrib/client_ssl_auth/filters/network/test/test_data/vpn_response_1.json")));
+  message->body().add(
+      api_->fileSystem()
+          .fileReadToEnd(TestEnvironment::runfilesPath(
+              "contrib/client_ssl_auth/filters/network/test/test_data/vpn_response_1.json"))
+          .value());
   callbacks_->onSuccess(request_, std::move(message));
   EXPECT_EQ(1U,
             stats_store_

envoy/filesystem/BUILD

@@ -15,6 +15,7 @@ envoy_cc_library(
         "//envoy/api:io_error_interface",
         "//envoy/api:os_sys_calls_interface",
         "//envoy/common:time_interface",
+        "@com_google_absl//absl/status:statusor",
     ],
 )
 

envoy/filesystem/filesystem.h

@@ -10,6 +10,7 @@
 #include "envoy/common/pure.h"
 #include "envoy/common/time.h"
 
+#include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "absl/types/optional.h"
 
@@ -195,18 +196,17 @@ class Instance {
   virtual ssize_t fileSize(const std::string& path) PURE;
 
   /**
-   * @return full file content as a string.
-   * @throw EnvoyException if the file cannot be read.
+   * @return full file content as a string or an error if the file can not be read.
    * Be aware, this is not most highly performing file reading method.
    */
-  virtual std::string fileReadToEnd(const std::string& path) PURE;
+  virtual absl::StatusOr<std::string> fileReadToEnd(const std::string& path) PURE;
 
   /**
    * @path file path to split
-   * @return PathSplitResult containing the parent directory of the input path and the file name
-   * @note will throw an exception if path does not contain any path separator character
+   * @return PathSplitResult containing the parent directory of the input path and the file name or
+   * an error status.
    */
-  virtual PathSplitResult splitPathFromFilename(absl::string_view path) PURE;
+  virtual absl::StatusOr<PathSplitResult> splitPathFromFilename(absl::string_view path) PURE;
 
   /**
    * Determine if the path is on a list of paths Envoy will refuse to access. This

source/common/config/datasource.cc

@@ -18,9 +18,12 @@ static constexpr uint32_t RetryCount = 1;
 std::string read(const envoy::config::core::v3::DataSource& source, bool allow_empty,
                  Api::Api& api) {
   std::string data;
+  absl::StatusOr<std::string> file_or_error;
   switch (source.specifier_case()) {
   case envoy::config::core::v3::DataSource::SpecifierCase::kFilename:
-    data = api.fileSystem().fileReadToEnd(source.filename());
+    file_or_error = api.fileSystem().fileReadToEnd(source.filename());
+    THROW_IF_STATUS_NOT_OK(file_or_error, throw);
+    data = file_or_error.value();
     break;
   case envoy::config::core::v3::DataSource::SpecifierCase::kInlineBytes:
     data = source.inline_bytes();

source/common/filesystem/inotify/watcher_impl.cc

@@ -35,7 +35,9 @@ WatcherImpl::~WatcherImpl() { close(inotify_fd_); }
 void WatcherImpl::addWatch(absl::string_view path, uint32_t events, OnChangedCb callback) {
   // Because of general inotify pain, we always watch the directory that the file lives in,
   // and then synthetically raise per file events.
-  const PathSplitResult result = file_system_.splitPathFromFilename(path);
+  auto result_or_error = file_system_.splitPathFromFilename(path);
+  THROW_IF_STATUS_NOT_OK(result_or_error, throw);
+  const PathSplitResult result = result_or_error.value();
 
   const uint32_t watch_mask = IN_MODIFY | IN_MOVED_TO;
   int watch_fd = inotify_add_watch(inotify_fd_, std::string(result.directory_).c_str(), watch_mask);

source/common/filesystem/kqueue/watcher_impl.cc

@@ -49,7 +49,9 @@ WatcherImpl::FileWatchPtr WatcherImpl::addWatch(absl::string_view path, uint32_t
       return nullptr;
     }
 
-    watch_fd = open(std::string(file_system_.splitPathFromFilename(path).directory_).c_str(), 0);
+    const auto result_or_error = file_system_.splitPathFromFilename(path);
+    THROW_IF_STATUS_NOT_OK(result_or_error, throw);
+    watch_fd = open(std::string(result_or_error.value().directory_).c_str(), 0);
     if (watch_fd == -1) {
       return nullptr;
     }
@@ -106,7 +108,10 @@ void WatcherImpl::onKqueueEvent() {
     ASSERT(file != nullptr);
     ASSERT(watch_fd == file->fd_);
 
-    auto pathname = file_system_.splitPathFromFilename(file->file_);
+    absl::StatusOr<PathSplitResult> pathname_or_error =
+        file_system_.splitPathFromFilename(file->file_);
+    THROW_IF_STATUS_NOT_OK(pathname_or_error, throw);
+    PathSplitResult& pathname = pathname_or_error.value();
 
     if (file->watching_dir_) {
       if (event.fflags & NOTE_DELETE) {

source/common/filesystem/posix/filesystem_impl.cc

@@ -137,9 +137,11 @@ static constexpr absl::optional<SystemTime> systemTimeFromTimespec(const struct
   return timespecToChrono(t);
 }
 
-static FileInfo infoFromStat(absl::string_view path, const struct stat& s, FileType type) {
-  return {
-      std::string{InstanceImplPosix().splitPathFromFilename(path).file_},
+static Api::IoCallResult<FileInfo> infoFromStat(absl::string_view path, const struct stat& s,
+                                                FileType type) {
+  auto result_or_error = InstanceImplPosix().splitPathFromFilename(path);
+  FileInfo ret = {
+      result_or_error.status().ok() ? std::string{result_or_error.value().file_} : "",
       s.st_size,
       type,
 #ifdef _DARWIN_FEATURE_64_BIT_INODE
@@ -152,9 +154,10 @@ static FileInfo infoFromStat(absl::string_view path, const struct stat& s, FileT
       systemTimeFromTimespec(s.st_mtim),
 #endif
   };
+  return result_or_error.status().ok() ? resultSuccess(ret) : resultFailure(ret, EINVAL);
 }
 
-static FileInfo infoFromStat(absl::string_view path, const struct stat& s) {
+static Api::IoCallResult<FileInfo> infoFromStat(absl::string_view path, const struct stat& s) {
   return infoFromStat(path, s, typeFromStat(s));
 }
 
@@ -164,7 +167,7 @@ Api::IoCallResult<FileInfo> FileImplPosix::info() {
   if (::fstat(fd_, &s) != 0) {
     return resultFailure<FileInfo>({}, errno);
   }
-  return resultSuccess(infoFromStat(path(), s));
+  return infoFromStat(path(), s);
 }
 
 Api::IoCallResult<FileInfo> InstanceImplPosix::stat(absl::string_view path) {
@@ -177,12 +180,12 @@ Api::IoCallResult<FileInfo> InstanceImplPosix::stat(absl::string_view path) {
         // but the reference is broken as the target could not be stat()'ed.
         // After confirming this with an lstat, treat this file entity as
         // a regular file, which may be unlink()'ed.
-        return resultSuccess(infoFromStat(path, s, FileType::Regular));
+        return infoFromStat(path, s, FileType::Regular);
       }
     }
     return resultFailure<FileInfo>({}, errno);
   }
-  return resultSuccess(infoFromStat(path, s));
+  return infoFromStat(path, s);
 }
 
 Api::IoCallBoolResult InstanceImplPosix::createPath(absl::string_view path) {
@@ -282,14 +285,14 @@ ssize_t InstanceImplPosix::fileSize(const std::string& path) {
   return info.st_size;
 }
 
-std::string InstanceImplPosix::fileReadToEnd(const std::string& path) {
+absl::StatusOr<std::string> InstanceImplPosix::fileReadToEnd(const std::string& path) {
   if (illegalPath(path)) {
-    throw EnvoyException(absl::StrCat("Invalid path: ", path));
+    return absl::InvalidArgumentError(absl::StrCat("Invalid path: ", path));
   }
 
   std::ifstream file(path);
   if (file.fail()) {
-    throw EnvoyException(absl::StrCat("unable to read file: ", path));
+    return absl::InvalidArgumentError(absl::StrCat("unable to read file: ", path));
   }
 
   std::stringstream file_string;
@@ -298,17 +301,17 @@ std::string InstanceImplPosix::fileReadToEnd(const std::string& path) {
   return file_string.str();
 }
 
-PathSplitResult InstanceImplPosix::splitPathFromFilename(absl::string_view path) {
+absl::StatusOr<PathSplitResult> InstanceImplPosix::splitPathFromFilename(absl::string_view path) {
   size_t last_slash = path.rfind('/');
   if (last_slash == std::string::npos) {
-    throw EnvoyException(fmt::format("invalid file path {}", path));
+    return absl::InvalidArgumentError(fmt::format("invalid file path {}", path));
   }
   absl::string_view name = path.substr(last_slash + 1);
   // truncate all trailing slashes, except root slash
   if (last_slash == 0) {
     ++last_slash;
   }
-  return {path.substr(0, last_slash), name};
+  return PathSplitResult{path.substr(0, last_slash), name};
 }
 
 bool InstanceImplPosix::illegalPath(const std::string& path) {

source/common/filesystem/posix/filesystem_impl.h

@@ -58,8 +58,8 @@ class InstanceImplPosix : public Instance {
   bool fileExists(const std::string& path) override;
   bool directoryExists(const std::string& path) override;
   ssize_t fileSize(const std::string& path) override;
-  std::string fileReadToEnd(const std::string& path) override;
-  PathSplitResult splitPathFromFilename(absl::string_view path) override;
+  absl::StatusOr<std::string> fileReadToEnd(const std::string& path) override;
+  absl::StatusOr<PathSplitResult> splitPathFromFilename(absl::string_view path) override;
   bool illegalPath(const std::string& path) override;
   Api::IoCallResult<FileInfo> stat(absl::string_view path) override;
   Api::IoCallBoolResult createPath(absl::string_view path) override;

source/common/filesystem/win32/filesystem_impl.cc

@@ -150,21 +150,26 @@ static FileType fileTypeFromAttributeData(const WIN32_FILE_ATTRIBUTE_DATA& data)
   return FileType::Regular;
 }
 
-static FileInfo fileInfoFromAttributeData(absl::string_view path,
-                                          const WIN32_FILE_ATTRIBUTE_DATA& data) {
+static Api::IoCallResult<FileInfo>
+fileInfoFromAttributeData(absl::string_view path, const WIN32_FILE_ATTRIBUTE_DATA& data) {
   absl::optional<uint64_t> sz;
   FileType type = fileTypeFromAttributeData(data);
   if (type == FileType::Regular) {
     sz = fileSizeFromAttributeData(data);
   }
-  return {
-      std::string{InstanceImplWin32().splitPathFromFilename(path).file_},
+  absl::StatusOr<PathSplitResult> result_or_error = InstanceImplWin32().splitPathFromFilename(path);
+  FileInfo ret{
+      result_or_error.ok() ? std::string{result_or_error.value().file_} : "",
       sz,
       type,
       systemTimeFromFileTime(data.ftCreationTime),
       systemTimeFromFileTime(data.ftLastAccessTime),
       systemTimeFromFileTime(data.ftLastWriteTime),
   };
+  if (result_or_error.status().ok()) {
+    return resultSuccess(ret);
+  }
+  return resultFailure(ret, ERROR_INVALID_DATA);
 }
 
 Api::IoCallResult<FileInfo> FileImplWin32::info() {
@@ -174,7 +179,7 @@ Api::IoCallResult<FileInfo> FileImplWin32::info() {
   if (!result) {
     return resultFailure<FileInfo>({}, ::GetLastError());
   }
-  return resultSuccess(fileInfoFromAttributeData(path(), data));
+  return fileInfoFromAttributeData(path(), data);
 }
 
 Api::IoCallResult<FileInfo> InstanceImplWin32::stat(absl::string_view path) {
@@ -184,7 +189,7 @@ Api::IoCallResult<FileInfo> InstanceImplWin32::stat(absl::string_view path) {
   if (!result) {
     return resultFailure<FileInfo>({}, ::GetLastError());
   }
-  return resultSuccess(fileInfoFromAttributeData(full_path, data));
+  return fileInfoFromAttributeData(full_path, data);
 }
 
 Api::IoCallBoolResult InstanceImplWin32::createPath(absl::string_view path) {
@@ -276,9 +281,9 @@ ssize_t InstanceImplWin32::fileSize(const std::string& path) {
   return result;
 }
 
-std::string InstanceImplWin32::fileReadToEnd(const std::string& path) {
+absl::StatusOr<std::string> InstanceImplWin32::fileReadToEnd(const std::string& path) {
   if (illegalPath(path)) {
-    throw EnvoyException(absl::StrCat("Invalid path: ", path));
+    return absl::InvalidArgumentError(absl::StrCat("Invalid path: ", path));
   }
 
   // In integration tests (and potentially in production) we rename config files and this creates
@@ -290,9 +295,9 @@ std::string InstanceImplWin32::fileReadToEnd(const std::string& path) {
   if (fd == INVALID_HANDLE) {
     auto last_error = ::GetLastError();
     if (last_error == ERROR_FILE_NOT_FOUND) {
-      throw EnvoyException(absl::StrCat("Invalid path: ", path));
+      return absl::InvalidArgumentError(absl::StrCat("Invalid path: ", path));
     }
-    throw EnvoyException(absl::StrCat("unable to read file: ", path));
+    return absl::InvalidArgumentError(absl::StrCat("unable to read file: ", path));
   }
   DWORD buffer_size = 100;
   DWORD bytes_read = 0;
@@ -303,29 +308,29 @@ std::string InstanceImplWin32::fileReadToEnd(const std::string& path) {
       auto last_error = ::GetLastError();
       if (last_error == ERROR_FILE_NOT_FOUND) {
         CloseHandle(fd);
-        throw EnvoyException(absl::StrCat("Invalid path: ", path));
+        return absl::InvalidArgumentError(absl::StrCat("Invalid path: ", path));
       }
       CloseHandle(fd);
-      throw EnvoyException(absl::StrCat("unable to read file: ", path));
+      return absl::InvalidArgumentError(absl::StrCat("unable to read file: ", path));
     }
     complete_buffer.insert(complete_buffer.end(), buffer.begin(), buffer.begin() + bytes_read);
   } while (bytes_read == buffer_size);
   CloseHandle(fd);
   return std::string(complete_buffer.begin(), complete_buffer.end());
 }
 
-PathSplitResult InstanceImplWin32::splitPathFromFilename(absl::string_view path) {
+absl::StatusOr<PathSplitResult> InstanceImplWin32::splitPathFromFilename(absl::string_view path) {
   size_t last_slash = path.find_last_of(":/\\");
   if (last_slash == std::string::npos) {
-    throw EnvoyException(fmt::format("invalid file path {}", path));
+    return absl::InvalidArgumentError(fmt::format("invalid file path {}", path));
   }
   absl::string_view name = path.substr(last_slash + 1);
   // Truncate all trailing slashes, but retain the entire
   // single '/', 'd:' drive, and 'd:\' drive root paths
   if (last_slash == 0 || path[last_slash] == ':' || path[last_slash - 1] == ':') {
     ++last_slash;
   }
-  return {path.substr(0, last_slash), name};
+  return PathSplitResult{path.substr(0, last_slash), name};
 }
 
 // clang-format off

source/common/filesystem/win32/filesystem_impl.h

@@ -92,8 +92,8 @@ class InstanceImplWin32 : public Instance {
   bool fileExists(const std::string& path) override;
   bool directoryExists(const std::string& path) override;
   ssize_t fileSize(const std::string& path) override;
-  std::string fileReadToEnd(const std::string& path) override;
-  PathSplitResult splitPathFromFilename(absl::string_view path) override;
+  absl::StatusOr<std::string> fileReadToEnd(const std::string& path) override;
+  absl::StatusOr<PathSplitResult> splitPathFromFilename(absl::string_view path) override;
   bool illegalPath(const std::string& path) override;
   Api::IoCallResult<FileInfo> stat(absl::string_view path) override;
   Api::IoCallBoolResult createPath(absl::string_view path) override;

source/common/filesystem/win32/watcher_impl.cc

@@ -55,7 +55,9 @@ void WatcherImpl::addWatch(absl::string_view path, uint32_t events, OnChangedCb
     return;
   }
 
-  const PathSplitResult result = file_system_.splitPathFromFilename(path);
+  const absl::StatusOr<PathSplitResult> result_or_error = file_system_.splitPathFromFilename(path);
+  THROW_IF_STATUS_NOT_OK(result_or_error, throw);
+  const PathSplitResult& result = result_or_error.value();
   // ReadDirectoryChangesW only has a Unicode version, so we need
   // to use wide strings here
   const std::wstring directory = wstring_converter_.from_bytes(std::string(result.directory_));

source/common/protobuf/yaml_utility.cc

@@ -183,7 +183,9 @@ void MessageUtil::loadFromYaml(const std::string& yaml, Protobuf::Message& messa
 void MessageUtil::loadFromFile(const std::string& path, Protobuf::Message& message,
                                ProtobufMessage::ValidationVisitor& validation_visitor,
                                Api::Api& api) {
-  const std::string contents = api.fileSystem().fileReadToEnd(path);
+  auto file_or_error = api.fileSystem().fileReadToEnd(path);
+  THROW_IF_STATUS_NOT_OK(file_or_error, throw);
+  const std::string contents = file_or_error.value();
   // If the filename ends with .pb, attempt to parse it as a binary proto.
   if (absl::EndsWithIgnoreCase(path, FileExtensions::get().ProtoBinary)) {
     // Attempt to parse the binary format.

source/common/router/config_utility.cc

@@ -124,7 +124,9 @@ std::string ConfigUtility::parseDirectResponseBody(const envoy::config::route::v
       throw EnvoyException(fmt::format("response body file {} size is {} bytes; maximum is {}",
                                        filename, size, max_body_size_bytes));
     }
-    return api.fileSystem().fileReadToEnd(filename);
+    auto file_or_error = api.fileSystem().fileReadToEnd(filename);
+    THROW_IF_STATUS_NOT_OK(file_or_error, throw);
+    return file_or_error.value();
   }
   const std::string inline_body(body.inline_bytes().empty() ? body.inline_string()
                                                             : body.inline_bytes());

source/common/runtime/runtime_impl.cc

@@ -446,7 +446,9 @@ void DiskLayer::walkDirectory(const std::string& path, const std::string& prefix
 
       // Read the file and remove any comments. A comment is a line starting with a '#' character.
       // Comments are useful for placeholder files with no value.
-      const std::string text_file{api.fileSystem().fileReadToEnd(full_path)};
+      auto file_or_error = api.fileSystem().fileReadToEnd(full_path);
+      THROW_IF_STATUS_NOT_OK(file_or_error, throw);
+      const std::string text_file{file_or_error.value()};
 
       const auto lines = StringUtil::splitToken(text_file, "\n");
       for (const auto& line : lines) {

source/common/secret/sds_api.cc

@@ -119,8 +119,9 @@ absl::Status SdsApi::onConfigUpdate(const std::vector<Config::DecodedResourceRef
         for (auto const& filename : files) {
           // Watch for directory instead of file. This allows users to do atomic renames
           // on directory level (e.g. Kubernetes secret update).
-          const auto result = api_.fileSystem().splitPathFromFilename(filename);
-          watcher_->addWatch(absl::StrCat(result.directory_, "/"),
+          const auto result_or_error = api_.fileSystem().splitPathFromFilename(filename);
+          THROW_IF_STATUS_NOT_OK(result_or_error, throw);
+          watcher_->addWatch(absl::StrCat(result_or_error.value().directory_, "/"),
                              Filesystem::Watcher::Events::MovedTo,
                              [this](uint32_t) { onWatchUpdate(); });
         }
@@ -175,7 +176,9 @@ SdsApi::SecretData SdsApi::secretData() { return secret_data_; }
 SdsApi::FileContentMap SdsApi::loadFiles() {
   FileContentMap files;
   for (auto const& filename : getDataSourceFilenames()) {
-    files[filename] = api_.fileSystem().fileReadToEnd(filename);
+    auto file_or_error = api_.fileSystem().fileReadToEnd(filename);
+    THROW_IF_STATUS_NOT_OK(file_or_error, throw);
+    files[filename] = file_or_error.value();
   }
   return files;
 }

source/extensions/compression/zstd/common/dictionary_manager.h

@@ -89,7 +89,9 @@ template <class T, size_t (*deleter)(T*), unsigned (*getDictId)(const T*)> class
                                    public ThreadLocal::ThreadLocalObject {};
 
   void onDictionaryUpdate(unsigned origin_id, const std::string& filename) {
-    const auto data = api_.fileSystem().fileReadToEnd(filename);
+    auto file_or_error = api_.fileSystem().fileReadToEnd(filename);
+    THROW_IF_STATUS_NOT_OK(file_or_error, throw);
+    const auto data = file_or_error.value();
     if (!data.empty()) {
       auto dictionary = DictionarySharedPtr(builder_(data.data(), data.length()));
       auto id = getDictId(dictionary.get());