diff --git a/contrib/client_ssl_auth/filters/network/test/client_ssl_auth_test.cc b/contrib/client_ssl_auth/filters/network/test/client_ssl_auth_test.cc index 63e97ec041ea..b6b1aa25a786 100644 --- a/contrib/client_ssl_auth/filters/network/test/client_ssl_auth_test.cc +++ b/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_ diff --git a/envoy/filesystem/BUILD b/envoy/filesystem/BUILD index 39e6492051de..3652ad67c197 100644 --- a/envoy/filesystem/BUILD +++ b/envoy/filesystem/BUILD @@ -12,10 +12,10 @@ envoy_cc_library( name = "filesystem_interface", hdrs = ["filesystem.h"], deps = [ - "@com_google_absl//absl/status:statusor", "//envoy/api:io_error_interface", "//envoy/api:os_sys_calls_interface", "//envoy/common:time_interface", + "@com_google_absl//absl/status:statusor", ], ) diff --git a/envoy/filesystem/filesystem.h b/envoy/filesystem/filesystem.h index 4934926c6d37..6c58b61c98f9 100644 --- a/envoy/filesystem/filesystem.h +++ b/envoy/filesystem/filesystem.h @@ -203,10 +203,10 @@ class Instance { /** * @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 splitPathFromFilename(absl::string_view path) PURE; /** * Determine if the path is on a list of paths Envoy will refuse to access. This diff --git a/source/common/config/datasource.cc b/source/common/config/datasource.cc index 632699630c9a..db3e173a36bb 100644 --- a/source/common/config/datasource.cc +++ b/source/common/config/datasource.cc @@ -18,9 +18,10 @@ 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 file_or_error; switch (source.specifier_case()) { case envoy::config::core::v3::DataSource::SpecifierCase::kFilename: - auto file_or_error = api.fileSystem().fileReadToEnd(path); + file_or_error = api.fileSystem().fileReadToEnd(source.filename()); THROW_IF_STATUS_NOT_OK(file_or_error, throw); data = file_or_error.value(); break; diff --git a/source/common/filesystem/inotify/watcher_impl.cc b/source/common/filesystem/inotify/watcher_impl.cc index 61f8d2b69d93..c7a3db2cd23a 100644 --- a/source/common/filesystem/inotify/watcher_impl.cc +++ b/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); diff --git a/source/common/filesystem/kqueue/watcher_impl.cc b/source/common/filesystem/kqueue/watcher_impl.cc index 1cf9e0865bf5..e0a4246269da 100644 --- a/source/common/filesystem/kqueue/watcher_impl.cc +++ b/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; } diff --git a/source/common/filesystem/posix/filesystem_impl.cc b/source/common/filesystem/posix/filesystem_impl.cc index 579800f698b6..8198c95e45dd 100644 --- a/source/common/filesystem/posix/filesystem_impl.cc +++ b/source/common/filesystem/posix/filesystem_impl.cc @@ -137,9 +137,11 @@ static constexpr absl::optional 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 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,13 @@ static FileInfo infoFromStat(absl::string_view path, const struct stat& s, FileT systemTimeFromTimespec(s.st_mtim), #endif }; + if (result_or_error.status().ok()) { + return resultSuccess(ret); + } + return resultFailure(ret, EINVAL); } -static FileInfo infoFromStat(absl::string_view path, const struct stat& s) { +static Api::IoCallResult infoFromStat(absl::string_view path, const struct stat& s) { return infoFromStat(path, s, typeFromStat(s)); } @@ -164,7 +170,7 @@ Api::IoCallResult FileImplPosix::info() { if (::fstat(fd_, &s) != 0) { return resultFailure({}, errno); } - return resultSuccess(infoFromStat(path(), s)); + return infoFromStat(path(), s); } Api::IoCallResult InstanceImplPosix::stat(absl::string_view path) { @@ -177,12 +183,12 @@ Api::IoCallResult 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({}, errno); } - return resultSuccess(infoFromStat(path, s)); + return infoFromStat(path, s); } Api::IoCallBoolResult InstanceImplPosix::createPath(absl::string_view path) { @@ -298,17 +304,17 @@ absl::StatusOr InstanceImplPosix::fileReadToEnd(const std::string& return file_string.str(); } -PathSplitResult InstanceImplPosix::splitPathFromFilename(absl::string_view path) { +absl::StatusOr 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) { diff --git a/source/common/filesystem/posix/filesystem_impl.h b/source/common/filesystem/posix/filesystem_impl.h index feedea0da942..a17ab3973e5e 100644 --- a/source/common/filesystem/posix/filesystem_impl.h +++ b/source/common/filesystem/posix/filesystem_impl.h @@ -59,7 +59,7 @@ class InstanceImplPosix : public Instance { bool directoryExists(const std::string& path) override; ssize_t fileSize(const std::string& path) override; absl::StatusOr fileReadToEnd(const std::string& path) override; - PathSplitResult splitPathFromFilename(absl::string_view path) override; + absl::StatusOr splitPathFromFilename(absl::string_view path) override; bool illegalPath(const std::string& path) override; Api::IoCallResult stat(absl::string_view path) override; Api::IoCallBoolResult createPath(absl::string_view path) override; diff --git a/source/common/filesystem/win32/filesystem_impl.cc b/source/common/filesystem/win32/filesystem_impl.cc index 4c139142e953..4bf7cd9fd07e 100644 --- a/source/common/filesystem/win32/filesystem_impl.cc +++ b/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 +fileInfoFromAttributeData(absl::string_view path, const WIN32_FILE_ATTRIBUTE_DATA& data) { absl::optional sz; FileType type = fileTypeFromAttributeData(data); if (type == FileType::Regular) { sz = fileSizeFromAttributeData(data); } - return { - std::string{InstanceImplWin32().splitPathFromFilename(path).file_}, + absl::StatusOr result_or_error = InstanceImplWin32().splitPathFromFilename(path); + FileInfo ret{ + result_or_error.ok() ? std::string{result_or_error.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 FileImplWin32::info() { @@ -174,7 +179,7 @@ Api::IoCallResult FileImplWin32::info() { if (!result) { return resultFailure({}, ::GetLastError()); } - return resultSuccess(fileInfoFromAttributeData(path(), data)); + return fileInfoFromAttributeData(path(), data); } Api::IoCallResult InstanceImplWin32::stat(absl::string_view path) { @@ -184,7 +189,7 @@ Api::IoCallResult InstanceImplWin32::stat(absl::string_view path) { if (!result) { return resultFailure({}, ::GetLastError()); } - return resultSuccess(fileInfoFromAttributeData(full_path, data)); + return fileInfoFromAttributeData(full_path, data); } Api::IoCallBoolResult InstanceImplWin32::createPath(absl::string_view path) { @@ -314,10 +319,10 @@ absl::StatusOr InstanceImplWin32::fileReadToEnd(const std::string& return std::string(complete_buffer.begin(), complete_buffer.end()); } -PathSplitResult InstanceImplWin32::splitPathFromFilename(absl::string_view path) { +absl::StatusOr 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 diff --git a/source/common/filesystem/win32/filesystem_impl.h b/source/common/filesystem/win32/filesystem_impl.h index 534862a789f4..f7a5139c8af8 100644 --- a/source/common/filesystem/win32/filesystem_impl.h +++ b/source/common/filesystem/win32/filesystem_impl.h @@ -93,7 +93,7 @@ class InstanceImplWin32 : public Instance { bool directoryExists(const std::string& path) override; ssize_t fileSize(const std::string& path) override; absl::StatusOr fileReadToEnd(const std::string& path) override; - PathSplitResult splitPathFromFilename(absl::string_view path) override; + absl::StatusOr splitPathFromFilename(absl::string_view path) override; bool illegalPath(const std::string& path) override; Api::IoCallResult stat(absl::string_view path) override; Api::IoCallBoolResult createPath(absl::string_view path) override; diff --git a/source/common/filesystem/win32/watcher_impl.cc b/source/common/filesystem/win32/watcher_impl.cc index eaacb61cfd2f..601c787461bf 100644 --- a/source/common/filesystem/win32/watcher_impl.cc +++ b/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 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_)); diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index d61b81efcfa2..3c308b94e4f7 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -446,9 +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. - 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}; + 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) { diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index 1b113ae609cc..fcc0ddc4100e 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -116,8 +116,9 @@ absl::Status SdsApi::onConfigUpdate(const std::vectoraddWatch(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(); }); } @@ -167,7 +168,7 @@ SdsApi::SecretData SdsApi::secretData() { return secret_data_; } SdsApi::FileContentMap SdsApi::loadFiles() { FileContentMap files; for (auto const& filename : getDataSourceFilenames()) { - auto file_or_error = api.fileSystem().fileReadToEnd(path); + auto file_or_error = api_.fileSystem().fileReadToEnd(filename); THROW_IF_STATUS_NOT_OK(file_or_error, throw); files[filename] = file_or_error.value(); } diff --git a/source/extensions/compression/zstd/common/dictionary_manager.h b/source/extensions/compression/zstd/common/dictionary_manager.h index bd6a53033894..c70f2aba25b0 100644 --- a/source/extensions/compression/zstd/common/dictionary_manager.h +++ b/source/extensions/compression/zstd/common/dictionary_manager.h @@ -89,10 +89,8 @@ template class public ThreadLocal::ThreadLocalObject {}; void onDictionaryUpdate(unsigned origin_id, const std::string& filename) { -e auto file_or_error = api.fileSystem().fileReadToEnd(filename); - auto file_or_error = api.fileSystem().fileReadToEnd(filename); - THROW_IF_STATUS_NOT_OK(file_or_error, throw); - THROW_IF_STATUS_NOT_OK(file_or_error, throw); + 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())); diff --git a/source/extensions/filters/http/grpc_field_extraction/filter_config.cc b/source/extensions/filters/http/grpc_field_extraction/filter_config.cc index 96d3212511ec..996a5fff771c 100644 --- a/source/extensions/filters/http/grpc_field_extraction/filter_config.cc +++ b/source/extensions/filters/http/grpc_field_extraction/filter_config.cc @@ -56,8 +56,8 @@ void FilterConfig::initDescriptorPool(Api::Api& api) { switch (descriptor_config.specifier_case()) { case envoy::config::core::v3::DataSource::SpecifierCase::kFilename: { - if (!descriptor_set.ParseFromString( - api.fileSystem().fileReadToEnd(descriptor_config.filename()))) { + auto file_or_error = api.fileSystem().fileReadToEnd(descriptor_config.filename()); + if (!file_or_error.status().ok() || !descriptor_set.ParseFromString(file_or_error.value())) { throw Envoy::EnvoyException(fmt::format("unable to parse proto descriptor from file `{}`", descriptor_config.filename())); } diff --git a/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc b/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc index 37e7d9a7eb74..15aaeb4a0f31 100644 --- a/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc +++ b/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc @@ -121,13 +121,13 @@ JsonTranscoderConfig::JsonTranscoderConfig( switch (proto_config.descriptor_set_case()) { case envoy::extensions::filters::http::grpc_json_transcoder::v3::GrpcJsonTranscoder:: DescriptorSetCase::kProtoDescriptor: { - auto file_or_error = api.fileSystem().fileReadToEnd(proto_config.proto_descriptor()); - THROW_IF_STATUS_NOT_OK(file_or_error, throw); + auto file_or_error = api.fileSystem().fileReadToEnd(proto_config.proto_descriptor()); + THROW_IF_STATUS_NOT_OK(file_or_error, throw); if (!descriptor_set.ParseFromString(file_or_error.value())) { throw EnvoyException("transcoding_filter: Unable to parse proto descriptor"); } break; - } + } case envoy::extensions::filters::http::grpc_json_transcoder::v3::GrpcJsonTranscoder:: DescriptorSetCase::kProtoDescriptorBin: if (!descriptor_set.ParseFromString(proto_config.proto_descriptor_bin())) { diff --git a/source/extensions/key_value/file_based/config.cc b/source/extensions/key_value/file_based/config.cc index 70e81d6872ab..195f07f6c0ed 100644 --- a/source/extensions/key_value/file_based/config.cc +++ b/source/extensions/key_value/file_based/config.cc @@ -16,7 +16,7 @@ FileBasedKeyValueStore::FileBasedKeyValueStore(Event::Dispatcher& dispatcher, ENVOY_LOG(info, "File for key value store does not yet exist: {}", filename); return; } - auto file_or_error = api.fileSystem().fileReadToEnd(filename_); + auto file_or_error = file_system_.fileReadToEnd(filename_); THROW_IF_STATUS_NOT_OK(file_or_error, throw); const std::string contents = file_or_error.value(); if (!parseContents(contents)) { diff --git a/source/extensions/resource_monitors/injected_resource/injected_resource_monitor.cc b/source/extensions/resource_monitors/injected_resource/injected_resource_monitor.cc index affcbb07d3ac..8cc213abac62 100644 --- a/source/extensions/resource_monitors/injected_resource/injected_resource_monitor.cc +++ b/source/extensions/resource_monitors/injected_resource/injected_resource_monitor.cc @@ -27,8 +27,8 @@ void InjectedResourceMonitor::updateResourceUsage(Server::ResourceUpdateCallback if (file_changed_) { file_changed_ = false; TRY_ASSERT_MAIN_THREAD { - auto file_or_error = api.fileSystem().fileReadToEnd(filename_); - THROW_IF_STATUS_NOT_OK(file_or_error, throw); + auto file_or_error = api_.fileSystem().fileReadToEnd(filename_); + THROW_IF_STATUS_NOT_OK(file_or_error, throw); const std::string contents = file_or_error.value(); double pressure; if (absl::SimpleAtod(contents, &pressure)) { diff --git a/test/common/filesystem/filesystem_impl_test.cc b/test/common/filesystem/filesystem_impl_test.cc index f909d987cbb7..1e6e4b574300 100644 --- a/test/common/filesystem/filesystem_impl_test.cc +++ b/test/common/filesystem/filesystem_impl_test.cc @@ -82,7 +82,7 @@ TEST_F(FileSystemImplTest, FileReadToEndSuccess) { const std::string data = "test string\ntest"; const std::string file_path = TestEnvironment::writeStringToFileForTest("test_envoy", data); - EXPECT_EQ(data, file_system_.fileReadToEnd(file_path)); + EXPECT_EQ(data, file_system_.fileReadToEnd(file_path).value()); } // Files are read into std::string; verify that all bytes (e.g., non-ascii characters) come back @@ -94,7 +94,7 @@ TEST_F(FileSystemImplTest, FileReadToEndSuccessBinary) { } const std::string file_path = TestEnvironment::writeStringToFileForTest("test_envoy", data); - const std::string read = file_system_.fileReadToEnd(file_path); + const std::string read = file_system_.fileReadToEnd(file_path).value(); const std::vector binary_read(read.begin(), read.end()); EXPECT_EQ(binary_read.size(), 256); for (unsigned i = 0; i < 256; i++) { @@ -104,8 +104,9 @@ TEST_F(FileSystemImplTest, FileReadToEndSuccessBinary) { TEST_F(FileSystemImplTest, FileReadToEndDoesNotExist) { unlink(TestEnvironment::temporaryPath("envoy_this_not_exist").c_str()); - EXPECT_THROW(file_system_.fileReadToEnd(TestEnvironment::temporaryPath("envoy_this_not_exist")), - EnvoyException); + EXPECT_FALSE(file_system_.fileReadToEnd(TestEnvironment::temporaryPath("envoy_this_not_exist")) + .status() + .ok()); } #ifndef WIN32 @@ -117,14 +118,14 @@ TEST_F(FileSystemImplTest, DISABLED_FileReadToEndNotReadable) { std::filesystem::permissions(file_path, std::filesystem::perms::owner_all, std::filesystem::perm_options::remove); - EXPECT_THROW(file_system_.fileReadToEnd(file_path), EnvoyException); + EXPECT_FALSE(file_system_.fileReadToEnd(file_path).status().ok()); } #endif TEST_F(FileSystemImplTest, FileReadToEndDenylisted) { - EXPECT_THROW(file_system_.fileReadToEnd("/dev/urandom"), EnvoyException); - EXPECT_THROW(file_system_.fileReadToEnd("/proc/cpuinfo"), EnvoyException); - EXPECT_THROW(file_system_.fileReadToEnd("/sys/block/sda/dev"), EnvoyException); + EXPECT_FALSE(file_system_.fileReadToEnd("/dev/urandom").status().ok()); + EXPECT_FALSE(file_system_.fileReadToEnd("/proc/cpuinfo").status().ok()); + EXPECT_FALSE(file_system_.fileReadToEnd("/sys/block/sda/dev").status().ok()); } #ifndef WIN32 @@ -143,36 +144,36 @@ TEST_F(FileSystemImplTest, CanonicalPathFail) { TEST_F(FileSystemImplTest, SplitPathFromFilename) { PathSplitResult result; - result = file_system_.splitPathFromFilename("/foo/bar/baz"); + result = file_system_.splitPathFromFilename("/foo/bar/baz").value(); EXPECT_EQ(result.directory_, "/foo/bar"); EXPECT_EQ(result.file_, "baz"); - result = file_system_.splitPathFromFilename("/foo/bar"); + result = file_system_.splitPathFromFilename("/foo/bar").value(); EXPECT_EQ(result.directory_, "/foo"); EXPECT_EQ(result.file_, "bar"); - result = file_system_.splitPathFromFilename("/foo"); + result = file_system_.splitPathFromFilename("/foo").value(); EXPECT_EQ(result.directory_, "/"); EXPECT_EQ(result.file_, "foo"); - result = file_system_.splitPathFromFilename("/"); + result = file_system_.splitPathFromFilename("/").value(); EXPECT_EQ(result.directory_, "/"); EXPECT_EQ(result.file_, ""); - EXPECT_THROW(file_system_.splitPathFromFilename("nopathdelimeter"), EnvoyException); + EXPECT_FALSE(file_system_.splitPathFromFilename("nopathdelimeter").ok()); #ifdef WIN32 - result = file_system_.splitPathFromFilename("c:\\foo/bar"); + result = file_system_.splitPathFromFilename("c:\\foo/bar").value(); EXPECT_EQ(result.directory_, "c:\\foo"); EXPECT_EQ(result.file_, "bar"); - result = file_system_.splitPathFromFilename("c:/foo\\bar"); + result = file_system_.splitPathFromFilename("c:/foo\\bar").value(); EXPECT_EQ(result.directory_, "c:/foo"); EXPECT_EQ(result.file_, "bar"); - result = file_system_.splitPathFromFilename("c:\\foo"); + result = file_system_.splitPathFromFilename("c:\\foo").value(); EXPECT_EQ(result.directory_, "c:\\"); EXPECT_EQ(result.file_, "foo"); - result = file_system_.splitPathFromFilename("c:foo"); + result = file_system_.splitPathFromFilename("c:foo").value(); EXPECT_EQ(result.directory_, "c:"); EXPECT_EQ(result.file_, "foo"); - result = file_system_.splitPathFromFilename("c:"); + result = file_system_.splitPathFromFilename("c:").value(); EXPECT_EQ(result.directory_, "c:"); EXPECT_EQ(result.file_, ""); - result = file_system_.splitPathFromFilename("\\\\?\\C:\\"); + result = file_system_.splitPathFromFilename("\\\\?\\C:\\").value(); EXPECT_EQ(result.directory_, "\\\\?\\C:\\"); EXPECT_EQ(result.file_, ""); #endif diff --git a/test/common/http/http2/frame_replay.cc b/test/common/http/http2/frame_replay.cc index 8a604884330f..c2755a683d40 100644 --- a/test/common/http/http2/frame_replay.cc +++ b/test/common/http/http2/frame_replay.cc @@ -12,8 +12,10 @@ namespace Http { namespace Http2 { FileFrame::FileFrame(absl::string_view path) : api_(Api::createApiForTest()) { - const std::string contents = api_->fileSystem().fileReadToEnd( - TestEnvironment::runfilesPath("test/common/http/http2/" + std::string(path))); + const std::string contents = api_->fileSystem() + .fileReadToEnd(TestEnvironment::runfilesPath( + "test/common/http/http2/" + std::string(path))) + .value(); frame_.resize(contents.size()); contents.copy(reinterpret_cast(frame_.data()), frame_.size()); } diff --git a/test/common/quic/platform/quiche_test_output_impl.cc b/test/common/quic/platform/quiche_test_output_impl.cc index 0dc9207b3a38..fa8cbecf66ab 100644 --- a/test/common/quic/platform/quiche_test_output_impl.cc +++ b/test/common/quic/platform/quiche_test_output_impl.cc @@ -95,7 +95,7 @@ bool QuicheLoadTestOutputImpl(absl::string_view filename, std::string* data) { QUICHE_LOG(ERROR) << "Test output file does not exist: " << read_path; return false; } - *data = file_system.fileReadToEnd(read_path); + *data = file_system.fileReadToEnd(read_path).value(); return true; } diff --git a/test/common/secret/sds_api_test.cc b/test/common/secret/sds_api_test.cc index 9e92060f114e..891813b1de7e 100644 --- a/test/common/secret/sds_api_test.cc +++ b/test/common/secret/sds_api_test.cc @@ -197,9 +197,10 @@ class SdsRotationApiTest : public SdsApiTestBase { api_ = Api::createApiForTest(filesystem_); setupMocks(); EXPECT_CALL(filesystem_, splitPathFromFilename(_)) - .WillRepeatedly(Invoke([](absl::string_view path) -> Filesystem::PathSplitResult { - return Filesystem::fileSystemForTest().splitPathFromFilename(path); - })); + .WillRepeatedly( + Invoke([](absl::string_view path) -> absl::StatusOr { + return Filesystem::fileSystemForTest().splitPathFromFilename(path); + })); } Secret::MockSecretCallbacks secret_callback_; diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index 009eec46583f..9b3f7a773767 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -41,8 +41,8 @@ namespace { // asConfigYaml returns a new config that empties the configPath() and populates configYaml() OptionsImpl asConfigYaml(const OptionsImpl& src, Api::Api& api) { - return Envoy::Server::createTestOptionsImpl("", api.fileSystem().fileReadToEnd(src.configPath()), - src.localAddressIpVersion()); + return Envoy::Server::createTestOptionsImpl( + "", api.fileSystem().fileReadToEnd(src.configPath()).value(), src.localAddressIpVersion()); } static std::vector unsuported_win32_configs = { @@ -65,7 +65,7 @@ class ConfigTest { return api_->threadFactory(); })); ON_CALL(file_system_, fileReadToEnd(_)) - .WillByDefault(Invoke([&](const std::string& file) -> std::string { + .WillByDefault(Invoke([&](const std::string& file) -> absl::StatusOr { return api_->fileSystem().fileReadToEnd(file); })); ON_CALL(os_sys_calls_, close(_)).WillByDefault(Return(Api::SysCallIntResult{0, 0})); diff --git a/test/config_test/example_configs_test.cc b/test/config_test/example_configs_test.cc index 02d0504fbd9a..8802263f5e72 100644 --- a/test/config_test/example_configs_test.cc +++ b/test/config_test/example_configs_test.cc @@ -14,7 +14,8 @@ TEST(ExampleConfigsTest, All) { {TestEnvironment::runfilesPath("test/config_test/example_configs_test_setup.sh")}); Filesystem::InstanceImpl file_system; const auto config_file_count = std::stoi( - file_system.fileReadToEnd(TestEnvironment::temporaryDirectory() + "/config-file-count.txt")); + file_system.fileReadToEnd(TestEnvironment::temporaryDirectory() + "/config-file-count.txt") + .value()); // Change working directory, otherwise we won't be able to read files using relative paths. #ifdef PATH_MAX diff --git a/test/extensions/filters/http/grpc_field_extraction/filter_config_test.cc b/test/extensions/filters/http/grpc_field_extraction/filter_config_test.cc index 7d0e243ea225..55485685d229 100644 --- a/test/extensions/filters/http/grpc_field_extraction/filter_config_test.cc +++ b/test/extensions/filters/http/grpc_field_extraction/filter_config_test.cc @@ -51,8 +51,9 @@ using FilterConfigTestOk = FilterConfigTestBase; TEST_F(FilterConfigTestOk, DescriptorInline) { parseConfigProto(); *proto_config_.mutable_descriptor_set()->mutable_inline_bytes() = - api_->fileSystem().fileReadToEnd( - TestEnvironment::runfilesPath("test/proto/apikeys.descriptor")); + api_->fileSystem() + .fileReadToEnd(TestEnvironment::runfilesPath("test/proto/apikeys.descriptor")) + .value(); filter_config_ = std::make_unique(proto_config_, std::make_unique(), *api_); EXPECT_EQ(filter_config_->findExtractor("undefined"), nullptr); @@ -232,8 +233,9 @@ TEST_F(FilterConfigTestException, ErrorParsingDescriptorFromFile) { TEST_F(FilterConfigTestException, UnsupportedDescriptorSourceTyep) { parseConfigProto(); *proto_config_.mutable_descriptor_set()->mutable_inline_string() = - api_->fileSystem().fileReadToEnd( - TestEnvironment::runfilesPath("test/proto/apikeys.descriptor")); + api_->fileSystem() + .fileReadToEnd(TestEnvironment::runfilesPath("test/proto/apikeys.descriptor")) + .value(); EXPECT_THAT_THROWS_MESSAGE( std::make_unique(proto_config_, std::make_unique(), *api_), diff --git a/test/extensions/filters/http/grpc_field_extraction/filter_test.cc b/test/extensions/filters/http/grpc_field_extraction/filter_test.cc index 8f0dca099f03..886d72591247 100644 --- a/test/extensions/filters/http/grpc_field_extraction/filter_test.cc +++ b/test/extensions/filters/http/grpc_field_extraction/filter_test.cc @@ -45,8 +45,9 @@ class FilterTestBase : public ::testing::Test { ASSERT_TRUE(Protobuf::TextFormat::ParseFromString(config.empty() ? protoConfig() : config, &proto_config_)); *proto_config_.mutable_descriptor_set()->mutable_inline_bytes() = - api_->fileSystem().fileReadToEnd( - TestEnvironment::runfilesPath("test/proto/apikeys.descriptor")); + api_->fileSystem() + .fileReadToEnd(TestEnvironment::runfilesPath("test/proto/apikeys.descriptor")) + .value(); ON_CALL(mock_decoder_callbacks_, decoderBufferLimit()) .WillByDefault(testing::Return(UINT32_MAX)); filter_config_ = std::make_unique( diff --git a/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc b/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc index 488de5895508..4b940cdac87a 100644 --- a/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc +++ b/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc @@ -69,8 +69,10 @@ class GrpcJsonTranscoderConfigTest : public testing::Test, public GrpcJsonTransc std::string makeProtoDescriptor(std::function process) { FileDescriptorSet descriptor_set; - descriptor_set.ParseFromString(api_->fileSystem().fileReadToEnd( - TestEnvironment::runfilesPath("test/proto/bookstore.descriptor"))); + descriptor_set.ParseFromString( + api_->fileSystem() + .fileReadToEnd(TestEnvironment::runfilesPath("test/proto/bookstore.descriptor")) + .value()); process(descriptor_set); @@ -131,8 +133,10 @@ TEST_F(GrpcJsonTranscoderConfigTest, ParseConfigSkipRecalculating) { TEST_F(GrpcJsonTranscoderConfigTest, ParseBinaryConfig) { envoy::extensions::filters::http::grpc_json_transcoder::v3::GrpcJsonTranscoder proto_config; - proto_config.set_proto_descriptor_bin(api_->fileSystem().fileReadToEnd( - TestEnvironment::runfilesPath("test/proto/bookstore.descriptor"))); + proto_config.set_proto_descriptor_bin( + api_->fileSystem() + .fileReadToEnd(TestEnvironment::runfilesPath("test/proto/bookstore.descriptor")) + .value()); proto_config.add_services("bookstore.Bookstore"); EXPECT_NO_THROW(JsonTranscoderConfig config(proto_config, *api_)); } diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_proto_integration_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_proto_integration_test.cc index 9aaf9fee88d2..cc365938b0ef 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_proto_integration_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_proto_integration_test.cc @@ -238,7 +238,7 @@ TEST_P(ProxyProtoTcpIntegrationTest, AccessLog) { std::string log_result; // Access logs only get flushed to disk periodically, so poll until the log is non-empty do { - log_result = api_->fileSystem().fileReadToEnd(access_log_path); + log_result = api_->fileSystem().fileReadToEnd(access_log_path).value(); } while (log_result.empty()); EXPECT_EQ(log_result, "remote=1.2.3.4:12345 local=254.254.254.254:1234"); diff --git a/test/extensions/filters/network/thrift_proxy/integration.cc b/test/extensions/filters/network/thrift_proxy/integration.cc index 236750130871..c2d5239fa274 100644 --- a/test/extensions/filters/network/thrift_proxy/integration.cc +++ b/test/extensions/filters/network/thrift_proxy/integration.cc @@ -102,7 +102,7 @@ void BaseThriftIntegrationTest::preparePayloads(const PayloadOptions& options, void BaseThriftIntegrationTest::readAll(std::string file, Buffer::Instance& buffer) { file = TestEnvironment::substitute(file, version_); - std::string data = api_->fileSystem().fileReadToEnd(file); + std::string data = api_->fileSystem().fileReadToEnd(file).value(); buffer.add(data); } diff --git a/test/extensions/transport_sockets/tls/test_private_key_method_provider.cc b/test/extensions/transport_sockets/tls/test_private_key_method_provider.cc index 1f01175716ea..3844314ce60a 100644 --- a/test/extensions/transport_sockets/tls/test_private_key_method_provider.cc +++ b/test/extensions/transport_sockets/tls/test_private_key_method_provider.cc @@ -350,8 +350,11 @@ TestPrivateKeyMethodProvider::TestPrivateKeyMethodProvider( } } - std::string private_key = - factory_context.serverFactoryContext().api().fileSystem().fileReadToEnd(private_key_path); + std::string private_key = factory_context.serverFactoryContext() + .api() + .fileSystem() + .fileReadToEnd(private_key_path) + .value(); bssl::UniquePtr bio( BIO_new_mem_buf(const_cast(private_key.data()), private_key.size())); bssl::UniquePtr pkey(PEM_read_bio_PrivateKey(bio.get(), nullptr, nullptr, nullptr)); diff --git a/test/fuzz/main.cc b/test/fuzz/main.cc index 07e4952369a6..976006275c2b 100644 --- a/test/fuzz/main.cc +++ b/test/fuzz/main.cc @@ -45,7 +45,7 @@ class FuzzerCorpusTest : public testing::TestWithParam { TEST_P(FuzzerCorpusTest, RunOneCorpusFile) { ENVOY_LOG_MISC(info, "Corpus file: {}", GetParam()); - const std::string buf = api_->fileSystem().fileReadToEnd(GetParam()); + const std::string buf = api_->fileSystem().fileReadToEnd(GetParam()).value(); // Everything from here on is the same as under the fuzzer lib. LLVMFuzzerTestOneInput(reinterpret_cast(buf.c_str()), buf.size()); } diff --git a/test/integration/admin_html/test_server.cc b/test/integration/admin_html/test_server.cc index 2a9b2107d0fc..20ebe1b73621 100644 --- a/test/integration/admin_html/test_server.cc +++ b/test/integration/admin_html/test_server.cc @@ -39,7 +39,11 @@ Http::Code testCallback(Http::ResponseHeaderMap& response_headers, Buffer::Insta Filesystem::InstanceImpl file_system; std::string path = absl::StrCat(prefix, leaf); - TRY_ASSERT_MAIN_THREAD { response.add(file_system.fileReadToEnd(path)); } + TRY_ASSERT_MAIN_THREAD { + auto file_or_error = file_system.fileReadToEnd(path); + THROW_IF_STATUS_NOT_OK(file_or_error, throw); + response.add(file_or_error.value()); + } END_TRY catch (EnvoyException& e) { response.add(e.what()); @@ -61,7 +65,7 @@ class DebugHtmlResourceProvider : public Server::AdminHtmlUtil::ResourceProvider std::string path = absl::StrCat("source/server/admin/html/", resource_name); Filesystem::InstanceImpl file_system; TRY_ASSERT_MAIN_THREAD { - buf = file_system.fileReadToEnd(path); + buf = file_system.fileReadToEnd(path).value(); ENVOY_LOG_MISC(info, "Read {} bytes from {}", buf.size(), path); } END_TRY diff --git a/test/mocks/filesystem/mocks.h b/test/mocks/filesystem/mocks.h index c3fbf05ab894..524cefd269f3 100644 --- a/test/mocks/filesystem/mocks.h +++ b/test/mocks/filesystem/mocks.h @@ -64,7 +64,7 @@ class MockInstance : public Instance { MOCK_METHOD(bool, directoryExists, (const std::string&)); MOCK_METHOD(ssize_t, fileSize, (const std::string&)); MOCK_METHOD(absl::StatusOr, fileReadToEnd, (const std::string&)); - MOCK_METHOD(PathSplitResult, splitPathFromFilename, (absl::string_view)); + MOCK_METHOD(absl::StatusOr, splitPathFromFilename, (absl::string_view)); MOCK_METHOD(bool, illegalPath, (const std::string&)); MOCK_METHOD(Api::IoCallResult, stat, (absl::string_view)); MOCK_METHOD(Api::IoCallBoolResult, createPath, (absl::string_view)); diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 8ade9b1d649c..5035a710cf38 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -1227,13 +1227,13 @@ TEST_P(ServerInstanceImplTest, LogToFile) { GET_MISC_LOGGER().set_level(spdlog::level::info); ENVOY_LOG_MISC(warn, "LogToFile test string"); Logger::Registry::getSink()->flush(); - std::string log = server_->api().fileSystem().fileReadToEnd(path); + std::string log = server_->api().fileSystem().fileReadToEnd(path).value(); EXPECT_GT(log.size(), 0); EXPECT_TRUE(log.find("LogToFile test string") != std::string::npos); // Test that critical messages get immediately flushed ENVOY_LOG_MISC(critical, "LogToFile second test string"); - log = server_->api().fileSystem().fileReadToEnd(path); + log = server_->api().fileSystem().fileReadToEnd(path).value(); EXPECT_TRUE(log.find("LogToFile second test string") != std::string::npos); } diff --git a/test/test_common/environment.cc b/test/test_common/environment.cc index a43f0cdea647..8eacacd5d015 100644 --- a/test/test_common/environment.cc +++ b/test/test_common/environment.cc @@ -98,7 +98,7 @@ void TestEnvironment::createPath(const std::string& path) { return; } const Filesystem::PathSplitResult parent = - Filesystem::fileSystemForTest().splitPathFromFilename(path); + Filesystem::fileSystemForTest().splitPathFromFilename(path).value(); if (parent.file_.length() > 0) { TestEnvironment::createPath(std::string(parent.directory_)); } @@ -357,7 +357,7 @@ std::string TestEnvironment::temporaryFileSubstitute(const std::string& path, } std::string TestEnvironment::readFileToStringForTest(const std::string& filename) { - return Filesystem::fileSystemForTest().fileReadToEnd(filename); + return Filesystem::fileSystemForTest().fileReadToEnd(filename).value(); } std::string TestEnvironment::temporaryFileSubstitute(const std::string& path, @@ -385,7 +385,7 @@ std::string TestEnvironment::temporaryFileSubstitute(const std::string& path, // Substitute paths and other common things. out_json_string = substitute(out_json_string, version); - auto name = Filesystem::fileSystemForTest().splitPathFromFilename(path).file_; + auto name = Filesystem::fileSystemForTest().splitPathFromFilename(path).value().file_; const std::string extension = std::string(name.substr(name.rfind('.'))); const std::string out_json_path = TestEnvironment::temporaryPath(name) + ".with.ports" + extension; diff --git a/test/test_common/file_system_for_test.cc b/test/test_common/file_system_for_test.cc index 0101c2ce2620..e15b41dcf8da 100644 --- a/test/test_common/file_system_for_test.cc +++ b/test/test_common/file_system_for_test.cc @@ -19,7 +19,7 @@ struct MemFileInfo { FileInfo toFileInfo(absl::string_view path) { absl::MutexLock lock(&lock_); return { - std::string{fileSystemForTest().splitPathFromFilename(path).file_}, + std::string{fileSystemForTest().splitPathFromFilename(path).value().file_}, data_.length(), FileType::Regular, create_time_, diff --git a/test/test_common/file_system_for_test.h b/test/test_common/file_system_for_test.h index 7b629cf75eae..ce457031799b 100644 --- a/test/test_common/file_system_for_test.h +++ b/test/test_common/file_system_for_test.h @@ -32,7 +32,7 @@ class MemfileInstanceImpl : public Instance { absl::StatusOr fileReadToEnd(const std::string& path) override; - PathSplitResult splitPathFromFilename(absl::string_view path) override { + absl::StatusOr splitPathFromFilename(absl::string_view path) override { return file_system_->splitPathFromFilename(path); } diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 4db04cffde53..5f5335023201 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -226,7 +226,7 @@ RouterCheckTool::RouterCheckTool( } Json::ObjectSharedPtr loadFromFile(const std::string& file_path, Api::Api& api) { - std::string contents = api.fileSystem().fileReadToEnd(file_path); + std::string contents = api.fileSystem().fileReadToEnd(file_path).value(); if (absl::EndsWith(file_path, ".yaml")) { contents = MessageUtil::getJsonStringFromMessageOrError(ValueUtil::loadFromYaml(contents)); } @@ -238,7 +238,7 @@ RouterCheckTool::compareEntries(const std::string& expected_routes) { envoy::RouterCheckToolSchema::Validation validation_config; auto stats = std::make_unique(); auto api = Api::createApiForTest(*stats); - const std::string contents = api->fileSystem().fileReadToEnd(expected_routes); + const std::string contents = api->fileSystem().fileReadToEnd(expected_routes).value(); TestUtility::loadFromFile(expected_routes, validation_config, *api); TestUtility::validate(validation_config); diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml index dd28952a5dda..b48f2a46dcc0 100644 --- a/tools/code_format/config.yaml +++ b/tools/code_format/config.yaml @@ -157,10 +157,8 @@ paths: - source/common/rds/rds_route_config_subscription.cc - source/common/filesystem/inotify/watcher_impl.cc - source/common/filesystem/posix/directory_iterator_impl.cc - - source/common/filesystem/posix/filesystem_impl.cc - source/common/filesystem/kqueue/watcher_impl.cc - source/common/filesystem/win32/directory_iterator_impl.cc - - source/common/filesystem/win32/filesystem_impl.cc - source/common/filesystem/win32/watcher_impl.cc - source/common/common/utility.cc - source/common/common/regex.cc