From 9d578d61c752302a684910f00d8f157b99c8fb1b Mon Sep 17 00:00:00 2001 From: Raven Black Date: Fri, 30 Jan 2026 22:41:30 +0000 Subject: [PATCH 01/22] Add file server filter Signed-off-by: Raven Black --- CODEOWNERS | 1 + .../filters/http/file_server/v3/BUILD | 13 + .../http/file_server/v3/file_server.proto | 90 ++++ api/versioning/BUILD | 1 + bazel/repositories.bzl | 1 + .../http/http_filters/file_server_filter.rst | 20 + .../http/http_filters/http_filters.rst | 1 + source/extensions/extensions_build_config.bzl | 1 + source/extensions/extensions_metadata.yaml | 7 + .../extensions/filters/http/file_server/BUILD | 46 ++ .../filters/http/file_server/config.cc | 90 ++++ .../filters/http/file_server/config.h | 36 ++ .../filters/http/file_server/file_streamer.cc | 216 +++++++++ .../filters/http/file_server/file_streamer.h | 70 +++ .../filters/http/file_server/filter.cc | 141 ++++++ .../filters/http/file_server/filter.h | 58 +++ .../filters/http/file_server/filter_config.cc | 116 +++++ .../filters/http/file_server/filter_config.h | 54 +++ .../extensions/filters/http/file_server/BUILD | 46 ++ .../filters/http/file_server/config_test.cc | 212 ++++++++ .../filters/http/file_server/filter_test.cc | 459 ++++++++++++++++++ .../http/file_server/integration_test.cc | 173 +++++++ 22 files changed, 1852 insertions(+) create mode 100644 api/envoy/extensions/filters/http/file_server/v3/BUILD create mode 100644 api/envoy/extensions/filters/http/file_server/v3/file_server.proto create mode 100644 docs/root/configuration/http/http_filters/file_server_filter.rst create mode 100644 source/extensions/filters/http/file_server/BUILD create mode 100644 source/extensions/filters/http/file_server/config.cc create mode 100644 source/extensions/filters/http/file_server/config.h create mode 100644 source/extensions/filters/http/file_server/file_streamer.cc create mode 100644 source/extensions/filters/http/file_server/file_streamer.h create mode 100644 source/extensions/filters/http/file_server/filter.cc create mode 100644 source/extensions/filters/http/file_server/filter.h create mode 100644 source/extensions/filters/http/file_server/filter_config.cc create mode 100644 source/extensions/filters/http/file_server/filter_config.h create mode 100644 test/extensions/filters/http/file_server/BUILD create mode 100644 test/extensions/filters/http/file_server/config_test.cc create mode 100644 test/extensions/filters/http/file_server/filter_test.cc create mode 100644 test/extensions/filters/http/file_server/integration_test.cc diff --git a/CODEOWNERS b/CODEOWNERS index 2db1acffc9c33..9770a584ee3ed 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -261,6 +261,7 @@ extensions/upstreams/tcp @ggreenway @mattklein123 /*/extensions/config/validators/minimum_clusters @adisuissa @yanavlasov # File system based extensions /*/extensions/common/async_files @mattklein123 @ravenblackx +/*/extensions/filters/http/file_server @ravenblackx @phlax /*/extensions/filters/http/file_system_buffer @mattklein123 @ravenblackx /*/extensions/http/cache/file_system_http_cache @ggreenway @UNOWNED /*/extensions/http/cache_v2/file_system_http_cache @ggreenway @ravenblackx diff --git a/api/envoy/extensions/filters/http/file_server/v3/BUILD b/api/envoy/extensions/filters/http/file_server/v3/BUILD new file mode 100644 index 0000000000000..5b108dcfee6c7 --- /dev/null +++ b/api/envoy/extensions/filters/http/file_server/v3/BUILD @@ -0,0 +1,13 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = [ + "//envoy/extensions/common/async_files/v3:pkg", + "@com_github_cncf_xds//udpa/annotations:pkg", + "@com_github_cncf_xds//xds/annotations/v3:pkg", + ], +) diff --git a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto new file mode 100644 index 0000000000000..d6792895ff034 --- /dev/null +++ b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto @@ -0,0 +1,90 @@ +syntax = "proto3"; + +package envoy.extensions.filters.http.file_server.v3; + +import "envoy/extensions/common/async_files/v3/async_file_manager.proto"; + +import "xds/annotations/v3/status.proto"; + +import "udpa/annotations/status.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.filters.http.file_server.v3"; +option java_outer_classname = "FileServerProto"; +option java_multiple_files = true; +option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/file_server/v3;file_serverv3"; +option (udpa.annotations.file_status).package_version_status = ACTIVE; +option (xds.annotations.v3.file_status).work_in_progress = true; + +// [#protodoc-title: FileServerConfig] +// [#extension: envoy.filters.http.file_server] + +// A :ref:`file server ` filter configuration. +// [#next-free-field: 6] +message FileServerConfig { + message PathMapping { + // If no ``path_prefix`` is matched, the filter does not intercept a request. + // + // If a ``path_prefix`` is matched, that prefix is removed from the request + // and replaced with ``filesystem_prefix`` to form a filesystem path for + // the request. + // + // Prefix ``/`` will match all GET requests. + string path_prefix = 1 [(validate.rules).string = {min_len: 1}]; + + // Replaces a matched ``path_prefix`` to form a filesystem path for a + // request. May be relative to the working directory of the envoy execution, + // or an absolute path. + string filesystem_prefix = 2 [(validate.rules).string = {min_len: 1}]; + } + + message DirectoryBehavior { + message DirectoryList { + } + + oneof behavior_type { + option (validate.required) = true; + + // Attempts to serve the given file within the directory, e.g. ``index.html``. + string try_file = 1 [(validate.rules).string = {pattern: "^[^/]+$"}]; + + // Responds with an html formatted list of the files in the directory. + // [#not-implemented-hide:] Directory operations currently have no async implementation. + DirectoryList directory_list = 2; + } + } + + // A configuration for an AsyncFileManager. + // + // This can be unset to allow for per-route-only configs. If this is unset + // on a per-route config and set in the route config, the version in the + // route config is used. If this is unset either way when a ``path_mapping`` + // is matched, a 503 Internal Server Error response is served. + common.async_files.v3.AsyncFileManagerConfig manager_config = 1; + + // The longest matching path_mapping takes precedence. + repeated PathMapping path_mappings = 2; + + // A map from filename suffix to content type header. + // e.g. {"txt": "text/plain"} + // + // File suffixes may not contain ``.`` as the filename suffix after + // the last ``.`` is used to perform an O(1) lookup against the keys. + // + // An empty string suffix will only match files ending with a ``.``. + // + // Files with no suffix (e.g. ``README``) can be matched as the full string. + map content_types = 3; + + // If ``content_types`` does not contain a match for a file suffix, + // ``default_content_type`` is used. + // + // If there is no match and ``default_content_type`` is empty, the + // ``content-type`` header will be omitted from the response. + string default_content_type = 4; + + // If the requested path refers to a directory, the given behaviors are + // tried in order until one succeeds. If the end of the list is reached + // with no success, the result is a 403 Forbidden. + repeated DirectoryBehavior directory_behaviors = 5; +} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index dc7d01f7bd8c3..08194c44c3ade 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -137,6 +137,7 @@ proto_library( "//envoy/extensions/filters/http/ext_authz/v3:pkg", "//envoy/extensions/filters/http/ext_proc/v3:pkg", "//envoy/extensions/filters/http/fault/v3:pkg", + "//envoy/extensions/filters/http/file_server/v3:pkg", "//envoy/extensions/filters/http/file_system_buffer/v3:pkg", "//envoy/extensions/filters/http/gcp_authn/v3:pkg", "//envoy/extensions/filters/http/geoip/v3:pkg", diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 5da178f85b8d8..ef0359f7db73c 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -8,6 +8,7 @@ PPC_SKIP_TARGETS = ["envoy.string_matcher.lua", "envoy.filters.http.lua", "envoy WINDOWS_SKIP_TARGETS = [ "envoy.extensions.http.cache.file_system_http_cache", "envoy.extensions.http.cache_v2.file_system_http_cache", + "envoy.filters.http.file_server", "envoy.filters.http.file_system_buffer", "envoy.filters.http.language", "envoy.filters.http.sxg", diff --git a/docs/root/configuration/http/http_filters/file_server_filter.rst b/docs/root/configuration/http/http_filters/file_server_filter.rst new file mode 100644 index 0000000000000..c81fec0ff5535 --- /dev/null +++ b/docs/root/configuration/http/http_filters/file_server_filter.rst @@ -0,0 +1,20 @@ +.. _config_http_filters_file_server: + +File Server +=========== + +The file server filter can be used to respond with the contents of a file from the filesystem. + +The ``content-length`` header will be the size of the file. + +The ``content-type`` header will be set based on filename suffix and filter configuration. + +.. note:: + + This filter is not yet supported on Windows. + +Configuration +------------- + +* This filter should be configured with the type URL ``type.googleapis.com/envoy.extensions.filters.http.file_server.v3.FileServerConfig``. +* :ref:`v3 API reference ` diff --git a/docs/root/configuration/http/http_filters/http_filters.rst b/docs/root/configuration/http/http_filters/http_filters.rst index 60dd52c504519..768008ce2c18f 100644 --- a/docs/root/configuration/http/http_filters/http_filters.rst +++ b/docs/root/configuration/http/http_filters/http_filters.rst @@ -31,6 +31,7 @@ HTTP filters ext_authz_filter ext_proc_filter fault_filter + file_server_filter file_system_buffer_filter gcp_authn_filter geoip_filter diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index 829ac71727d7a..ebbca2993777c 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -173,6 +173,7 @@ EXTENSIONS = { "envoy.filters.http.ext_authz": "//source/extensions/filters/http/ext_authz:config", "envoy.filters.http.ext_proc": "//source/extensions/filters/http/ext_proc:config", "envoy.filters.http.fault": "//source/extensions/filters/http/fault:config", + "envoy.filters.http.file_server": "//source/extensions/filters/http/file_server:config", "envoy.filters.http.file_system_buffer": "//source/extensions/filters/http/file_system_buffer:config", "envoy.filters.http.gcp_authn": "//source/extensions/filters/http/gcp_authn:config", "envoy.filters.http.geoip": "//source/extensions/filters/http/geoip:config", diff --git a/source/extensions/extensions_metadata.yaml b/source/extensions/extensions_metadata.yaml index 1322a4b4920fb..00570785ff066 100644 --- a/source/extensions/extensions_metadata.yaml +++ b/source/extensions/extensions_metadata.yaml @@ -447,6 +447,13 @@ envoy.filters.http.fault: status: stable type_urls: - envoy.extensions.filters.http.fault.v3.HTTPFault +envoy.filters.http.file_server: + categories: + - envoy.filters.http + security_posture: unknown + status: wip + type_urls: + - envoy.extensions.filters.http.file_server.v3.FileServerConfig envoy.filters.http.file_system_buffer: categories: - envoy.filters.http diff --git a/source/extensions/filters/http/file_server/BUILD b/source/extensions/filters/http/file_server/BUILD new file mode 100644 index 0000000000000..581407a4e9a4e --- /dev/null +++ b/source/extensions/filters/http/file_server/BUILD @@ -0,0 +1,46 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_cc_library", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_library( + name = "file_server_lib", + srcs = [ + "file_streamer.cc", + "filter.cc", + "filter_config.cc", + ], + hdrs = [ + "file_streamer.h", + "filter.h", + "filter_config.h", + ], + deps = [ + "//envoy/buffer:buffer_interface", + "//envoy/server:instance_interface", + "//source/common/common:enum_to_int", + "//source/common/common:radix_tree_lib", + "//source/common/http:codes_lib", + "//source/common/http:header_map_lib", + "//source/extensions/common/async_files", + "//source/extensions/filters/http/common:pass_through_filter_lib", + "@envoy_api//envoy/extensions/filters/http/file_server/v3:pkg_cc_proto", + ], +) + +envoy_cc_extension( + name = "config", + srcs = ["config.cc"], + hdrs = ["config.h"], + deps = [ + ":file_server_lib", + "//source/extensions/filters/http/common:factory_base_lib", + "@envoy_api//envoy/extensions/filters/http/file_server/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/filters/http/file_server/config.cc b/source/extensions/filters/http/file_server/config.cc new file mode 100644 index 0000000000000..ee7ea06cdbe85 --- /dev/null +++ b/source/extensions/filters/http/file_server/config.cc @@ -0,0 +1,90 @@ +#include "source/extensions/filters/http/file_server/config.h" + +#include +#include +#include + +#include "envoy/extensions/filters/http/file_server/v3/file_server.pb.validate.h" + +#include "source/extensions/filters/http/file_server/filter.h" +#include "source/extensions/filters/http/file_server/filter_config.h" + +#include "absl/container/flat_hash_map.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +namespace { +absl::Status validateProto(const ProtoFileServerConfig& config) { + absl::flat_hash_set seen; + bool inserted; + for (const auto& mapping : config.path_mappings()) { + std::tie(std::ignore, inserted) = seen.emplace(mapping.path_prefix()); + if (!inserted) { + return absl::InvalidArgumentError( + absl::StrCat("duplicate path_prefix: ", mapping.path_prefix())); + } + } + seen.erase(seen.begin(), seen.end()); + bool directory_tried = false; + for (const auto& directory_behavior : config.directory_behaviors()) { + if (!directory_behavior.try_file().empty()) { + std::tie(std::ignore, inserted) = seen.emplace(directory_behavior.try_file()); + if (!inserted) { + return absl::InvalidArgumentError(absl::StrCat( + "duplicate try_file in directory_behaviors: ", directory_behavior.try_file())); + } + } else { + if (directory_tried) { + return absl::InvalidArgumentError("multiple directory_list directives"); + } + directory_tried = true; + } + } + for (const auto& content_type_pair : config.content_types()) { + if (content_type_pair.first.find(".") != std::string::npos) { + return absl::InvalidArgumentError(absl::StrCat( + "file suffix in content_types may not contain a period: ", content_type_pair.first)); + } + } + return absl::OkStatus(); +} +} // namespace + +FileServerFilterFactory::FileServerFilterFactory() + : DualFactoryBase(FileServerFilter::filterName()) {} + +absl::StatusOr FileServerFilterFactory::createFilterFactoryFromProtoTyped( + const ProtoFileServerConfig& config, const std::string&, DualInfo, + Server::Configuration::ServerFactoryContext& context) { + RETURN_IF_NOT_OK(validateProto(config)); + auto file_server_config = FileServerConfig::create(config, context); + if (!file_server_config.ok()) { + return file_server_config.status(); + } + return [fsc = std::move(file_server_config.value())]( + Http::FilterChainFactoryCallbacks& callbacks) -> void { + callbacks.addStreamDecoderFilter(FileServerFilter::createShared(fsc)); + }; +} + +absl::StatusOr +FileServerFilterFactory::createRouteSpecificFilterConfigTyped( + const ProtoFileServerConfig& config, Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor&) { + RETURN_IF_NOT_OK(validateProto(config)); + auto file_server_config = FileServerConfig::create(config, context); + if (!file_server_config.ok()) { + return file_server_config.status(); + } + return file_server_config.value(); +} + +REGISTER_FACTORY(FileServerFilterFactory, Server::Configuration::NamedHttpFilterConfigFactory); + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/file_server/config.h b/source/extensions/filters/http/file_server/config.h new file mode 100644 index 0000000000000..f2c14dcbdc954 --- /dev/null +++ b/source/extensions/filters/http/file_server/config.h @@ -0,0 +1,36 @@ +#pragma once + +#include + +#include "envoy/extensions/filters/http/file_server/v3/file_server.pb.h" + +#include "source/extensions/filters/http/common/factory_base.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +using ProtoFileServerConfig = envoy::extensions::filters::http::file_server::v3::FileServerConfig; + +class FileServerFilterFactory + : public Extensions::HttpFilters::Common::DualFactoryBase { +public: + FileServerFilterFactory(); + + absl::StatusOr + createFilterFactoryFromProtoTyped(const ProtoFileServerConfig& config, + const std::string& stats_prefix, DualInfo info, + Server::Configuration::ServerFactoryContext& context) override; + + absl::StatusOr + createRouteSpecificFilterConfigTyped(const ProtoFileServerConfig& config, + Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validator) override; +}; + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/file_server/file_streamer.cc b/source/extensions/filters/http/file_server/file_streamer.cc new file mode 100644 index 0000000000000..a17ce61a68767 --- /dev/null +++ b/source/extensions/filters/http/file_server/file_streamer.cc @@ -0,0 +1,216 @@ +#include "source/extensions/filters/http/file_server/file_streamer.h" + +#include "envoy/http/codes.h" + +#include "source/common/common/enum_to_int.h" +#include "source/common/http/header_map_impl.h" +#include "source/extensions/common/async_files/async_file_manager.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +namespace { +Http::Code abslStatusToHttpStatus(absl::StatusCode code) { + switch (code) { + case absl::StatusCode::kOk: + return Http::Code::OK; + case absl::StatusCode::kCancelled: + return static_cast(499); + case absl::StatusCode::kUnknown: + return Http::Code::InternalServerError; + case absl::StatusCode::kInvalidArgument: + return Http::Code::BadRequest; + case absl::StatusCode::kDeadlineExceeded: + return Http::Code::GatewayTimeout; + case absl::StatusCode::kNotFound: + return Http::Code::NotFound; + case absl::StatusCode::kAlreadyExists: + return Http::Code::Conflict; + case absl::StatusCode::kPermissionDenied: + return Http::Code::Forbidden; + case absl::StatusCode::kResourceExhausted: + return Http::Code::TooManyRequests; + case absl::StatusCode::kFailedPrecondition: + return Http::Code::BadRequest; + case absl::StatusCode::kAborted: + return Http::Code::Conflict; + case absl::StatusCode::kOutOfRange: + return Http::Code::RangeNotSatisfiable; + case absl::StatusCode::kUnimplemented: + return Http::Code::ServiceUnavailable; + case absl::StatusCode::kDataLoss: + return Http::Code::InternalServerError; + case absl::StatusCode::kUnauthenticated: + return Http::Code::Unauthorized; + default: + return Http::Code::InternalServerError; + } +} + +const Http::LowerCaseString& acceptRangesHeaderKey() { + CONSTRUCT_ON_FIRST_USE(Http::LowerCaseString, "accept-ranges"); +} + +} // namespace + +void FileStreamer::begin(const FileServerConfig& config, Event::Dispatcher& dispatcher, + uint64_t start, uint64_t end, std::filesystem::path file_path) { + file_server_config_ = &config; + dispatcher_ = &dispatcher; + pos_ = start; + end_ = end; + file_path_ = std::move(file_path); + cancel_callback_ = file_server_config_->asyncFileManager()->stat( + dispatcher_, file_path_.string(), [this](absl::StatusOr result) { + if (!result.ok()) { + client_.errorFromFile(abslStatusToHttpStatus(result.status().code()), + absl::StrCat("file_server_stat_error")); + return; + } + const struct stat& s = result.value(); + if (S_ISDIR(s.st_mode)) { + startDir(0); + return; + } + cancel_callback_ = file_server_config_->asyncFileManager()->openExistingFile( + dispatcher_, file_path_.string(), Common::AsyncFiles::AsyncFileManager::Mode::ReadOnly, + [this](absl::StatusOr result) { + if (!result.ok()) { + client_.errorFromFile(abslStatusToHttpStatus(result.status().code()), + absl::StrCat("file_server_open_error")); + return; + } + async_file_ = std::move(result.value()); + startFile(); + }); + }); +} + +void FileStreamer::startDir(int behavior_index) { + OptRef behavior = + file_server_config_->directoryBehavior(behavior_index); + if (!behavior) { + client_.errorFromFile(Http::Code::Forbidden, "file_server_no_valid_directory_behavior"); + return; + } + switch (behavior->behavior_type_case()) { + case ProtoFileServerConfig::DirectoryBehavior::kTryFile: + cancel_callback_ = file_server_config_->asyncFileManager()->openExistingFile( + dispatcher_, (file_path_ / std::filesystem::path{behavior->try_file()}).string(), + Common::AsyncFiles::AsyncFileManager::Mode::ReadOnly, + [this, behavior_index](absl::StatusOr result) { + if (!result.ok()) { + // Try the next directoryBehavior. + // Since the action is dispatched, this isn't recursion. + return startDir(behavior_index + 1); + } + file_path_ = + file_path_ / std::filesystem::path{ + file_server_config_->directoryBehavior(behavior_index)->try_file()}; + async_file_ = std::move(result.value()); + startFile(); + }); + return; + case ProtoFileServerConfig::DirectoryBehavior::kDirectoryList: + client_.errorFromFile(Http::Code::Forbidden, "file_server_directory_list_not_implemented"); + return; + case ProtoFileServerConfig::DirectoryBehavior::BEHAVIOR_TYPE_NOT_SET: + // Unreachable due to proto validations. + client_.errorFromFile(Http::Code::InternalServerError, "file_server_empty_behavior_type"); + return; + } +} + +void FileStreamer::startFile() { + ASSERT(async_file_); + auto queued = async_file_->stat(dispatcher_, [this](absl::StatusOr result) { + if (!result.ok()) { + client_.errorFromFile(abslStatusToHttpStatus(result.status().code()), + "file_server_opened_file_stat_failed"); + return; + } + const struct stat& s = result.value(); + if (static_cast(s.st_size) < end_) { + client_.errorFromFile(Http::Code::RangeNotSatisfiable, "file_server_range_not_satisfiable"); + return; + } + auto headers = Http::ResponseHeaderMapImpl::create(); + headers->setReference(acceptRangesHeaderKey(), "bytes"); + if (pos_ || end_) { + // Range request gets PartialContent, a content-range, and reduced content-length header. + if (!end_) { + end_ = s.st_size; + } + headers->setContentLength(end_ - pos_); + // Subtract one from end_ in this header because range headers use [start, end) vs. + // end_ is in normal programmer [start, end] style. + headers->setReferenceKey(Envoy::Http::Headers::get().ContentRange, + absl::StrCat("bytes ", pos_, "-", end_ - 1, "/", s.st_size)); + headers->setStatus(enumToInt(Http::Code::PartialContent)); + } else { + end_ = s.st_size; + headers->setContentLength(s.st_size); + headers->setStatus(enumToInt(Http::Code::OK)); + } + absl::string_view ct = file_server_config_->contentTypeForPath(file_path_); + if (!ct.empty()) { + headers->setContentType(ct); + } + if (client_.headersFromFile(std::move(headers))) { + readBodyChunk(); + } + }); + ASSERT(queued.ok()); + cancel_callback_ = std::move(queued.value()); +} + +void FileStreamer::pause() { paused_ = true; } + +void FileStreamer::unpause() { + if (paused_) { + paused_ = false; + if (action_has_been_postponed_by_pause_) { + action_has_been_postponed_by_pause_ = false; + readBodyChunk(); + } + } +} + +void FileStreamer::readBodyChunk() { + ASSERT(async_file_); + static const uint64_t kMaxReadSize = 32 * 1024; + uint64_t sz = std::min(end_ - pos_, kMaxReadSize); + auto queued = + async_file_->read(dispatcher_, pos_, sz, [this](absl::StatusOr result) { + if (!result.ok()) { + client_.errorFromFile(abslStatusToHttpStatus(result.status().code()), + "file_server_read_operation_failed"); + return; + } + Buffer::InstancePtr buf = std::move(result.value()); + pos_ += buf->length(); + client_.bodyChunkFromFile(std::move(buf), pos_ == end_); + if (!paused_ && pos_ != end_) { + readBodyChunk(); + } else if (paused_) { + action_has_been_postponed_by_pause_ = true; + } + }); + ASSERT(queued.ok()); + cancel_callback_ = std::move(queued.value()); +} + +void FileStreamer::abort() { cancel_callback_(); } + +FileStreamer::~FileStreamer() { + if (async_file_) { + async_file_->close(nullptr, [](absl::Status) {}).IgnoreError(); + } +} + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/file_server/file_streamer.h b/source/extensions/filters/http/file_server/file_streamer.h new file mode 100644 index 0000000000000..5371f138a02d3 --- /dev/null +++ b/source/extensions/filters/http/file_server/file_streamer.h @@ -0,0 +1,70 @@ +#pragma once + +#include "envoy/buffer/buffer.h" +#include "envoy/http/codes.h" +#include "envoy/http/header_map.h" + +#include "source/extensions/common/async_files/async_file_manager.h" +#include "source/extensions/filters/http/file_server/filter_config.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +using Extensions::Common::AsyncFiles::AsyncFileHandle; +using Extensions::Common::AsyncFiles::AsyncFileManager; +using Extensions::Common::AsyncFiles::CancelFunction; + +class FileStreamerClient { +public: + virtual void errorFromFile(Http::Code code, absl::string_view log_message) PURE; + // Return true to keep going - false is for HEAD requests. + virtual bool headersFromFile(Http::ResponseHeaderMapPtr response_headers) PURE; + virtual void bodyChunkFromFile(Buffer::InstancePtr buf, bool end_stream) PURE; +}; + +class FileStreamer { +public: + explicit FileStreamer(FileStreamerClient& client) : client_(client) {} + ~FileStreamer(); + // Starts reading and streaming the file. + // end == 0 means read to end of file. + void begin(const FileServerConfig& config, Event::Dispatcher& dispatcher, uint64_t start, + uint64_t end, std::filesystem::path file_path); + // Call when the downstream buffer is over watermark. + // Stops at the completion of the current action if not unpaused first. + void pause(); + // Call when the downstream buffer is under watermark. + // Starts the next action if previously paused. + void unpause(); + // Call when the filter is destroyed for whatever reason. + void abort(); + +private: + Event::Dispatcher& dispatcher() { return *dispatcher_; } + const FileServerConfig* file_server_config_; + void startFile(); + void startDir(int behavior_index); + void onFileOpened(AsyncFileHandle handle); + void readBodyChunk(); + Event::Dispatcher* dispatcher_; + FileStreamerClient& client_; + std::filesystem::path file_path_; + uint64_t pos_; + // If zero, fetches entire file. + // To get the last byte, end_ must be the size of the file, not the inclusive last byte + // like a range request uses. + uint64_t end_ = 0; + bool paused_ = false; + bool action_has_been_postponed_by_pause_ = false; + AsyncFileHandle async_file_; + CancelFunction cancel_callback_ = []() {}; +}; + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/file_server/filter.cc b/source/extensions/filters/http/file_server/filter.cc new file mode 100644 index 0000000000000..402ee8f143232 --- /dev/null +++ b/source/extensions/filters/http/file_server/filter.cc @@ -0,0 +1,141 @@ +#include "source/extensions/filters/http/file_server/filter.h" + +#include +#include + +#include "envoy/buffer/buffer.h" + +#include "source/common/http/codes.h" +#include "source/common/http/headers.h" +#include "source/common/http/utility.h" +#include "source/extensions/filters/http/file_server/filter_config.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +using ::Envoy::Http::CodeUtility; +using Http::RequestHeaderMap; +using Http::Utility::PercentEncoding; + +namespace { +// Returns 0, 0 if range headers are not present or invalid. +std::pair parseRangeHeader(const Http::RequestHeaderMap& headers) { + const Envoy::Http::HeaderMap::GetResult range_header = + headers.get(Envoy::Http::Headers::get().Range); + if (range_header.size() != 1) { + return {0, 0}; + } + absl::string_view range_str = range_header[0]->value().getStringView(); + if (!absl::ConsumePrefix(&range_str, "bytes=")) { + return {0, 0}; + } + if (absl::StrContains(range_str, ',')) { + // Not handling multiple-range requests. + return {0, 0}; + } + std::pair split = absl::StrSplit(range_str, '-'); + if (split.first.empty()) { + // Not handling suffix requests. + return {0, 0}; + } + uint64_t start = 0, end = 0; + if (!absl::SimpleAtoi(split.first, &start)) { + return {0, 0}; + } + if (!absl::SimpleAtoi(split.second, &end)) { + return {start, 0}; + } + // Add one because range headers use [start, end] and programmers use [start, end) + return {start, end + 1}; +} +} // namespace + +const std::string& FileServerFilter::filterName() { + CONSTRUCT_ON_FIRST_USE(std::string, "envoy.filters.http.file_server"); +} + +void FileServerFilter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) { + callbacks.addDownstreamWatermarkCallbacks(*this); + Http::PassThroughDecoderFilter::setDecoderFilterCallbacks(callbacks); +} + +void FileServerFilter::onAboveWriteBufferHighWatermark() { file_streamer_.pause(); } + +void FileServerFilter::onBelowWriteBufferLowWatermark() { file_streamer_.unpause(); } + +Http::FilterHeadersStatus FileServerFilter::decodeHeaders(RequestHeaderMap& headers, + bool end_stream ABSL_ATTRIBUTE_UNUSED) { + std::cerr << "XXXXX decodeHeaders" << std::endl; + if (!decoder_callbacks_->route() || !headers.Path()) { + return Http::FilterHeadersStatus::Continue; + } + const FileServerConfig* config = + Http::Utility::resolveMostSpecificPerFilterConfig(decoder_callbacks_); + if (!config) { + config = file_server_config_.get(); + } + std::string path = PercentEncoding::decode(headers.Path()->value().getStringView()); + std::shared_ptr mapping = config->pathMapping(path); + if (!mapping) { + std::cerr << "XXXXX !mapping" << std::endl; + // If the request didn't match a mapping, skip this filter. + return Http::FilterHeadersStatus::Continue; + } + std::cerr << "XXXXX matched a mapping" << std::endl; + absl::optional file_path = config->applyPathMapping(path, *mapping); + if (!file_path) { + decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, + CodeUtility::toString(Http::Code::BadRequest), nullptr, + absl::nullopt, "file_server_rejected_non_normalized_path"); + return Http::FilterHeadersStatus::StopIteration; + } + if (!headers.Method()) { + decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, + CodeUtility::toString(Http::Code::BadRequest), nullptr, + absl::nullopt, "file_server_rejected_missing_method"); + return Http::FilterHeadersStatus::StopIteration; + } + if (headers.Method()->value() != Http::Headers::get().MethodValues.Head && + headers.Method()->value() != Http::Headers::get().MethodValues.Get) { + decoder_callbacks_->sendLocalReply(Http::Code::MethodNotAllowed, + CodeUtility::toString(Http::Code::MethodNotAllowed), nullptr, + absl::nullopt, "file_server_rejected_method"); + return Http::FilterHeadersStatus::StopIteration; + } + // Parse range header, if present, into start and end (otherwise or on error, 0,0) + auto [start, end] = parseRangeHeader(headers); + is_head_ = headers.Method()->value() == Http::Headers::get().MethodValues.Head; + file_streamer_.begin(*config, decoder_callbacks_->dispatcher(), start, end, + std::move(*file_path)); + return Http::FilterHeadersStatus::StopIteration; +} + +bool FileServerFilter::headersFromFile(Http::ResponseHeaderMapPtr response_headers) { + bool end_response = is_head_ || response_headers->getContentLengthValue() == "0"; + decoder_callbacks_->encodeHeaders(std::move(response_headers), end_response, "file_server"); + headers_sent_ = true; + return !end_response; +} + +void FileServerFilter::bodyChunkFromFile(Buffer::InstancePtr buffer, bool end_stream) { + decoder_callbacks_->encodeData(*buffer, end_stream); +} + +void FileServerFilter::errorFromFile(Http::Code code, absl::string_view log_message) { + if (!headers_sent_) { + decoder_callbacks_->sendLocalReply(code, CodeUtility::toString(code), nullptr, absl::nullopt, + log_message); + } else { + decoder_callbacks_->streamInfo().setResponseCodeDetails(log_message); + decoder_callbacks_->resetStream(Http::StreamResetReason::LocalReset, log_message); + } +} + +void FileServerFilter::onDestroy() { file_streamer_.abort(); } + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/file_server/filter.h b/source/extensions/filters/http/file_server/filter.h new file mode 100644 index 0000000000000..4e057dbaed63a --- /dev/null +++ b/source/extensions/filters/http/file_server/filter.h @@ -0,0 +1,58 @@ +#pragma once + +#include +#include +#include + +#include "envoy/buffer/buffer.h" +#include "envoy/http/filter.h" + +#include "source/extensions/filters/http/common/pass_through_filter.h" +#include "source/extensions/filters/http/file_server/file_streamer.h" +#include "source/extensions/filters/http/file_server/filter_config.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +class FileServerFilter : public Http::PassThroughDecoderFilter, + public FileStreamerClient, + public Http::DownstreamWatermarkCallbacks { +public: + static std::shared_ptr + createShared(std::shared_ptr file_server_config) { + return std::shared_ptr(new FileServerFilter(std::move(file_server_config))); + } + + static const std::string& filterName(); + + void errorFromFile(Http::Code code, absl::string_view log_message) override; + bool headersFromFile(Http::ResponseHeaderMapPtr response_headers) override; + void bodyChunkFromFile(Buffer::InstancePtr buf, bool end_stream) override; + + void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) override; + Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers, + bool end_stream) override; + void onDestroy() override; + // Http::DownstreamWatermarkCallbacks + void onAboveWriteBufferHighWatermark() override; + void onBelowWriteBufferLowWatermark() override; + +private: + explicit FileServerFilter(std::shared_ptr file_server_config) + : file_server_config_(file_server_config), file_streamer_(*this) {} + std::shared_ptr file_server_config_; + friend class FileServerConfigTest; // Allow test access to file_server_config_. + FileStreamer file_streamer_; + bool is_head_; + bool headers_sent_ = false; + CancelFunction cancel_callback_ = []() {}; +}; + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/file_server/filter_config.cc b/source/extensions/filters/http/file_server/filter_config.cc new file mode 100644 index 0000000000000..0a2ee5d6e4d5c --- /dev/null +++ b/source/extensions/filters/http/file_server/filter_config.cc @@ -0,0 +1,116 @@ +#include "source/extensions/filters/http/file_server/filter_config.h" + +#include + +#include "envoy/common/exception.h" + +#include "source/common/common/thread.h" +#include "source/extensions/common/async_files/async_file_manager_factory.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +using ::Envoy::Extensions::Common::AsyncFiles::AsyncFileManagerFactory; + +namespace { + +RadixTree> +makePathMappings(const ProtoFileServerConfig& config) { + RadixTree> tree; + for (const auto& mapping : config.path_mappings()) { + tree.add(mapping.path_prefix(), std::make_shared(mapping)); + } + return tree; +} + +} // namespace + +absl::StatusOr> +FileServerConfig::create(const ProtoFileServerConfig& config, + Envoy::Server::Configuration::ServerFactoryContext& context) { + auto factory = AsyncFileManagerFactory::singleton(&context.singletonManager()); + TRY_ASSERT_MAIN_THREAD { + // TODO(ravenblack): make getAsyncFileManager use StatusOr instead of throw. + auto async_file_manager = factory->getAsyncFileManager(config.manager_config()); + return std::make_shared(config, std::move(async_file_manager)); + } + END_TRY + catch (const EnvoyException& e) { + ENVOY_LOG_MISC(warn, "invalid AsyncFileManagerConfig"); + } + return std::make_shared(config, nullptr); +} + +FileServerConfig::FileServerConfig(const ProtoFileServerConfig& config, + std::shared_ptr manager) + : async_file_manager_(std::move(manager)), path_mappings_(makePathMappings(config)), + content_types_(config.content_types().begin(), config.content_types().end()), + default_content_type_(config.default_content_type()), + directory_behaviors_(config.directory_behaviors().begin(), + config.directory_behaviors().end()) {} + +std::shared_ptr +FileServerConfig::pathMapping(absl::string_view path) const { + return path_mappings_.findLongestPrefix(path); +} + +absl::optional +FileServerConfig::applyPathMapping(absl::string_view path_with_query, + const ProtoFileServerConfig::PathMapping& mapping) { + std::pair split = absl::StrSplit(path_with_query, '?'); + absl::string_view kept_path = split.first.substr(mapping.path_prefix().length()); + if (kept_path.starts_with('/')) { + if (mapping.path_prefix().ends_with('/')) { + // Avoid accepting a value that parses away a double-slash at the join-point. + // (Other double-slashes will be rejected by the lexically_normal check.) + return absl::nullopt; + } + // filesystem::path operator / treats the second operand starting with a / as + // meaning replace the entire path, and we don't want to do that. + kept_path.remove_prefix(1); + } + if (kept_path.starts_with('/')) { + // We don't want to remove more than one slash, to avoid foo/bar, foo//bar + // and foo///bar all acting valid. + return absl::nullopt; + } + std::filesystem::path file_path = + std::filesystem::path{mapping.filesystem_prefix()} / std::filesystem::path{kept_path}; + if (file_path != file_path.lexically_normal() || + !file_path.string().starts_with(mapping.filesystem_prefix())) { + // Ensure we're not accidentally looking outside the designated filesystem prefix. + return absl::nullopt; + } + return file_path; +} + +absl::string_view FileServerConfig::contentTypeForPath(const std::filesystem::path& path) const { + std::string suffix = path.extension(); + if (suffix.empty()) { + // For files with no suffix, use the whole filename. + suffix = path.stem(); + } else { + // Remove the dot. + suffix = suffix.substr(1); + } + auto it = content_types_.find(suffix); + if (it == content_types_.end()) { + return default_content_type_; + } + return it->second; +} + +OptRef +FileServerConfig::directoryBehavior(size_t index) const { + if (index >= directory_behaviors_.size()) { + return absl::nullopt; + } + return directory_behaviors_[index]; +} + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/file_server/filter_config.h b/source/extensions/filters/http/file_server/filter_config.h new file mode 100644 index 0000000000000..7d7c392ebb384 --- /dev/null +++ b/source/extensions/filters/http/file_server/filter_config.h @@ -0,0 +1,54 @@ +#pragma once + +#include +#include + +#include "envoy/extensions/filters/http/file_server/v3/file_server.pb.h" +#include "envoy/server/factory_context.h" + +#include "source/common/common/radix_tree.h" +#include "source/extensions/common/async_files/async_file_manager.h" + +#include "absl/container/flat_hash_map.h" +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +using ProtoFileServerConfig = envoy::extensions::filters::http::file_server::v3::FileServerConfig; +using ::Envoy::Extensions::Common::AsyncFiles::AsyncFileManager; + +class FileServerConfig : public Router::RouteSpecificFilterConfig { +public: + static absl::StatusOr> + create(const ProtoFileServerConfig& config, + Envoy::Server::Configuration::ServerFactoryContext& context); + FileServerConfig(const ProtoFileServerConfig& config, std::shared_ptr manager); + + const std::shared_ptr& asyncFileManager() const { return async_file_manager_; } + // Returns nullptr if there is no corresponding path mapping (filter should be bypassed). + std::shared_ptr + pathMapping(absl::string_view path) const; + // Returns nullopt if the resulting path is not lexically normalized, + // e.g. foo/./bar rather than foo/bar, or foo/../bar rather than bar. + static absl::optional + applyPathMapping(absl::string_view path, const ProtoFileServerConfig::PathMapping& mapping); + + absl::string_view contentTypeForPath(const std::filesystem::path& path) const; + // nullopt if out of behaviors. + OptRef directoryBehavior(size_t index) const; + +private: + const std::shared_ptr async_file_manager_; + const RadixTree> path_mappings_; + const absl::flat_hash_map content_types_; + const std::string default_content_type_; + const std::vector directory_behaviors_; +}; + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/file_server/BUILD b/test/extensions/filters/http/file_server/BUILD new file mode 100644 index 0000000000000..b87a3faa8b00e --- /dev/null +++ b/test/extensions/filters/http/file_server/BUILD @@ -0,0 +1,46 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_package", +) +load( + "//test/extensions:extensions_build_system.bzl", + "envoy_extension_cc_test", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_extension_cc_test( + name = "file_server_test", + srcs = [ + "config_test.cc", + "filter_test.cc", + ], + extension_names = ["envoy.filters.http.file_server"], + rbe_pool = "6gig", + tags = ["skip_on_windows"], + deps = [ + "//source/extensions/filters/http/file_server:config", + "//test/extensions/common/async_files:mocks", + "//test/mocks/server:server_mocks", + "//test/mocks/upstream:upstream_mocks", + "//test/test_common:status_utility_lib", + ], +) + +envoy_extension_cc_test( + name = "file_server_integration_test", + srcs = [ + "integration_test.cc", + ], + extension_names = ["envoy.filters.http.file_server"], + rbe_pool = "6gig", + tags = ["skip_on_windows"], + deps = [ + "//source/extensions/filters/http/file_server:config", + "//test/integration:http_protocol_integration_lib", + "@com_google_absl//absl/strings", + "@com_google_absl//absl/strings:string_view", + ], +) diff --git a/test/extensions/filters/http/file_server/config_test.cc b/test/extensions/filters/http/file_server/config_test.cc new file mode 100644 index 0000000000000..8010e9f91587f --- /dev/null +++ b/test/extensions/filters/http/file_server/config_test.cc @@ -0,0 +1,212 @@ +#include "source/extensions/filters/http/file_server/config.h" +#include "source/extensions/filters/http/file_server/filter.h" + +#include "test/mocks/server/factory_context.h" +#include "test/mocks/server/server_factory_context.h" +#include "test/test_common/status_utility.h" +#include "test/test_common/utility.h" + +#include "absl/status/status.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +using StatusHelpers::HasStatus; +using ::testing::Eq; +using ::testing::HasSubstr; +using ::testing::IsNull; +using ::testing::NotNull; +using ::testing::Optional; +using ::testing::Property; + +MATCHER_P(OptRefWith, m, "") { + if (arg == absl::nullopt) { + *result_listener << "is nullopt"; + return false; + } + return ExplainMatchResult(m, arg.ref(), result_listener); +}; + +class FileServerConfigTest : public testing::Test { +public: + static ProtoFileServerConfig configFromYaml(absl::string_view yaml) { + std::string s(yaml); + ProtoFileServerConfig config; + TestUtility::loadFromYaml(s, config); + return config; + } + + static auto factory() { + return Registry::FactoryRegistry:: + getFactory(FileServerFilter::filterName()); + } + + static ProtoFileServerConfig emptyConfig() { + return *dynamic_cast(factory()->createEmptyConfigProto().get()); + } + + static std::function)> + captureConfig(std::shared_ptr* config) { + return [config](std::shared_ptr captured) { + *config = std::dynamic_pointer_cast(captured)->file_server_config_; + }; + } + + std::shared_ptr + captureConfigFromProto(const ProtoFileServerConfig& proto_config) { + Http::FilterFactoryCb cb = + factory() + ->createFilterFactoryFromProto(proto_config, "stats", mock_factory_context_) + .value(); + Http::MockFilterChainFactoryCallbacks filter_callback; + std::shared_ptr config; + EXPECT_CALL(filter_callback, addStreamDecoderFilter(_)) + .WillOnce(Invoke(captureConfig(&config))); + cb(filter_callback); + return config; + } + + std::shared_ptr + makeRouteConfig(const ProtoFileServerConfig& route_proto_config) { + return std::dynamic_pointer_cast( + factory() + ->createRouteSpecificFilterConfig(route_proto_config, mock_server_factory_context_, + ProtobufMessage::getNullValidationVisitor()) + .value()); + } + NiceMock mock_factory_context_; + NiceMock mock_server_factory_context_; +}; + +TEST_F(FileServerConfigTest, DuplicateDirectoryFilesIsConfigError) { + auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +directory_behaviors: + - try_file: "index.html" + - try_file: "index.html" +)"), + "stats", mock_factory_context_); + EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("index.html"))); +} + +TEST_F(FileServerConfigTest, DuplicateDirectoryListIsConfigError) { + auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +directory_behaviors: + - directory_list: {} + - directory_list: {} +)"), + "stats", mock_factory_context_); + EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, + HasSubstr("multiple directory_list directives"))); +} + +TEST_F(FileServerConfigTest, DuplicatePathPrefixIsConfigError) { + auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +path_mappings: + - path_prefix: "/banana" + filesystem_prefix: "/banana" + - path_prefix: "/banana" + filesystem_prefix: "/other" +)"), + "stats", mock_factory_context_); + EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("banana"))); +} + +TEST_F(FileServerConfigTest, SuffixForContentTypeContainingPeriodIsError) { + auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +content_types: + "txt": "text/plain" + ".html": "text/html" +)"), + "stats", mock_factory_context_); + EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr(".html"))); +} + +TEST_F(FileServerConfigTest, EmptyConfigIsOk) { + std::shared_ptr config = captureConfigFromProto(emptyConfig()); + EXPECT_THAT(config->contentTypeForPath("xxx.txt"), Eq("")); + EXPECT_THAT(config->asyncFileManager(), IsNull()); + EXPECT_THAT(config->directoryBehavior(0), Eq(absl::nullopt)); +} + +TEST_F(FileServerConfigTest, ValidConfigPopulatesConfigObjectAppropriately) { + std::shared_ptr config = captureConfigFromProto(configFromYaml(R"( +manager_config: + thread_pool: + thread_count: 1 +path_mappings: + - path_prefix: /path1/ + filesystem_prefix: /fs1 + - path_prefix: /path1/path2 + filesystem_prefix: fs2 +content_types: + "txt": "text/plain" + "html": "text/html" + "": "text/x-no-suffix" + "README": "text/markdown" +default_content_type: "application/octet-stream" +directory_behaviors: + - try_file: "index.html" + - try_file: "index.txt" + - directory_list: {} +)")); + EXPECT_THAT(config->pathMapping("/"), IsNull()); + EXPECT_THAT(config->pathMapping("/path1"), IsNull()); + EXPECT_THAT(config->pathMapping("/path1/"), NotNull()); + auto mapping = config->pathMapping("/path1/banana"); + ASSERT_THAT(mapping, NotNull()); + EXPECT_THAT(config->applyPathMapping("/path1/banana", *mapping), + Optional(std::filesystem::path{"/fs1/banana"})); + EXPECT_THAT(config->applyPathMapping("/path1//banana", *mapping), Eq(absl::nullopt)); + EXPECT_THAT(config->applyPathMapping("/path1/./banana", *mapping), Eq(absl::nullopt)); + EXPECT_THAT(config->applyPathMapping("/path1/../banana", *mapping), Eq(absl::nullopt)); + mapping = config->pathMapping("/path1/path2/banana"); + ASSERT_THAT(mapping, NotNull()); + EXPECT_THAT(config->applyPathMapping("/path1/path2/banana", *mapping), + Optional(std::filesystem::path{"fs2/banana"})); + EXPECT_THAT(config->applyPathMapping("/path1/path2//banana", *mapping), Eq(absl::nullopt)); + EXPECT_THAT(config->contentTypeForPath("/fs1/index.html"), Eq("text/html")); + // Multiple dots in the filename uses the last one as suffix. + EXPECT_THAT(config->contentTypeForPath("/fs1/index.banana.html"), Eq("text/html")); + EXPECT_THAT(config->contentTypeForPath("/fs1/index.txt"), Eq("text/plain")); + EXPECT_THAT(config->contentTypeForPath("/fs2/README"), Eq("text/markdown")); + EXPECT_THAT(config->contentTypeForPath("/fs2/README."), Eq("text/x-no-suffix")); + EXPECT_THAT(config->contentTypeForPath("/fs1/other"), Eq("application/octet-stream")); + EXPECT_THAT(config->asyncFileManager(), NotNull()); + EXPECT_THAT(config->directoryBehavior(0), + OptRefWith(Property("try_file", &ProtoFileServerConfig::DirectoryBehavior::try_file, + Eq("index.html")))); + EXPECT_THAT(config->directoryBehavior(1), + OptRefWith(Property("try_file", &ProtoFileServerConfig::DirectoryBehavior::try_file, + Eq("index.txt")))); + EXPECT_THAT(config->directoryBehavior(2), + OptRefWith(Property("has_directory_list", + &ProtoFileServerConfig::DirectoryBehavior::has_directory_list, + Eq(true)))); + EXPECT_THAT(config->directoryBehavior(3), Eq(absl::nullopt)); +} + +TEST_F(FileServerConfigTest, DuplicateDirectoryFilesIsConfigErrorInRouteConfig) { + auto status_or = factory()->createRouteSpecificFilterConfig( + configFromYaml(R"( +directory_behaviors: + - try_file: "index.html" + - try_file: "index.html" +)"), + mock_server_factory_context_, ProtobufMessage::getNullValidationVisitor()); + EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("index.html"))); +} + +TEST_F(FileServerConfigTest, EmptyConfigIsOkInRouteConfig) { + std::shared_ptr config = makeRouteConfig(emptyConfig()); + EXPECT_THAT(config->contentTypeForPath("xxx.txt"), Eq("")); + EXPECT_THAT(config->asyncFileManager(), IsNull()); + EXPECT_THAT(config->directoryBehavior(0), Eq(absl::nullopt)); +} + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/file_server/filter_test.cc b/test/extensions/filters/http/file_server/filter_test.cc new file mode 100644 index 0000000000000..aef7bf552426a --- /dev/null +++ b/test/extensions/filters/http/file_server/filter_test.cc @@ -0,0 +1,459 @@ +#include "source/extensions/filters/http/file_server/config.h" +#include "source/extensions/filters/http/file_server/filter.h" + +#include "test/extensions/common/async_files/mocks.h" +#include "test/mocks/http/mocks.h" +#include "test/test_common/status_utility.h" +#include "test/test_common/utility.h" + +#include "absl/status/status.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +using Extensions::Common::AsyncFiles::MockAsyncFileContext; +using Extensions::Common::AsyncFiles::MockAsyncFileHandle; +using Extensions::Common::AsyncFiles::MockAsyncFileManager; +using StatusHelpers::HasStatus; +using ::testing::AnyNumber; +using ::testing::InSequence; +using ::testing::NiceMock; +using ::testing::NotNull; +using ::testing::ReturnRef; +using ::testing::StrictMock; + +MATCHER_P(OptRefWith, m, "") { + if (arg == absl::nullopt) { + *result_listener << "is nullopt"; + return false; + } + return ExplainMatchResult(m, arg.ref(), result_listener); +}; + +class FileServerFilterTest : public testing::Test { +public: + std::shared_ptr configFromYaml(absl::string_view yaml) { + std::string s(yaml); + ProtoFileServerConfig proto_config; + TestUtility::loadFromYaml(s, proto_config); + return std::make_shared(proto_config, mock_async_file_manager_); + } + std::shared_ptr testFilter() { + auto filter = FileServerFilter::createShared(configFromYaml(R"( +path_mappings: + - path_prefix: /path1 + filesystem_prefix: fs1 +content_types: + "txt": "text/plain" + "html": "text/html" +default_content_type: "application/octet-stream" +directory_behaviors: + - try_file: "index.html" + - try_file: "index.txt" + - directory_list: {} +)")); + filter->setDecoderFilterCallbacks(decoder_callbacks_); + // It's a NiceMock but we do want to be notified of unexpected sendLocalReply. + EXPECT_CALL(decoder_callbacks_, sendLocalReply).Times(0); + EXPECT_CALL(decoder_callbacks_, dispatcher) + .Times(AnyNumber()) + .WillRepeatedly(ReturnRef(*dispatcher_)); + return filter; + } + + void pumpDispatcher() { dispatcher_->run(Event::Dispatcher::RunType::Block); } + + AsyncFileHandle makeMockFile() { + mock_file_handle_ = + std::make_shared>(mock_async_file_manager_); + return mock_file_handle_; + } + + std::string responseCodeDetails() { + return decoder_callbacks_.stream_info_.response_code_details_.value_or(""); + } + + std::shared_ptr mock_async_file_manager_ = + std::make_shared>(); + MockAsyncFileHandle mock_file_handle_; + NiceMock decoder_callbacks_; + Api::ApiPtr api_ = Api::createApiForTest(); + Event::DispatcherPtr dispatcher_ = api_->allocateDispatcher("test_thread"); +}; + +TEST_F(FileServerFilterTest, PassThroughIfNoPath) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":host", "test.host"}, + {":method", "GET"}, + {":scheme", "https"}, + }; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter->decodeHeaders(request_headers, true)); +} + +TEST_F(FileServerFilterTest, PassThroughIfNotMatchingPathMapping) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/"}, + {":host", "test.host"}, + {":method", "GET"}, + {":scheme", "https"}, + }; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter->decodeHeaders(request_headers, true)); +} + +TEST_F(FileServerFilterTest, BadRequestIfNonNormalizedPath) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/../"}, + {":host", "test.host"}, + {":method", "GET"}, + {":scheme", "https"}, + }; + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::BadRequest, _, _, _, + "file_server_rejected_non_normalized_path")); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); +} + +TEST_F(FileServerFilterTest, BadRequestIfMissingMethod) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::BadRequest, _, _, _, + "file_server_rejected_missing_method")); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); +} + +TEST_F(FileServerFilterTest, StillMatchesPathIfPercentEncodingUsed) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + // %31 is encoding of '1' + {":path", "/path%31/foo"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + // Missing method is only checked if the path matched, so this is a quick test + // for "it matched the path and then failed later". + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::BadRequest, _, _, _, + "file_server_rejected_missing_method")); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); +} + +TEST_F(FileServerFilterTest, MethodNotAllowedIfMatchedPathAndUnsupportedMethod) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo"}, + {":method", "POST"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + EXPECT_CALL(decoder_callbacks_, + sendLocalReply(Http::Code::MethodNotAllowed, _, _, _, "file_server_rejected_method")); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); +} + +TEST_F(FileServerFilterTest, FileStatFailedNotFoundRespondsNotFound) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + EXPECT_CALL(decoder_callbacks_, + sendLocalReply(Http::Code::NotFound, _, _, _, "file_server_stat_error")); + mock_async_file_manager_->nextActionCompletes( + absl::StatusOr{absl::NotFoundError("mocked not found")}); + pumpDispatcher(); +} + +TEST_F(FileServerFilterTest, FilterOnDestroyWhileFileActionInFlightAbortsResponse) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + mock_async_file_manager_->nextActionCompletes( + absl::StatusOr{absl::NotFoundError("mocked not found")}); + filter->onDestroy(); + pumpDispatcher(); + // Should have been no call to sendLocalReply due to abort. +} + +TEST_F(FileServerFilterTest, TriesAllDirectoryBehaviorsInOrder) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + { + InSequence seq; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_CALL(*mock_async_file_manager_, openExistingFile(_, "fs1/foo/index.html", _, _)); + EXPECT_CALL(*mock_async_file_manager_, openExistingFile(_, "fs1/foo/index.txt", _, _)); + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Forbidden, _, _, _, + "file_server_directory_list_not_implemented")); + } + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + struct stat stat_result = {}; + stat_result.st_mode = S_IFDIR; + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes( + absl::StatusOr{absl::NotFoundError("mocked not found index.html")}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes( + absl::StatusOr{absl::NotFoundError("mocked not found index.txt")}); + pumpDispatcher(); +} + +TEST_F(FileServerFilterTest, ErrorOpeningExistingFileGivesErrorResponse) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo/index.html"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + { + InSequence seq; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_CALL(*mock_async_file_manager_, openExistingFile(_, "fs1/foo/index.html", _, _)); + EXPECT_CALL(decoder_callbacks_, + sendLocalReply(Http::Code::Forbidden, _, _, _, "file_server_open_error")); + } + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + struct stat stat_result = {}; + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{ + absl::PermissionDeniedError("mocked permission denied index.html")}); + pumpDispatcher(); +} + +TEST_F(FileServerFilterTest, OpeningIndexFileStartsFileAndStatErrorGivesErrorResponse) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + makeMockFile(); + { + InSequence seq; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_CALL(*mock_async_file_manager_, openExistingFile(_, "fs1/foo/index.html", _, _)); + EXPECT_CALL(*mock_file_handle_, stat); + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::InternalServerError, _, _, _, + "file_server_opened_file_stat_failed")); + } + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + struct stat stat_result = {}; + stat_result.st_mode = S_IFDIR; + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{mock_file_handle_}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes( + absl::StatusOr{absl::InternalError("mocked stat-for-size fail")}); + pumpDispatcher(); +} + +TEST_F(FileServerFilterTest, HeadRequestJustStatsFileAndResponds) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo/index.html"}, + {":method", "HEAD"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + makeMockFile(); + Http::TestResponseHeaderMapImpl expected_headers{ + {":status", "200"}, + {"accept-ranges", "bytes"}, + {"content-length", "12345"}, + {"content-type", "text/html"}, + }; + { + InSequence seq; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_CALL(*mock_async_file_manager_, openExistingFile(_, "fs1/foo/index.html", _, _)); + EXPECT_CALL(*mock_file_handle_, stat); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_headers), true)); + } + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + struct stat stat_result = {}; + stat_result.st_size = 12345; + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{mock_file_handle_}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); +} + +TEST_F(FileServerFilterTest, GetRequestResetsStreamOnReadError) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo/index.html"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + makeMockFile(); + Http::TestResponseHeaderMapImpl expected_headers{ + {":status", "200"}, + {"accept-ranges", "bytes"}, + {"content-length", "12345"}, + {"content-type", "text/html"}, + }; + { + InSequence seq; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_CALL(*mock_async_file_manager_, openExistingFile(_, "fs1/foo/index.html", _, _)); + EXPECT_CALL(*mock_file_handle_, stat); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_headers), false)); + EXPECT_CALL(*mock_file_handle_, read(_, 0, 12345, _)); + EXPECT_CALL(decoder_callbacks_, resetStream); + } + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + struct stat stat_result = {}; + stat_result.st_size = 12345; + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{mock_file_handle_}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes( + absl::StatusOr{absl::InternalError("mocked read error")}); + pumpDispatcher(); + EXPECT_EQ(responseCodeDetails(), "file_server_read_operation_failed"); +} + +TEST_F(FileServerFilterTest, GetRequestPausesWhenOverBufferLimit) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo/index.html"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + makeMockFile(); + Http::TestResponseHeaderMapImpl expected_headers{ + {":status", "200"}, + {"accept-ranges", "bytes"}, + {"content-length", "42000"}, + {"content-type", "text/html"}, + }; + // chunk1 is the max read size. + std::string chunk1(32 * 1024, 'A'); + // chunk2 is the remainder. + std::string chunk2(42000 - chunk1.length(), 'B'); + { + InSequence seq; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_CALL(*mock_async_file_manager_, openExistingFile(_, "fs1/foo/index.html", _, _)); + EXPECT_CALL(*mock_file_handle_, stat); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_headers), false)); + EXPECT_CALL(*mock_file_handle_, read(_, 0, chunk1.length(), _)); + EXPECT_CALL(decoder_callbacks_, encodeData(BufferStringEqual(chunk1), false)); + EXPECT_CALL(*mock_file_handle_, read(_, chunk1.length(), chunk2.length(), _)); + EXPECT_CALL(decoder_callbacks_, encodeData(BufferStringEqual(chunk2), true)); + } + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + struct stat stat_result = {}; + stat_result.st_size = chunk1.length() + chunk2.length(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{mock_file_handle_}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes( + absl::StatusOr{std::make_unique(chunk1)}); + filter->onAboveWriteBufferHighWatermark(); + pumpDispatcher(); + ASSERT_TRUE(mock_async_file_manager_->queue_.empty()) + << "next action should not have been queued due to high watermark"; + filter->onBelowWriteBufferLowWatermark(); + ASSERT_FALSE(mock_async_file_manager_->queue_.empty()) + << "next action should have been queued due to low watermark"; + mock_async_file_manager_->nextActionCompletes( + absl::StatusOr{std::make_unique(chunk2)}); + pumpDispatcher(); + EXPECT_EQ(responseCodeDetails(), "file_server"); +} + +TEST_F(FileServerFilterTest, BufferLimitsDontPauseIfClearedBeforeActionCompletes) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo/index.html"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + makeMockFile(); + Http::TestResponseHeaderMapImpl expected_headers{ + {":status", "200"}, + {"accept-ranges", "bytes"}, + {"content-length", "42000"}, + {"content-type", "text/html"}, + }; + // chunk1 is the max read size. + std::string chunk1(32 * 1024, 'A'); + // chunk2 is the remainder. + std::string chunk2(42000 - chunk1.length(), 'B'); + { + InSequence seq; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_CALL(*mock_async_file_manager_, openExistingFile(_, "fs1/foo/index.html", _, _)); + EXPECT_CALL(*mock_file_handle_, stat); + EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&expected_headers), false)); + EXPECT_CALL(*mock_file_handle_, read(_, 0, chunk1.length(), _)); + EXPECT_CALL(decoder_callbacks_, encodeData(BufferStringEqual(chunk1), false)); + EXPECT_CALL(*mock_file_handle_, read(_, chunk1.length(), chunk2.length(), _)); + EXPECT_CALL(decoder_callbacks_, encodeData(BufferStringEqual(chunk2), true)); + } + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + struct stat stat_result = {}; + stat_result.st_size = chunk1.length() + chunk2.length(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{mock_file_handle_}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); + mock_async_file_manager_->nextActionCompletes( + absl::StatusOr{std::make_unique(chunk1)}); + filter->onAboveWriteBufferHighWatermark(); + filter->onBelowWriteBufferLowWatermark(); + pumpDispatcher(); + ASSERT_FALSE(mock_async_file_manager_->queue_.empty()) + << "next action should have been queued because watermark was cleared before previous action " + "completed"; + mock_async_file_manager_->nextActionCompletes( + absl::StatusOr{std::make_unique(chunk2)}); + pumpDispatcher(); + EXPECT_EQ(responseCodeDetails(), "file_server"); +} + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/file_server/integration_test.cc b/test/extensions/filters/http/file_server/integration_test.cc new file mode 100644 index 0000000000000..c00962b6bf062 --- /dev/null +++ b/test/extensions/filters/http/file_server/integration_test.cc @@ -0,0 +1,173 @@ +#include + +#include "test/integration/http_protocol_integration.h" +#include "test/test_common/utility.h" + +#include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +using ::testing::AllOf; + +class FileServerIntegrationTest : public HttpProtocolIntegrationTest { +public: + static constexpr absl::string_view index_txt_contents_ = "12345678"; + static constexpr absl::string_view banana_html_contents_ = "abcdefgh"; + static constexpr absl::string_view readme_md_contents_ = "README CONTENT"; + static constexpr absl::string_view index_html_contents_ = "87654321"; + static absl::string_view testTmpDir() { + auto env_tmpdir = std::getenv("TEST_TMPDIR"); + if (env_tmpdir) { + return env_tmpdir; + } + env_tmpdir = std::getenv("TMPDIR"); + return env_tmpdir ? env_tmpdir : "/tmp"; + } + + static void prepareTmpFiles() { + std::cerr << "Writing test filesystem in tmpdir: " << testTmpDir() << std::endl; + TestEnvironment::createPath(absl::StrCat(testTmpDir(), "/fs1")); + TestEnvironment::createPath(absl::StrCat(testTmpDir(), "/fs2")); + TestEnvironment::writeStringToFileForTest(absl::StrCat(testTmpDir(), "/fs1/banana.html"), + std::string{banana_html_contents_}, true); + TestEnvironment::writeStringToFileForTest(absl::StrCat(testTmpDir(), "/fs1/index.txt"), + std::string{index_txt_contents_}, true); + TestEnvironment::writeStringToFileForTest(absl::StrCat(testTmpDir(), "/fs1/README.md"), + std::string{readme_md_contents_}, true); + TestEnvironment::writeStringToFileForTest(absl::StrCat(testTmpDir(), "/fs2/index.html"), + std::string{index_html_contents_}, true); + } + + static void SetUpTestSuite() { prepareTmpFiles(); } + + std::string testConfig() { + return absl::StrCat(R"( +name: "envoy.filters.http.file_server" +typed_config: + "@type": "type.googleapis.com/envoy.extensions.filters.http.file_server.v3.FileServerConfig" + manager_config: + thread_pool: + thread_count: 1 + path_mappings: + - path_prefix: /path1 + filesystem_prefix: )", + testTmpDir(), R"(/fs1 + - path_prefix: /path1/path2 + filesystem_prefix: )", + testTmpDir(), R"(/fs2 + content_types: + "txt": "text/plain" + "html": "text/html" + default_content_type: "application/octet-stream" + directory_behaviors: + - try_file: "index.html" + - try_file: "index.txt" + - directory_list: {} +)"); + } + + void initializeFilter(const std::string& config) { + config_helper_.prependFilter(config); + initialize(); + codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); + } + + IntegrationStreamDecoderPtr + sendHeaderOnlyRequestAwaitResponse(const Http::TestRequestHeaderMapImpl& headers) { + IntegrationStreamDecoderPtr response_decoder = codec_client_->makeHeaderOnlyRequest(headers); + // Wait for the response to be read by the codec client. + EXPECT_TRUE(response_decoder->waitForEndStream()); + return response_decoder; + } + + Http::TestRequestHeaderMapImpl requestPath(std::string path) { + return Http::TestRequestHeaderMapImpl{ + {":method", "GET"}, + {":path", path}, + {":authority", "some_authority"}, + {":scheme", "http"}, + }; + } +}; + +// Nothing about this filter interacts with the http protocols in any way, so there's no need +// to run combinatorial iterations of each test, we can just run one. +INSTANTIATE_TEST_SUITE_P( + Protocols, FileServerIntegrationTest, + testing::ValuesIn({HttpProtocolIntegrationTest::getProtocolTestParams()[0]}), + HttpProtocolIntegrationTest::protocolTestParamsToString); + +TEST_P(FileServerIntegrationTest, ReadsConfiguredIndexFileOnRequestForDirectory) { + initializeFilter(testConfig()); + Http::TestRequestHeaderMapImpl request_headers = requestPath("/path1"); + auto response = sendHeaderOnlyRequestAwaitResponse(request_headers); + EXPECT_THAT(response->headers(), + AllOf(ContainsHeader(":status", "200"), ContainsHeader("content-type", "text/plain"), + ContainsHeader("content-length", absl::StrCat(index_txt_contents_.length())))); + EXPECT_THAT(response->body(), index_txt_contents_); +} + +TEST_P(FileServerIntegrationTest, ReadsSpecifiedFile) { + initializeFilter(testConfig()); + Http::TestRequestHeaderMapImpl request_headers = requestPath("/path1/banana.html"); + auto response = sendHeaderOnlyRequestAwaitResponse(request_headers); + EXPECT_THAT( + response->headers(), + AllOf(ContainsHeader(":status", "200"), ContainsHeader("content-type", "text/html"), + ContainsHeader("content-length", absl::StrCat(banana_html_contents_.length())))); + EXPECT_THAT(response->body(), banana_html_contents_); +} + +TEST_P(FileServerIntegrationTest, IgnoresInvalidlyFormattedRangeHeader) { + initializeFilter(testConfig()); + Http::TestRequestHeaderMapImpl request_headers = requestPath("/path1/banana.html"); + request_headers.addCopy("range", "megatrons=3-5"); + auto response = sendHeaderOnlyRequestAwaitResponse(request_headers); + EXPECT_THAT( + response->headers(), + AllOf(ContainsHeader(":status", "200"), ContainsHeader("content-type", "text/html"), + ContainsHeader("content-length", absl::StrCat(banana_html_contents_.length())))); + EXPECT_THAT(response->body(), banana_html_contents_); +} + +TEST_P(FileServerIntegrationTest, ReadsSpecifiedFileWithRange) { + initializeFilter(testConfig()); + Http::TestRequestHeaderMapImpl request_headers = requestPath("/path1/banana.html"); + request_headers.addCopy("range", "bytes=3-5"); + auto response = sendHeaderOnlyRequestAwaitResponse(request_headers); + EXPECT_THAT(response->headers(), + AllOf(ContainsHeader(":status", "206"), ContainsHeader("content-type", "text/html"), + ContainsHeader("content-length", "3"), + ContainsHeader("content-range", "bytes 3-5/8"))); + EXPECT_THAT(response->body(), banana_html_contents_.substr(3, 3)); +} + +TEST_P(FileServerIntegrationTest, ReadsSpecifiedFileWithRangeToEnd) { + initializeFilter(testConfig()); + Http::TestRequestHeaderMapImpl request_headers = requestPath("/path1/banana.html"); + request_headers.addCopy("range", "bytes=3-"); + auto response = sendHeaderOnlyRequestAwaitResponse(request_headers); + EXPECT_THAT(response->headers(), + AllOf(ContainsHeader(":status", "206"), ContainsHeader("content-type", "text/html"), + ContainsHeader("content-length", "5"), + ContainsHeader("content-range", "bytes 3-7/8"))); + EXPECT_THAT(response->body(), banana_html_contents_.substr(3)); +} + +TEST_P(FileServerIntegrationTest, RejectsRangeRequestLargerThanFile) { + initializeFilter(testConfig()); + Http::TestRequestHeaderMapImpl request_headers = requestPath("/path1/banana.html"); + request_headers.addCopy("range", "bytes=3-20"); + auto response = sendHeaderOnlyRequestAwaitResponse(request_headers); + EXPECT_THAT(response->headers(), + ContainsHeader(":status", absl::StrCat(Http::Code::RangeNotSatisfiable))); +} + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy From 36edcd29405b895b3b0b3bb64d72c2d0dd321e2b Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 2 Feb 2026 17:08:42 +0000 Subject: [PATCH 02/22] Improve coverage, fix docgen Signed-off-by: Raven Black --- api/BUILD | 1 + .../filters/http/file_server/file_streamer.h | 1 - .../filters/http/file_server/filter.cc | 3 - .../filters/http/file_server/filter.h | 1 - .../filters/http/file_server/filter_config.cc | 29 ++++---- .../filters/http/file_server/filter_config.h | 11 ++- .../filters/http/file_server/config_test.cc | 40 +++++++++++ .../filters/http/file_server/filter_test.cc | 71 +++++++++++++++++-- .../http/file_server/integration_test.cc | 36 ++++++++++ 9 files changed, 166 insertions(+), 27 deletions(-) diff --git a/api/BUILD b/api/BUILD index 09c3fe194b31c..1ba5235647c2d 100644 --- a/api/BUILD +++ b/api/BUILD @@ -198,6 +198,7 @@ proto_library( "//envoy/extensions/filters/http/ext_authz/v3:pkg", "//envoy/extensions/filters/http/ext_proc/v3:pkg", "//envoy/extensions/filters/http/fault/v3:pkg", + "//envoy/extensions/filters/http/file_server/v3:pkg", "//envoy/extensions/filters/http/file_system_buffer/v3:pkg", "//envoy/extensions/filters/http/gcp_authn/v3:pkg", "//envoy/extensions/filters/http/geoip/v3:pkg", diff --git a/source/extensions/filters/http/file_server/file_streamer.h b/source/extensions/filters/http/file_server/file_streamer.h index 5371f138a02d3..a1bbe74f6d2fe 100644 --- a/source/extensions/filters/http/file_server/file_streamer.h +++ b/source/extensions/filters/http/file_server/file_streamer.h @@ -44,7 +44,6 @@ class FileStreamer { void abort(); private: - Event::Dispatcher& dispatcher() { return *dispatcher_; } const FileServerConfig* file_server_config_; void startFile(); void startDir(int behavior_index); diff --git a/source/extensions/filters/http/file_server/filter.cc b/source/extensions/filters/http/file_server/filter.cc index 402ee8f143232..b99be15e559de 100644 --- a/source/extensions/filters/http/file_server/filter.cc +++ b/source/extensions/filters/http/file_server/filter.cc @@ -67,7 +67,6 @@ void FileServerFilter::onBelowWriteBufferLowWatermark() { file_streamer_.unpause Http::FilterHeadersStatus FileServerFilter::decodeHeaders(RequestHeaderMap& headers, bool end_stream ABSL_ATTRIBUTE_UNUSED) { - std::cerr << "XXXXX decodeHeaders" << std::endl; if (!decoder_callbacks_->route() || !headers.Path()) { return Http::FilterHeadersStatus::Continue; } @@ -79,11 +78,9 @@ Http::FilterHeadersStatus FileServerFilter::decodeHeaders(RequestHeaderMap& head std::string path = PercentEncoding::decode(headers.Path()->value().getStringView()); std::shared_ptr mapping = config->pathMapping(path); if (!mapping) { - std::cerr << "XXXXX !mapping" << std::endl; // If the request didn't match a mapping, skip this filter. return Http::FilterHeadersStatus::Continue; } - std::cerr << "XXXXX matched a mapping" << std::endl; absl::optional file_path = config->applyPathMapping(path, *mapping); if (!file_path) { decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, diff --git a/source/extensions/filters/http/file_server/filter.h b/source/extensions/filters/http/file_server/filter.h index 4e057dbaed63a..53db15d204717 100644 --- a/source/extensions/filters/http/file_server/filter.h +++ b/source/extensions/filters/http/file_server/filter.h @@ -49,7 +49,6 @@ class FileServerFilter : public Http::PassThroughDecoderFilter, FileStreamer file_streamer_; bool is_head_; bool headers_sent_ = false; - CancelFunction cancel_callback_ = []() {}; }; } // namespace FileServer diff --git a/source/extensions/filters/http/file_server/filter_config.cc b/source/extensions/filters/http/file_server/filter_config.cc index 0a2ee5d6e4d5c..d7d638f669670 100644 --- a/source/extensions/filters/http/file_server/filter_config.cc +++ b/source/extensions/filters/http/file_server/filter_config.cc @@ -5,15 +5,12 @@ #include "envoy/common/exception.h" #include "source/common/common/thread.h" -#include "source/extensions/common/async_files/async_file_manager_factory.h" namespace Envoy { namespace Extensions { namespace HttpFilters { namespace FileServer { -using ::Envoy::Extensions::Common::AsyncFiles::AsyncFileManagerFactory; - namespace { RadixTree> @@ -31,21 +28,27 @@ absl::StatusOr> FileServerConfig::create(const ProtoFileServerConfig& config, Envoy::Server::Configuration::ServerFactoryContext& context) { auto factory = AsyncFileManagerFactory::singleton(&context.singletonManager()); - TRY_ASSERT_MAIN_THREAD { - // TODO(ravenblack): make getAsyncFileManager use StatusOr instead of throw. - auto async_file_manager = factory->getAsyncFileManager(config.manager_config()); - return std::make_shared(config, std::move(async_file_manager)); - } - END_TRY - catch (const EnvoyException& e) { - ENVOY_LOG_MISC(warn, "invalid AsyncFileManagerConfig"); + if (config.manager_config().manager_type_case() != + envoy::extensions::common::async_files::v3::AsyncFileManagerConfig::MANAGER_TYPE_NOT_SET) { + TRY_ASSERT_MAIN_THREAD { + // TODO(ravenblack): make getAsyncFileManager use StatusOr instead of throw. + auto async_file_manager = factory->getAsyncFileManager(config.manager_config()); + return std::make_shared(config, std::move(factory), + std::move(async_file_manager)); + } + END_TRY + catch (const EnvoyException& e) { + return absl::InvalidArgumentError(e.what()); + } } - return std::make_shared(config, nullptr); + return std::make_shared(config, nullptr, nullptr); } FileServerConfig::FileServerConfig(const ProtoFileServerConfig& config, + std::shared_ptr factory, std::shared_ptr manager) - : async_file_manager_(std::move(manager)), path_mappings_(makePathMappings(config)), + : async_file_manager_factory_(std::move(factory)), async_file_manager_(std::move(manager)), + path_mappings_(makePathMappings(config)), content_types_(config.content_types().begin(), config.content_types().end()), default_content_type_(config.default_content_type()), directory_behaviors_(config.directory_behaviors().begin(), diff --git a/source/extensions/filters/http/file_server/filter_config.h b/source/extensions/filters/http/file_server/filter_config.h index 7d7c392ebb384..68c112a201a50 100644 --- a/source/extensions/filters/http/file_server/filter_config.h +++ b/source/extensions/filters/http/file_server/filter_config.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -8,6 +9,7 @@ #include "source/common/common/radix_tree.h" #include "source/extensions/common/async_files/async_file_manager.h" +#include "source/extensions/common/async_files/async_file_manager_factory.h" #include "absl/container/flat_hash_map.h" #include "absl/strings/string_view.h" @@ -19,13 +21,16 @@ namespace FileServer { using ProtoFileServerConfig = envoy::extensions::filters::http::file_server::v3::FileServerConfig; using ::Envoy::Extensions::Common::AsyncFiles::AsyncFileManager; +using ::Envoy::Extensions::Common::AsyncFiles::AsyncFileManagerFactory; class FileServerConfig : public Router::RouteSpecificFilterConfig { public: static absl::StatusOr> create(const ProtoFileServerConfig& config, Envoy::Server::Configuration::ServerFactoryContext& context); - FileServerConfig(const ProtoFileServerConfig& config, std::shared_ptr manager); + FileServerConfig(const ProtoFileServerConfig& config, + std::shared_ptr factory, + std::shared_ptr manager); const std::shared_ptr& asyncFileManager() const { return async_file_manager_; } // Returns nullptr if there is no corresponding path mapping (filter should be bypassed). @@ -41,11 +46,13 @@ class FileServerConfig : public Router::RouteSpecificFilterConfig { OptRef directoryBehavior(size_t index) const; private: + // The factory is held to keep the singleton alive. + const std::shared_ptr async_file_manager_factory_; const std::shared_ptr async_file_manager_; const RadixTree> path_mappings_; const absl::flat_hash_map content_types_; const std::string default_content_type_; - const std::vector directory_behaviors_; + const std::vector directory_behaviors_; }; } // namespace FileServer diff --git a/test/extensions/filters/http/file_server/config_test.cc b/test/extensions/filters/http/file_server/config_test.cc index 8010e9f91587f..92ded311f2e26 100644 --- a/test/extensions/filters/http/file_server/config_test.cc +++ b/test/extensions/filters/http/file_server/config_test.cc @@ -206,6 +206,46 @@ TEST_F(FileServerConfigTest, EmptyConfigIsOkInRouteConfig) { EXPECT_THAT(config->directoryBehavior(0), Eq(absl::nullopt)); } +TEST_F(FileServerConfigTest, InvalidFileManagerConfigFailsInRouteConfig) { + auto mismatched_config = factory()->createRouteSpecificFilterConfig( + configFromYaml(R"( +manager_config: + id: "mismatched" + thread_pool: + thread_count: 2 +)"), + mock_server_factory_context_, ProtobufMessage::getNullValidationVisitor()); + auto status_or = factory()->createRouteSpecificFilterConfig( + configFromYaml(R"( +manager_config: + id: "mismatched" + thread_pool: + thread_count: 1 +)"), + mock_server_factory_context_, ProtobufMessage::getNullValidationVisitor()); + EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, + HasSubstr("AsyncFileManager mismatched config"))); +} + +TEST_F(FileServerConfigTest, InvalidFileManagerConfigFailsInMainConfig) { + auto mismatched_config = factory()->createFilterFactoryFromProto(configFromYaml(R"( +manager_config: + id: "mismatched" + thread_pool: + thread_count: 2 +)"), + "stats", mock_factory_context_); + auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +manager_config: + id: "mismatched" + thread_pool: + thread_count: 1 +)"), + "stats", mock_factory_context_); + EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, + HasSubstr("AsyncFileManager mismatched config"))); +} + } // namespace FileServer } // namespace HttpFilters } // namespace Extensions diff --git a/test/extensions/filters/http/file_server/filter_test.cc b/test/extensions/filters/http/file_server/filter_test.cc index aef7bf552426a..0f244d23121a1 100644 --- a/test/extensions/filters/http/file_server/filter_test.cc +++ b/test/extensions/filters/http/file_server/filter_test.cc @@ -39,7 +39,15 @@ class FileServerFilterTest : public testing::Test { std::string s(yaml); ProtoFileServerConfig proto_config; TestUtility::loadFromYaml(s, proto_config); - return std::make_shared(proto_config, mock_async_file_manager_); + return std::make_shared(proto_config, nullptr, mock_async_file_manager_); + } + void initFilter(FileServerFilter& filter) { + filter.setDecoderFilterCallbacks(decoder_callbacks_); + // It's a NiceMock but we do want to be notified of unexpected sendLocalReply. + EXPECT_CALL(decoder_callbacks_, sendLocalReply).Times(0); + EXPECT_CALL(decoder_callbacks_, dispatcher) + .Times(AnyNumber()) + .WillRepeatedly(ReturnRef(*dispatcher_)); } std::shared_ptr testFilter() { auto filter = FileServerFilter::createShared(configFromYaml(R"( @@ -55,12 +63,7 @@ default_content_type: "application/octet-stream" - try_file: "index.txt" - directory_list: {} )")); - filter->setDecoderFilterCallbacks(decoder_callbacks_); - // It's a NiceMock but we do want to be notified of unexpected sendLocalReply. - EXPECT_CALL(decoder_callbacks_, sendLocalReply).Times(0); - EXPECT_CALL(decoder_callbacks_, dispatcher) - .Times(AnyNumber()) - .WillRepeatedly(ReturnRef(*dispatcher_)); + initFilter(*filter); return filter; } @@ -192,6 +195,60 @@ TEST_F(FileServerFilterTest, FilterOnDestroyWhileFileActionInFlightAbortsRespons // Should have been no call to sendLocalReply due to abort. } +TEST_F(FileServerFilterTest, ErrorsOnDirectoryWithNoConfiguredBehavior) { + auto filter = FileServerFilter::createShared(configFromYaml(R"( +path_mappings: + - path_prefix: /path1 + filesystem_prefix: fs1 +)")); + initFilter(*filter); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + { + InSequence seq; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Forbidden, _, _, _, + "file_server_no_valid_directory_behavior")); + } + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + struct stat stat_result = {}; + stat_result.st_mode = S_IFDIR; + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); +} + +TEST_F(FileServerFilterTest, ErrorsOnDirectoryWithImpossiblyConfiguredBehaviorForCoverage) { + auto filter = FileServerFilter::createShared(configFromYaml(R"( +path_mappings: + - path_prefix: /path1 + filesystem_prefix: fs1 +directory_behaviors: + - {} +)")); + initFilter(*filter); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + { + InSequence seq; + EXPECT_CALL(*mock_async_file_manager_, stat); + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::InternalServerError, _, _, _, + "file_server_empty_behavior_type")); + } + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); + struct stat stat_result = {}; + stat_result.st_mode = S_IFDIR; + mock_async_file_manager_->nextActionCompletes(absl::StatusOr{stat_result}); + pumpDispatcher(); +} + TEST_F(FileServerFilterTest, TriesAllDirectoryBehaviorsInOrder) { auto filter = testFilter(); Http::TestRequestHeaderMapImpl request_headers{ diff --git a/test/extensions/filters/http/file_server/integration_test.cc b/test/extensions/filters/http/file_server/integration_test.cc index c00962b6bf062..b084aa0b8ed82 100644 --- a/test/extensions/filters/http/file_server/integration_test.cc +++ b/test/extensions/filters/http/file_server/integration_test.cc @@ -134,6 +134,42 @@ TEST_P(FileServerIntegrationTest, IgnoresInvalidlyFormattedRangeHeader) { EXPECT_THAT(response->body(), banana_html_contents_); } +TEST_P(FileServerIntegrationTest, IgnoresMultipleRangeHeader) { + initializeFilter(testConfig()); + Http::TestRequestHeaderMapImpl request_headers = requestPath("/path1/banana.html"); + request_headers.addCopy("range", "bytes=3-5,6-9"); + auto response = sendHeaderOnlyRequestAwaitResponse(request_headers); + EXPECT_THAT( + response->headers(), + AllOf(ContainsHeader(":status", "200"), ContainsHeader("content-type", "text/html"), + ContainsHeader("content-length", absl::StrCat(banana_html_contents_.length())))); + EXPECT_THAT(response->body(), banana_html_contents_); +} + +TEST_P(FileServerIntegrationTest, IgnoresSuffixRangeHeader) { + initializeFilter(testConfig()); + Http::TestRequestHeaderMapImpl request_headers = requestPath("/path1/banana.html"); + request_headers.addCopy("range", "bytes=-6"); + auto response = sendHeaderOnlyRequestAwaitResponse(request_headers); + EXPECT_THAT( + response->headers(), + AllOf(ContainsHeader(":status", "200"), ContainsHeader("content-type", "text/html"), + ContainsHeader("content-length", absl::StrCat(banana_html_contents_.length())))); + EXPECT_THAT(response->body(), banana_html_contents_); +} + +TEST_P(FileServerIntegrationTest, IgnoresNonNumericRangeHeader) { + initializeFilter(testConfig()); + Http::TestRequestHeaderMapImpl request_headers = requestPath("/path1/banana.html"); + request_headers.addCopy("range", "bytes=banana-"); + auto response = sendHeaderOnlyRequestAwaitResponse(request_headers); + EXPECT_THAT( + response->headers(), + AllOf(ContainsHeader(":status", "200"), ContainsHeader("content-type", "text/html"), + ContainsHeader("content-length", absl::StrCat(banana_html_contents_.length())))); + EXPECT_THAT(response->body(), banana_html_contents_); +} + TEST_P(FileServerIntegrationTest, ReadsSpecifiedFileWithRange) { initializeFilter(testConfig()); Http::TestRequestHeaderMapImpl request_headers = requestPath("/path1/banana.html"); From ff9ba4fb3e3bbed18c95a683ca9279d828dbd00a Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 2 Feb 2026 18:44:57 +0000 Subject: [PATCH 03/22] Accommodate rename of com_github_cncf_xds Signed-off-by: Raven Black --- api/envoy/extensions/filters/http/file_server/v3/BUILD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/envoy/extensions/filters/http/file_server/v3/BUILD b/api/envoy/extensions/filters/http/file_server/v3/BUILD index 5b108dcfee6c7..d84b1253a0c94 100644 --- a/api/envoy/extensions/filters/http/file_server/v3/BUILD +++ b/api/envoy/extensions/filters/http/file_server/v3/BUILD @@ -7,7 +7,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ "//envoy/extensions/common/async_files/v3:pkg", - "@com_github_cncf_xds//udpa/annotations:pkg", - "@com_github_cncf_xds//xds/annotations/v3:pkg", + "@xds//udpa/annotations:pkg", + "@xds//xds/annotations/v3:pkg", ], ) From ff97678b0e52be875251263d070b329dd44c7558 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 2 Feb 2026 20:10:57 +0000 Subject: [PATCH 04/22] Move abslStatusToHttpStatus into its own library, and add coverage Signed-off-by: Raven Black --- .../extensions/filters/http/file_server/BUILD | 10 ++++ .../file_server/absl_status_to_http_status.cc | 48 +++++++++++++++++++ .../file_server/absl_status_to_http_status.h | 14 ++++++ .../filters/http/file_server/file_streamer.cc | 37 +------------- .../extensions/filters/http/file_server/BUILD | 9 ++++ .../absl_status_to_http_status_test.cc | 36 ++++++++++++++ 6 files changed, 118 insertions(+), 36 deletions(-) create mode 100644 source/extensions/filters/http/file_server/absl_status_to_http_status.cc create mode 100644 source/extensions/filters/http/file_server/absl_status_to_http_status.h create mode 100644 test/extensions/filters/http/file_server/absl_status_to_http_status_test.cc diff --git a/source/extensions/filters/http/file_server/BUILD b/source/extensions/filters/http/file_server/BUILD index 581407a4e9a4e..c803e9de3be17 100644 --- a/source/extensions/filters/http/file_server/BUILD +++ b/source/extensions/filters/http/file_server/BUILD @@ -9,6 +9,15 @@ licenses(["notice"]) # Apache 2 envoy_extension_package() +envoy_cc_library( + name = "absl_status_to_http_status", + srcs = ["absl_status_to_http_status.cc"], + hdrs = ["absl_status_to_http_status.h"], + deps = [ + "//envoy/http:codes_interface", + ], +) + envoy_cc_library( name = "file_server_lib", srcs = [ @@ -22,6 +31,7 @@ envoy_cc_library( "filter_config.h", ], deps = [ + ":absl_status_to_http_status", "//envoy/buffer:buffer_interface", "//envoy/server:instance_interface", "//source/common/common:enum_to_int", diff --git a/source/extensions/filters/http/file_server/absl_status_to_http_status.cc b/source/extensions/filters/http/file_server/absl_status_to_http_status.cc new file mode 100644 index 0000000000000..fb986756c2652 --- /dev/null +++ b/source/extensions/filters/http/file_server/absl_status_to_http_status.cc @@ -0,0 +1,48 @@ +#include "source/extensions/filters/http/file_server/absl_status_to_http_status.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +Http::Code abslStatusToHttpStatus(absl::StatusCode code) { + switch (code) { + case absl::StatusCode::kOk: + return Http::Code::OK; + case absl::StatusCode::kCancelled: + return static_cast(499); + case absl::StatusCode::kUnknown: + return Http::Code::InternalServerError; + case absl::StatusCode::kInvalidArgument: + return Http::Code::BadRequest; + case absl::StatusCode::kDeadlineExceeded: + return Http::Code::GatewayTimeout; + case absl::StatusCode::kNotFound: + return Http::Code::NotFound; + case absl::StatusCode::kAlreadyExists: + return Http::Code::Conflict; + case absl::StatusCode::kPermissionDenied: + return Http::Code::Forbidden; + case absl::StatusCode::kResourceExhausted: + return Http::Code::TooManyRequests; + case absl::StatusCode::kFailedPrecondition: + return Http::Code::BadRequest; + case absl::StatusCode::kAborted: + return Http::Code::Conflict; + case absl::StatusCode::kOutOfRange: + return Http::Code::RangeNotSatisfiable; + case absl::StatusCode::kUnimplemented: + return Http::Code::ServiceUnavailable; + case absl::StatusCode::kDataLoss: + return Http::Code::InternalServerError; + case absl::StatusCode::kUnauthenticated: + return Http::Code::Unauthorized; + default: + return Http::Code::InternalServerError; + } +} + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/source/extensions/filters/http/file_server/absl_status_to_http_status.h b/source/extensions/filters/http/file_server/absl_status_to_http_status.h new file mode 100644 index 0000000000000..701fd9865b56a --- /dev/null +++ b/source/extensions/filters/http/file_server/absl_status_to_http_status.h @@ -0,0 +1,14 @@ +#include "envoy/http/codes.h" +#include "absl/status/status.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +Http::Code abslStatusToHttpStatus(absl::StatusCode code); + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/file_server/file_streamer.cc b/source/extensions/filters/http/file_server/file_streamer.cc index a17ce61a68767..9c54ceb8d8173 100644 --- a/source/extensions/filters/http/file_server/file_streamer.cc +++ b/source/extensions/filters/http/file_server/file_streamer.cc @@ -5,6 +5,7 @@ #include "source/common/common/enum_to_int.h" #include "source/common/http/header_map_impl.h" #include "source/extensions/common/async_files/async_file_manager.h" +#include "source/extensions/filters/http/file_server/absl_status_to_http_status.h" namespace Envoy { namespace Extensions { @@ -12,42 +13,6 @@ namespace HttpFilters { namespace FileServer { namespace { -Http::Code abslStatusToHttpStatus(absl::StatusCode code) { - switch (code) { - case absl::StatusCode::kOk: - return Http::Code::OK; - case absl::StatusCode::kCancelled: - return static_cast(499); - case absl::StatusCode::kUnknown: - return Http::Code::InternalServerError; - case absl::StatusCode::kInvalidArgument: - return Http::Code::BadRequest; - case absl::StatusCode::kDeadlineExceeded: - return Http::Code::GatewayTimeout; - case absl::StatusCode::kNotFound: - return Http::Code::NotFound; - case absl::StatusCode::kAlreadyExists: - return Http::Code::Conflict; - case absl::StatusCode::kPermissionDenied: - return Http::Code::Forbidden; - case absl::StatusCode::kResourceExhausted: - return Http::Code::TooManyRequests; - case absl::StatusCode::kFailedPrecondition: - return Http::Code::BadRequest; - case absl::StatusCode::kAborted: - return Http::Code::Conflict; - case absl::StatusCode::kOutOfRange: - return Http::Code::RangeNotSatisfiable; - case absl::StatusCode::kUnimplemented: - return Http::Code::ServiceUnavailable; - case absl::StatusCode::kDataLoss: - return Http::Code::InternalServerError; - case absl::StatusCode::kUnauthenticated: - return Http::Code::Unauthorized; - default: - return Http::Code::InternalServerError; - } -} const Http::LowerCaseString& acceptRangesHeaderKey() { CONSTRUCT_ON_FIRST_USE(Http::LowerCaseString, "accept-ranges"); diff --git a/test/extensions/filters/http/file_server/BUILD b/test/extensions/filters/http/file_server/BUILD index b87a3faa8b00e..c721f501a01ef 100644 --- a/test/extensions/filters/http/file_server/BUILD +++ b/test/extensions/filters/http/file_server/BUILD @@ -1,5 +1,6 @@ load( "//bazel:envoy_build_system.bzl", + "envoy_cc_test", "envoy_package", ) load( @@ -11,6 +12,14 @@ licenses(["notice"]) # Apache 2 envoy_package() +envoy_cc_test( + name = "absl_status_to_http_status_test", + srcs = ["absl_status_to_http_status_test.cc"], + deps = [ + "//source/extensions/filters/http/file_server:absl_status_to_http_status", + ], +) + envoy_extension_cc_test( name = "file_server_test", srcs = [ diff --git a/test/extensions/filters/http/file_server/absl_status_to_http_status_test.cc b/test/extensions/filters/http/file_server/absl_status_to_http_status_test.cc new file mode 100644 index 0000000000000..96f1e7d474a6a --- /dev/null +++ b/test/extensions/filters/http/file_server/absl_status_to_http_status_test.cc @@ -0,0 +1,36 @@ +#include "source/extensions/filters/http/file_server/absl_status_to_http_status.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace FileServer { + +TEST(AbslStatusToHttpStatus, Coverage) { + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kOk), Http::Code::OK); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kCancelled), static_cast(499)); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kUnknown), Http::Code::InternalServerError); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kInvalidArgument), Http::Code::BadRequest); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kDeadlineExceeded), + Http::Code::GatewayTimeout); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kNotFound), Http::Code::NotFound); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kAlreadyExists), Http::Code::Conflict); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kPermissionDenied), Http::Code::Forbidden); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kResourceExhausted), + Http::Code::TooManyRequests); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kFailedPrecondition), Http::Code::BadRequest); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kAborted), Http::Code::Conflict); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kOutOfRange), Http::Code::RangeNotSatisfiable); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kUnimplemented), + Http::Code::ServiceUnavailable); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kDataLoss), Http::Code::InternalServerError); + EXPECT_EQ(abslStatusToHttpStatus(absl::StatusCode::kUnauthenticated), Http::Code::Unauthorized); + EXPECT_EQ(abslStatusToHttpStatus(static_cast(99999999)), + Http::Code::InternalServerError); +} + +} // namespace FileServer +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy From f46d7dd90dd2cc706fa20ce4cad0fff0f3b4b3c3 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 2 Feb 2026 20:56:54 +0000 Subject: [PATCH 05/22] Add one more line of test coverage! Signed-off-by: Raven Black --- .../filters/http/file_server/absl_status_to_http_status.cc | 2 +- test/extensions/filters/http/file_server/filter_test.cc | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/http/file_server/absl_status_to_http_status.cc b/source/extensions/filters/http/file_server/absl_status_to_http_status.cc index fb986756c2652..c2486a5e81731 100644 --- a/source/extensions/filters/http/file_server/absl_status_to_http_status.cc +++ b/source/extensions/filters/http/file_server/absl_status_to_http_status.cc @@ -45,4 +45,4 @@ Http::Code abslStatusToHttpStatus(absl::StatusCode code) { } // namespace FileServer } // namespace HttpFilters } // namespace Extensions -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/test/extensions/filters/http/file_server/filter_test.cc b/test/extensions/filters/http/file_server/filter_test.cc index 0f244d23121a1..689be97fbcec2 100644 --- a/test/extensions/filters/http/file_server/filter_test.cc +++ b/test/extensions/filters/http/file_server/filter_test.cc @@ -97,6 +97,12 @@ TEST_F(FileServerFilterTest, PassThroughIfNoPath) { EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter->decodeHeaders(request_headers, true)); } +TEST_F(FileServerFilterTest, DestroyBeforeHeadersIsOkay) { + auto filter = testFilter(); + filter->onDestroy(); + // Should not crash due to uninitialized abort functions or anything! +} + TEST_F(FileServerFilterTest, PassThroughIfNotMatchingPathMapping) { auto filter = testFilter(); Http::TestRequestHeaderMapImpl request_headers{ From 908bee7046a7015bb1e98c3f453fc9d577d1d1a4 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 2 Feb 2026 21:24:25 +0000 Subject: [PATCH 06/22] Remove unused usings Signed-off-by: Raven Black --- test/extensions/filters/http/file_server/filter_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/extensions/filters/http/file_server/filter_test.cc b/test/extensions/filters/http/file_server/filter_test.cc index 689be97fbcec2..eb6a067e42f45 100644 --- a/test/extensions/filters/http/file_server/filter_test.cc +++ b/test/extensions/filters/http/file_server/filter_test.cc @@ -17,11 +17,9 @@ namespace FileServer { using Extensions::Common::AsyncFiles::MockAsyncFileContext; using Extensions::Common::AsyncFiles::MockAsyncFileHandle; using Extensions::Common::AsyncFiles::MockAsyncFileManager; -using StatusHelpers::HasStatus; using ::testing::AnyNumber; using ::testing::InSequence; using ::testing::NiceMock; -using ::testing::NotNull; using ::testing::ReturnRef; using ::testing::StrictMock; From 16c41f063b05dc42cebf4b83b9603d35a1f730d3 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 2 Feb 2026 22:41:29 +0000 Subject: [PATCH 07/22] Remove deps that are inherited anyway, to avoid conflicting with #43265 Signed-off-by: Raven Black --- test/extensions/filters/http/file_server/BUILD | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/extensions/filters/http/file_server/BUILD b/test/extensions/filters/http/file_server/BUILD index c721f501a01ef..c6177bf55d379 100644 --- a/test/extensions/filters/http/file_server/BUILD +++ b/test/extensions/filters/http/file_server/BUILD @@ -49,7 +49,5 @@ envoy_extension_cc_test( deps = [ "//source/extensions/filters/http/file_server:config", "//test/integration:http_protocol_integration_lib", - "@com_google_absl//absl/strings", - "@com_google_absl//absl/strings:string_view", ], ) From d0749b19a3f9a0d931a6993e15189161568509b1 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Tue, 3 Feb 2026 16:27:25 +0000 Subject: [PATCH 08/22] try_file->default_file, directory_list->list, drop oneof Signed-off-by: Raven Black --- .../http/file_server/v3/file_server.proto | 18 +++--- .../filters/http/file_server/config.cc | 17 ++++-- .../filters/http/file_server/file_streamer.cc | 19 +++--- .../filters/http/file_server/config_test.cc | 60 +++++++++++++------ .../filters/http/file_server/filter_test.cc | 10 ++-- .../http/file_server/integration_test.cc | 6 +- 6 files changed, 79 insertions(+), 51 deletions(-) diff --git a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto index d6792895ff034..aa17bc69560fb 100644 --- a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto +++ b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto @@ -42,16 +42,14 @@ message FileServerConfig { message DirectoryList { } - oneof behavior_type { - option (validate.required) = true; - - // Attempts to serve the given file within the directory, e.g. ``index.html``. - string try_file = 1 [(validate.rules).string = {pattern: "^[^/]+$"}]; - - // Responds with an html formatted list of the files in the directory. - // [#not-implemented-hide:] Directory operations currently have no async implementation. - DirectoryList directory_list = 2; - } + // Attempts to serve the given file within the directory, e.g. ``index.html``. + // Precisely one of `default_file` and `list` must be set per `DirectoryBehavior`. + string default_file = 1 [(validate.rules).string = {pattern: "^[^/]*$"}]; + + // Responds with an html formatted list of the files and subdirectories in the directory. + // Precisely one of `default_file` and `list` must be set per `DirectoryBehavior`. + // [#not-implemented-hide:] Directory operations currently have no async implementation. + DirectoryList list = 2; } // A configuration for an AsyncFileManager. diff --git a/source/extensions/filters/http/file_server/config.cc b/source/extensions/filters/http/file_server/config.cc index ee7ea06cdbe85..848fb66e0f26d 100644 --- a/source/extensions/filters/http/file_server/config.cc +++ b/source/extensions/filters/http/file_server/config.cc @@ -29,16 +29,25 @@ absl::Status validateProto(const ProtoFileServerConfig& config) { } seen.erase(seen.begin(), seen.end()); bool directory_tried = false; + static const absl::string_view directory_options = "default_file or list"; for (const auto& directory_behavior : config.directory_behaviors()) { - if (!directory_behavior.try_file().empty()) { - std::tie(std::ignore, inserted) = seen.emplace(directory_behavior.try_file()); + if (directory_behavior.default_file().empty() && !directory_behavior.has_list()) { + return absl::InvalidArgumentError( + absl::StrCat("directory_behavior must set one of ", directory_options)); + } + if (!directory_behavior.default_file().empty() && directory_behavior.has_list()) { + return absl::InvalidArgumentError( + absl::StrCat("directory_behavior must have only one of ", directory_options)); + } + if (!directory_behavior.default_file().empty()) { + std::tie(std::ignore, inserted) = seen.emplace(directory_behavior.default_file()); if (!inserted) { return absl::InvalidArgumentError(absl::StrCat( - "duplicate try_file in directory_behaviors: ", directory_behavior.try_file())); + "duplicate default_file in directory_behaviors: ", directory_behavior.default_file())); } } else { if (directory_tried) { - return absl::InvalidArgumentError("multiple directory_list directives"); + return absl::InvalidArgumentError("multiple list directives"); } directory_tried = true; } diff --git a/source/extensions/filters/http/file_server/file_streamer.cc b/source/extensions/filters/http/file_server/file_streamer.cc index 9c54ceb8d8173..ae58da7055d09 100644 --- a/source/extensions/filters/http/file_server/file_streamer.cc +++ b/source/extensions/filters/http/file_server/file_streamer.cc @@ -60,10 +60,9 @@ void FileStreamer::startDir(int behavior_index) { client_.errorFromFile(Http::Code::Forbidden, "file_server_no_valid_directory_behavior"); return; } - switch (behavior->behavior_type_case()) { - case ProtoFileServerConfig::DirectoryBehavior::kTryFile: + if (!behavior->default_file().empty()) { cancel_callback_ = file_server_config_->asyncFileManager()->openExistingFile( - dispatcher_, (file_path_ / std::filesystem::path{behavior->try_file()}).string(), + dispatcher_, (file_path_ / std::filesystem::path{behavior->default_file()}).string(), Common::AsyncFiles::AsyncFileManager::Mode::ReadOnly, [this, behavior_index](absl::StatusOr result) { if (!result.ok()) { @@ -71,18 +70,18 @@ void FileStreamer::startDir(int behavior_index) { // Since the action is dispatched, this isn't recursion. return startDir(behavior_index + 1); } - file_path_ = - file_path_ / std::filesystem::path{ - file_server_config_->directoryBehavior(behavior_index)->try_file()}; + file_path_ = file_path_ / + std::filesystem::path{ + file_server_config_->directoryBehavior(behavior_index)->default_file()}; async_file_ = std::move(result.value()); startFile(); }); return; - case ProtoFileServerConfig::DirectoryBehavior::kDirectoryList: - client_.errorFromFile(Http::Code::Forbidden, "file_server_directory_list_not_implemented"); + } else if (behavior->has_list()) { + client_.errorFromFile(Http::Code::Forbidden, "file_server_list_not_implemented"); return; - case ProtoFileServerConfig::DirectoryBehavior::BEHAVIOR_TYPE_NOT_SET: - // Unreachable due to proto validations. + } else { + // Normally unreachable due to proto validations. client_.errorFromFile(Http::Code::InternalServerError, "file_server_empty_behavior_type"); return; } diff --git a/test/extensions/filters/http/file_server/config_test.cc b/test/extensions/filters/http/file_server/config_test.cc index 92ded311f2e26..d0a79b1728f7f 100644 --- a/test/extensions/filters/http/file_server/config_test.cc +++ b/test/extensions/filters/http/file_server/config_test.cc @@ -81,11 +81,32 @@ class FileServerConfigTest : public testing::Test { NiceMock mock_server_factory_context_; }; +TEST_F(FileServerConfigTest, EmptyDirectoryBehaviorIsConfigError) { + auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +directory_behaviors: + - {} +)"), + "stats", mock_factory_context_); + EXPECT_THAT(status_or, + HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("must set one of"))); +} + +TEST_F(FileServerConfigTest, OverpopulatedDirectoryBehaviorIsConfigError) { + auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +directory_behaviors: + - default_file: "index.html" + list: {} +)"), + "stats", mock_factory_context_); + EXPECT_THAT(status_or, + HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("must have only one of"))); +} + TEST_F(FileServerConfigTest, DuplicateDirectoryFilesIsConfigError) { auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( directory_behaviors: - - try_file: "index.html" - - try_file: "index.html" + - default_file: "index.html" + - default_file: "index.html" )"), "stats", mock_factory_context_); EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("index.html"))); @@ -94,12 +115,12 @@ TEST_F(FileServerConfigTest, DuplicateDirectoryFilesIsConfigError) { TEST_F(FileServerConfigTest, DuplicateDirectoryListIsConfigError) { auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( directory_behaviors: - - directory_list: {} - - directory_list: {} + - list: {} + - list: {} )"), "stats", mock_factory_context_); - EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, - HasSubstr("multiple directory_list directives"))); + EXPECT_THAT(status_or, + HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("multiple list directives"))); } TEST_F(FileServerConfigTest, DuplicatePathPrefixIsConfigError) { @@ -148,9 +169,9 @@ TEST_F(FileServerConfigTest, ValidConfigPopulatesConfigObjectAppropriately) { "README": "text/markdown" default_content_type: "application/octet-stream" directory_behaviors: - - try_file: "index.html" - - try_file: "index.txt" - - directory_list: {} + - default_file: "index.html" + - default_file: "index.txt" + - list: {} )")); EXPECT_THAT(config->pathMapping("/"), IsNull()); EXPECT_THAT(config->pathMapping("/path1"), IsNull()); @@ -175,15 +196,16 @@ default_content_type: "application/octet-stream" EXPECT_THAT(config->contentTypeForPath("/fs2/README."), Eq("text/x-no-suffix")); EXPECT_THAT(config->contentTypeForPath("/fs1/other"), Eq("application/octet-stream")); EXPECT_THAT(config->asyncFileManager(), NotNull()); - EXPECT_THAT(config->directoryBehavior(0), - OptRefWith(Property("try_file", &ProtoFileServerConfig::DirectoryBehavior::try_file, - Eq("index.html")))); - EXPECT_THAT(config->directoryBehavior(1), - OptRefWith(Property("try_file", &ProtoFileServerConfig::DirectoryBehavior::try_file, - Eq("index.txt")))); + EXPECT_THAT( + config->directoryBehavior(0), + OptRefWith(Property("default_file", &ProtoFileServerConfig::DirectoryBehavior::default_file, + Eq("index.html")))); + EXPECT_THAT( + config->directoryBehavior(1), + OptRefWith(Property("default_file", &ProtoFileServerConfig::DirectoryBehavior::default_file, + Eq("index.txt")))); EXPECT_THAT(config->directoryBehavior(2), - OptRefWith(Property("has_directory_list", - &ProtoFileServerConfig::DirectoryBehavior::has_directory_list, + OptRefWith(Property("has_list", &ProtoFileServerConfig::DirectoryBehavior::has_list, Eq(true)))); EXPECT_THAT(config->directoryBehavior(3), Eq(absl::nullopt)); } @@ -192,8 +214,8 @@ TEST_F(FileServerConfigTest, DuplicateDirectoryFilesIsConfigErrorInRouteConfig) auto status_or = factory()->createRouteSpecificFilterConfig( configFromYaml(R"( directory_behaviors: - - try_file: "index.html" - - try_file: "index.html" + - default_file: "index.html" + - default_file: "index.html" )"), mock_server_factory_context_, ProtobufMessage::getNullValidationVisitor()); EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("index.html"))); diff --git a/test/extensions/filters/http/file_server/filter_test.cc b/test/extensions/filters/http/file_server/filter_test.cc index eb6a067e42f45..a70f91eb63948 100644 --- a/test/extensions/filters/http/file_server/filter_test.cc +++ b/test/extensions/filters/http/file_server/filter_test.cc @@ -57,9 +57,9 @@ class FileServerFilterTest : public testing::Test { "html": "text/html" default_content_type: "application/octet-stream" directory_behaviors: - - try_file: "index.html" - - try_file: "index.txt" - - directory_list: {} + - default_file: "index.html" + - default_file: "index.txt" + - list: {} )")); initFilter(*filter); return filter; @@ -266,8 +266,8 @@ TEST_F(FileServerFilterTest, TriesAllDirectoryBehaviorsInOrder) { EXPECT_CALL(*mock_async_file_manager_, stat); EXPECT_CALL(*mock_async_file_manager_, openExistingFile(_, "fs1/foo/index.html", _, _)); EXPECT_CALL(*mock_async_file_manager_, openExistingFile(_, "fs1/foo/index.txt", _, _)); - EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::Forbidden, _, _, _, - "file_server_directory_list_not_implemented")); + EXPECT_CALL(decoder_callbacks_, + sendLocalReply(Http::Code::Forbidden, _, _, _, "file_server_list_not_implemented")); } EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); struct stat stat_result = {}; diff --git a/test/extensions/filters/http/file_server/integration_test.cc b/test/extensions/filters/http/file_server/integration_test.cc index b084aa0b8ed82..ad70877381e60 100644 --- a/test/extensions/filters/http/file_server/integration_test.cc +++ b/test/extensions/filters/http/file_server/integration_test.cc @@ -64,9 +64,9 @@ name: "envoy.filters.http.file_server" "html": "text/html" default_content_type: "application/octet-stream" directory_behaviors: - - try_file: "index.html" - - try_file: "index.txt" - - directory_list: {} + - default_file: "index.html" + - default_file: "index.txt" + - list: {} )"); } From edbbdba2947e3273d7ecaf17a9f926479fdc411e Mon Sep 17 00:00:00 2001 From: Raven Black Date: Tue, 3 Feb 2026 17:34:24 +0000 Subject: [PATCH 09/22] Add virtual destructor to FileStreamerClient Signed-off-by: Raven Black --- source/extensions/filters/http/file_server/file_streamer.h | 1 + 1 file changed, 1 insertion(+) diff --git a/source/extensions/filters/http/file_server/file_streamer.h b/source/extensions/filters/http/file_server/file_streamer.h index a1bbe74f6d2fe..d937aee77164b 100644 --- a/source/extensions/filters/http/file_server/file_streamer.h +++ b/source/extensions/filters/http/file_server/file_streamer.h @@ -24,6 +24,7 @@ class FileStreamerClient { // Return true to keep going - false is for HEAD requests. virtual bool headersFromFile(Http::ResponseHeaderMapPtr response_headers) PURE; virtual void bodyChunkFromFile(Buffer::InstancePtr buf, bool end_stream) PURE; + virtual ~FileStreamerClient() = default; }; class FileStreamer { From 9b9b443ab83a738e1e6ed1d61a1ae7135ea53b1d Mon Sep 17 00:00:00 2001 From: Raven Black Date: Tue, 3 Feb 2026 17:41:13 +0000 Subject: [PATCH 10/22] DirectoryList->List too Signed-off-by: Raven Black --- .../extensions/filters/http/file_server/v3/file_server.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto index aa17bc69560fb..f02596229d163 100644 --- a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto +++ b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto @@ -39,7 +39,7 @@ message FileServerConfig { } message DirectoryBehavior { - message DirectoryList { + message List { } // Attempts to serve the given file within the directory, e.g. ``index.html``. @@ -49,7 +49,7 @@ message FileServerConfig { // Responds with an html formatted list of the files and subdirectories in the directory. // Precisely one of `default_file` and `list` must be set per `DirectoryBehavior`. // [#not-implemented-hide:] Directory operations currently have no async implementation. - DirectoryList list = 2; + List list = 2; } // A configuration for an AsyncFileManager. From 0da138309850714004b82ccd7c5ef0de978bb83a Mon Sep 17 00:00:00 2001 From: Raven Black Date: Tue, 3 Feb 2026 20:10:59 +0000 Subject: [PATCH 11/22] Format Signed-off-by: Raven Black --- .../filters/http/file_server/absl_status_to_http_status.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/extensions/filters/http/file_server/absl_status_to_http_status.h b/source/extensions/filters/http/file_server/absl_status_to_http_status.h index 701fd9865b56a..1bd2b5ee906c7 100644 --- a/source/extensions/filters/http/file_server/absl_status_to_http_status.h +++ b/source/extensions/filters/http/file_server/absl_status_to_http_status.h @@ -1,4 +1,7 @@ +#pragma once + #include "envoy/http/codes.h" + #include "absl/status/status.h" namespace Envoy { From 006ed2d5f867a203e482d8de23d588c7cd49cca8 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Tue, 3 Feb 2026 20:11:45 +0000 Subject: [PATCH 12/22] Double backticks Signed-off-by: Raven Black --- .../extensions/filters/http/file_server/v3/file_server.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto index f02596229d163..4891f326a7608 100644 --- a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto +++ b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto @@ -43,11 +43,11 @@ message FileServerConfig { } // Attempts to serve the given file within the directory, e.g. ``index.html``. - // Precisely one of `default_file` and `list` must be set per `DirectoryBehavior`. + // Precisely one of ``default_file`` and ``list`` must be set per ``DirectoryBehavior``. string default_file = 1 [(validate.rules).string = {pattern: "^[^/]*$"}]; // Responds with an html formatted list of the files and subdirectories in the directory. - // Precisely one of `default_file` and `list` must be set per `DirectoryBehavior`. + // Precisely one of ``default_file`` and ``list`` must be set per ``DirectoryBehavior``. // [#not-implemented-hide:] Directory operations currently have no async implementation. List list = 2; } From 52c4555bd29f6c09d9deabd0a423c715cc16601c Mon Sep 17 00:00:00 2001 From: Raven Black Date: Thu, 5 Feb 2026 17:52:14 +0000 Subject: [PATCH 13/22] path_prefix->request_path_prefix, filesystem_prefix->file_path_prefix Signed-off-by: Raven Black --- .../http/file_server/v3/file_server.proto | 12 ++++++------ .../filters/http/file_server/config.cc | 4 ++-- .../filters/http/file_server/filter_config.cc | 10 +++++----- .../filters/http/file_server/config_test.cc | 18 +++++++++--------- .../filters/http/file_server/filter_test.cc | 12 ++++++------ .../http/file_server/integration_test.cc | 8 ++++---- 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto index 4891f326a7608..f4f17086afd8f 100644 --- a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto +++ b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto @@ -23,19 +23,19 @@ option (xds.annotations.v3.file_status).work_in_progress = true; // [#next-free-field: 6] message FileServerConfig { message PathMapping { - // If no ``path_prefix`` is matched, the filter does not intercept a request. + // If no ``request_path_prefix`` is matched, the filter does not intercept a request. // - // If a ``path_prefix`` is matched, that prefix is removed from the request - // and replaced with ``filesystem_prefix`` to form a filesystem path for + // If a ``request_path_prefix`` is matched, that prefix is removed from the request + // and replaced with ``file_path_prefix`` to form a filesystem path for // the request. // // Prefix ``/`` will match all GET requests. - string path_prefix = 1 [(validate.rules).string = {min_len: 1}]; + string request_path_prefix = 1 [(validate.rules).string = {min_len: 1}]; - // Replaces a matched ``path_prefix`` to form a filesystem path for a + // Replaces a matched ``request_path_prefix`` to form a filesystem path for a // request. May be relative to the working directory of the envoy execution, // or an absolute path. - string filesystem_prefix = 2 [(validate.rules).string = {min_len: 1}]; + string file_path_prefix = 2 [(validate.rules).string = {min_len: 1}]; } message DirectoryBehavior { diff --git a/source/extensions/filters/http/file_server/config.cc b/source/extensions/filters/http/file_server/config.cc index 848fb66e0f26d..b5d99b486a7a2 100644 --- a/source/extensions/filters/http/file_server/config.cc +++ b/source/extensions/filters/http/file_server/config.cc @@ -21,10 +21,10 @@ absl::Status validateProto(const ProtoFileServerConfig& config) { absl::flat_hash_set seen; bool inserted; for (const auto& mapping : config.path_mappings()) { - std::tie(std::ignore, inserted) = seen.emplace(mapping.path_prefix()); + std::tie(std::ignore, inserted) = seen.emplace(mapping.request_path_prefix()); if (!inserted) { return absl::InvalidArgumentError( - absl::StrCat("duplicate path_prefix: ", mapping.path_prefix())); + absl::StrCat("duplicate request_path_prefix: ", mapping.request_path_prefix())); } } seen.erase(seen.begin(), seen.end()); diff --git a/source/extensions/filters/http/file_server/filter_config.cc b/source/extensions/filters/http/file_server/filter_config.cc index d7d638f669670..2c047eda71cd4 100644 --- a/source/extensions/filters/http/file_server/filter_config.cc +++ b/source/extensions/filters/http/file_server/filter_config.cc @@ -17,7 +17,7 @@ RadixTree> makePathMappings(const ProtoFileServerConfig& config) { RadixTree> tree; for (const auto& mapping : config.path_mappings()) { - tree.add(mapping.path_prefix(), std::make_shared(mapping)); + tree.add(mapping.request_path_prefix(), std::make_shared(mapping)); } return tree; } @@ -63,9 +63,9 @@ absl::optional FileServerConfig::applyPathMapping(absl::string_view path_with_query, const ProtoFileServerConfig::PathMapping& mapping) { std::pair split = absl::StrSplit(path_with_query, '?'); - absl::string_view kept_path = split.first.substr(mapping.path_prefix().length()); + absl::string_view kept_path = split.first.substr(mapping.request_path_prefix().length()); if (kept_path.starts_with('/')) { - if (mapping.path_prefix().ends_with('/')) { + if (mapping.request_path_prefix().ends_with('/')) { // Avoid accepting a value that parses away a double-slash at the join-point. // (Other double-slashes will be rejected by the lexically_normal check.) return absl::nullopt; @@ -80,9 +80,9 @@ FileServerConfig::applyPathMapping(absl::string_view path_with_query, return absl::nullopt; } std::filesystem::path file_path = - std::filesystem::path{mapping.filesystem_prefix()} / std::filesystem::path{kept_path}; + std::filesystem::path{mapping.file_path_prefix()} / std::filesystem::path{kept_path}; if (file_path != file_path.lexically_normal() || - !file_path.string().starts_with(mapping.filesystem_prefix())) { + !file_path.string().starts_with(mapping.file_path_prefix())) { // Ensure we're not accidentally looking outside the designated filesystem prefix. return absl::nullopt; } diff --git a/test/extensions/filters/http/file_server/config_test.cc b/test/extensions/filters/http/file_server/config_test.cc index d0a79b1728f7f..efd0f9a9a1ae8 100644 --- a/test/extensions/filters/http/file_server/config_test.cc +++ b/test/extensions/filters/http/file_server/config_test.cc @@ -123,13 +123,13 @@ TEST_F(FileServerConfigTest, DuplicateDirectoryListIsConfigError) { HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("multiple list directives"))); } -TEST_F(FileServerConfigTest, DuplicatePathPrefixIsConfigError) { +TEST_F(FileServerConfigTest, DuplicateRequestPathPrefixIsConfigError) { auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( path_mappings: - - path_prefix: "/banana" - filesystem_prefix: "/banana" - - path_prefix: "/banana" - filesystem_prefix: "/other" + - request_path_prefix: "/banana" + file_path_prefix: "/banana" + - request_path_prefix: "/banana" + file_path_prefix: "/other" )"), "stats", mock_factory_context_); EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("banana"))); @@ -158,10 +158,10 @@ TEST_F(FileServerConfigTest, ValidConfigPopulatesConfigObjectAppropriately) { thread_pool: thread_count: 1 path_mappings: - - path_prefix: /path1/ - filesystem_prefix: /fs1 - - path_prefix: /path1/path2 - filesystem_prefix: fs2 + - request_path_prefix: /path1/ + file_path_prefix: /fs1 + - request_path_prefix: /path1/path2 + file_path_prefix: fs2 content_types: "txt": "text/plain" "html": "text/html" diff --git a/test/extensions/filters/http/file_server/filter_test.cc b/test/extensions/filters/http/file_server/filter_test.cc index a70f91eb63948..e6b304d57ddb1 100644 --- a/test/extensions/filters/http/file_server/filter_test.cc +++ b/test/extensions/filters/http/file_server/filter_test.cc @@ -50,8 +50,8 @@ class FileServerFilterTest : public testing::Test { std::shared_ptr testFilter() { auto filter = FileServerFilter::createShared(configFromYaml(R"( path_mappings: - - path_prefix: /path1 - filesystem_prefix: fs1 + - request_path_prefix: /path1 + file_path_prefix: fs1 content_types: "txt": "text/plain" "html": "text/html" @@ -202,8 +202,8 @@ TEST_F(FileServerFilterTest, FilterOnDestroyWhileFileActionInFlightAbortsRespons TEST_F(FileServerFilterTest, ErrorsOnDirectoryWithNoConfiguredBehavior) { auto filter = FileServerFilter::createShared(configFromYaml(R"( path_mappings: - - path_prefix: /path1 - filesystem_prefix: fs1 + - request_path_prefix: /path1 + file_path_prefix: fs1 )")); initFilter(*filter); Http::TestRequestHeaderMapImpl request_headers{ @@ -228,8 +228,8 @@ TEST_F(FileServerFilterTest, ErrorsOnDirectoryWithNoConfiguredBehavior) { TEST_F(FileServerFilterTest, ErrorsOnDirectoryWithImpossiblyConfiguredBehaviorForCoverage) { auto filter = FileServerFilter::createShared(configFromYaml(R"( path_mappings: - - path_prefix: /path1 - filesystem_prefix: fs1 + - request_path_prefix: /path1 + file_path_prefix: fs1 directory_behaviors: - {} )")); diff --git a/test/extensions/filters/http/file_server/integration_test.cc b/test/extensions/filters/http/file_server/integration_test.cc index ad70877381e60..204d0eee87be2 100644 --- a/test/extensions/filters/http/file_server/integration_test.cc +++ b/test/extensions/filters/http/file_server/integration_test.cc @@ -53,11 +53,11 @@ name: "envoy.filters.http.file_server" thread_pool: thread_count: 1 path_mappings: - - path_prefix: /path1 - filesystem_prefix: )", + - request_path_prefix: /path1 + file_path_prefix: )", testTmpDir(), R"(/fs1 - - path_prefix: /path1/path2 - filesystem_prefix: )", + - request_path_prefix: /path1/path2 + file_path_prefix: )", testTmpDir(), R"(/fs2 content_types: "txt": "text/plain" From 81ae8dc5061b1729465ce965dcf26722d32ede1b Mon Sep 17 00:00:00 2001 From: Raven Black Date: Fri, 6 Feb 2026 20:08:11 +0000 Subject: [PATCH 14/22] Format Signed-off-by: Raven Black --- source/extensions/filters/http/file_server/filter_config.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/http/file_server/filter_config.cc b/source/extensions/filters/http/file_server/filter_config.cc index 2c047eda71cd4..a900eae0be849 100644 --- a/source/extensions/filters/http/file_server/filter_config.cc +++ b/source/extensions/filters/http/file_server/filter_config.cc @@ -17,7 +17,8 @@ RadixTree> makePathMappings(const ProtoFileServerConfig& config) { RadixTree> tree; for (const auto& mapping : config.path_mappings()) { - tree.add(mapping.request_path_prefix(), std::make_shared(mapping)); + tree.add(mapping.request_path_prefix(), + std::make_shared(mapping)); } return tree; } From f0481d7838f4c0501ee273a38a035a46b6dd9cbc Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 9 Feb 2026 18:53:20 +0000 Subject: [PATCH 15/22] Address copilot comments Signed-off-by: Raven Black --- .../http/file_server/v3/file_server.proto | 8 +++---- .../filters/http/file_server/config.cc | 3 ++- .../filters/http/file_server/file_streamer.cc | 6 ++++- .../filters/http/file_server/filter.cc | 10 ++++++++ .../filters/http/file_server/filter.h | 2 +- .../filters/http/file_server/filter_config.cc | 5 +++- .../filters/http/file_server/filter_config.h | 1 + .../filters/http/file_server/config_test.cc | 3 +-- .../filters/http/file_server/filter_test.cc | 23 +++++++++++++++++++ .../http/file_server/integration_test.cc | 1 + 10 files changed, 51 insertions(+), 11 deletions(-) diff --git a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto index f4f17086afd8f..27165e38b0b8e 100644 --- a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto +++ b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto @@ -52,12 +52,10 @@ message FileServerConfig { List list = 2; } - // A configuration for an AsyncFileManager. + // A configuration for the AsyncFileManager to be used to read from the filesystem. // - // This can be unset to allow for per-route-only configs. If this is unset - // on a per-route config and set in the route config, the version in the - // route config is used. If this is unset either way when a ``path_mapping`` - // is matched, a 503 Internal Server Error response is served. + // This may be unset to allow for per-route-only configs. If this is unset when + // a ``path_mapping`` is matched, a 500 Internal Server Error response is served. common.async_files.v3.AsyncFileManagerConfig manager_config = 1; // The longest matching path_mapping takes precedence. diff --git a/source/extensions/filters/http/file_server/config.cc b/source/extensions/filters/http/file_server/config.cc index b5d99b486a7a2..550afa273b724 100644 --- a/source/extensions/filters/http/file_server/config.cc +++ b/source/extensions/filters/http/file_server/config.cc @@ -9,7 +9,8 @@ #include "source/extensions/filters/http/file_server/filter.h" #include "source/extensions/filters/http/file_server/filter_config.h" -#include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" +#include "absl/strings/str_cat.h" namespace Envoy { namespace Extensions { diff --git a/source/extensions/filters/http/file_server/file_streamer.cc b/source/extensions/filters/http/file_server/file_streamer.cc index ae58da7055d09..c4adb38025146 100644 --- a/source/extensions/filters/http/file_server/file_streamer.cc +++ b/source/extensions/filters/http/file_server/file_streamer.cc @@ -7,6 +7,8 @@ #include "source/extensions/common/async_files/async_file_manager.h" #include "source/extensions/filters/http/file_server/absl_status_to_http_status.h" +#include "absl/strings/str_cat.h" + namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -22,6 +24,7 @@ const Http::LowerCaseString& acceptRangesHeaderKey() { void FileStreamer::begin(const FileServerConfig& config, Event::Dispatcher& dispatcher, uint64_t start, uint64_t end, std::filesystem::path file_path) { + ASSERT(config.asyncFileManager() != nullptr); file_server_config_ = &config; dispatcher_ = &dispatcher; pos_ = start; @@ -96,7 +99,8 @@ void FileStreamer::startFile() { return; } const struct stat& s = result.value(); - if (static_cast(s.st_size) < end_) { + if (static_cast(s.st_size) < end_ || static_cast(s.st_size) < pos_ || + (end_ != 0 && end_ < pos_)) { client_.errorFromFile(Http::Code::RangeNotSatisfiable, "file_server_range_not_satisfiable"); return; } diff --git a/source/extensions/filters/http/file_server/filter.cc b/source/extensions/filters/http/file_server/filter.cc index b99be15e559de..a06e2e1557ef8 100644 --- a/source/extensions/filters/http/file_server/filter.cc +++ b/source/extensions/filters/http/file_server/filter.cc @@ -10,6 +10,10 @@ #include "source/common/http/utility.h" #include "source/extensions/filters/http/file_server/filter_config.h" +#include "absl/strings/match.h" +#include "absl/strings/numbers.h" +#include "absl/strings/str_split.h" + namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -101,6 +105,12 @@ Http::FilterHeadersStatus FileServerFilter::decodeHeaders(RequestHeaderMap& head absl::nullopt, "file_server_rejected_method"); return Http::FilterHeadersStatus::StopIteration; } + if (config->asyncFileManager() == nullptr) { + decoder_callbacks_->sendLocalReply( + Http::Code::InternalServerError, CodeUtility::toString(Http::Code::InternalServerError), + nullptr, absl::nullopt, "file_server_no_file_manager_configured"); + return Http::FilterHeadersStatus::StopIteration; + } // Parse range header, if present, into start and end (otherwise or on error, 0,0) auto [start, end] = parseRangeHeader(headers); is_head_ = headers.Method()->value() == Http::Headers::get().MethodValues.Head; diff --git a/source/extensions/filters/http/file_server/filter.h b/source/extensions/filters/http/file_server/filter.h index 53db15d204717..2ea680dab914f 100644 --- a/source/extensions/filters/http/file_server/filter.h +++ b/source/extensions/filters/http/file_server/filter.h @@ -47,7 +47,7 @@ class FileServerFilter : public Http::PassThroughDecoderFilter, std::shared_ptr file_server_config_; friend class FileServerConfigTest; // Allow test access to file_server_config_. FileStreamer file_streamer_; - bool is_head_; + bool is_head_ = false; bool headers_sent_ = false; }; diff --git a/source/extensions/filters/http/file_server/filter_config.cc b/source/extensions/filters/http/file_server/filter_config.cc index a900eae0be849..64a0cc13ea882 100644 --- a/source/extensions/filters/http/file_server/filter_config.cc +++ b/source/extensions/filters/http/file_server/filter_config.cc @@ -6,6 +6,8 @@ #include "source/common/common/thread.h" +#include "absl/strings/str_split.h" + namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -84,7 +86,8 @@ FileServerConfig::applyPathMapping(absl::string_view path_with_query, std::filesystem::path{mapping.file_path_prefix()} / std::filesystem::path{kept_path}; if (file_path != file_path.lexically_normal() || !file_path.string().starts_with(mapping.file_path_prefix())) { - // Ensure we're not accidentally looking outside the designated filesystem prefix. + // Ensure we're not accidentally looking outside the designated filesystem prefix + // in any way controlled by the client. (Symlink escapes are up to the filesystem owner.) return absl::nullopt; } return file_path; diff --git a/source/extensions/filters/http/file_server/filter_config.h b/source/extensions/filters/http/file_server/filter_config.h index 68c112a201a50..212f74e6f2f77 100644 --- a/source/extensions/filters/http/file_server/filter_config.h +++ b/source/extensions/filters/http/file_server/filter_config.h @@ -5,6 +5,7 @@ #include #include "envoy/extensions/filters/http/file_server/v3/file_server.pb.h" +#include "envoy/router/router.h" #include "envoy/server/factory_context.h" #include "source/common/common/radix_tree.h" diff --git a/test/extensions/filters/http/file_server/config_test.cc b/test/extensions/filters/http/file_server/config_test.cc index efd0f9a9a1ae8..d96076cd1db21 100644 --- a/test/extensions/filters/http/file_server/config_test.cc +++ b/test/extensions/filters/http/file_server/config_test.cc @@ -63,8 +63,7 @@ class FileServerConfigTest : public testing::Test { .value(); Http::MockFilterChainFactoryCallbacks filter_callback; std::shared_ptr config; - EXPECT_CALL(filter_callback, addStreamDecoderFilter(_)) - .WillOnce(Invoke(captureConfig(&config))); + EXPECT_CALL(filter_callback, addStreamDecoderFilter(_)).WillOnce(captureConfig(&config)); cb(filter_callback); return config; } diff --git a/test/extensions/filters/http/file_server/filter_test.cc b/test/extensions/filters/http/file_server/filter_test.cc index e6b304d57ddb1..bb28f0994b475 100644 --- a/test/extensions/filters/http/file_server/filter_test.cc +++ b/test/extensions/filters/http/file_server/filter_test.cc @@ -85,6 +85,29 @@ default_content_type: "application/octet-stream" Event::DispatcherPtr dispatcher_ = api_->allocateDispatcher("test_thread"); }; +TEST_F(FileServerFilterTest, InternalServerErrorIfNoFileManagerConfigured) { + std::string yaml(R"( +path_mappings: + - request_path_prefix: /path1 + file_path_prefix: fs1 +)"); + ProtoFileServerConfig proto_config; + TestUtility::loadFromYaml(yaml, proto_config); + // Create with nullptr for AsyncFileManager. + auto filter = FileServerFilter::createShared( + std::make_shared(proto_config, nullptr, nullptr)); + initFilter(*filter); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/banana.html"}, + {":host", "test.host"}, + {":method", "GET"}, + {":scheme", "https"}, + }; + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::InternalServerError, _, _, _, + "file_server_no_file_manager_configured")); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); +} + TEST_F(FileServerFilterTest, PassThroughIfNoPath) { auto filter = testFilter(); Http::TestRequestHeaderMapImpl request_headers{ diff --git a/test/extensions/filters/http/file_server/integration_test.cc b/test/extensions/filters/http/file_server/integration_test.cc index 204d0eee87be2..50b21046488e5 100644 --- a/test/extensions/filters/http/file_server/integration_test.cc +++ b/test/extensions/filters/http/file_server/integration_test.cc @@ -1,4 +1,5 @@ #include +#include #include "test/integration/http_protocol_integration.h" #include "test/test_common/utility.h" From fa7ea2a663bb7d6d8577c77eff3fc4a882494999 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Tue, 10 Feb 2026 20:02:54 +0000 Subject: [PATCH 16/22] Hide the unused message type from docgen Signed-off-by: Raven Black --- .../extensions/filters/http/file_server/v3/file_server.proto | 1 + 1 file changed, 1 insertion(+) diff --git a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto index 27165e38b0b8e..f1439a3e639d0 100644 --- a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto +++ b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto @@ -39,6 +39,7 @@ message FileServerConfig { } message DirectoryBehavior { + // [#not-implemented-hide:] Directory operations currently have no async implementation. message List { } From f80dfe86048f6e01b6404a82a1af9e110ed807dc Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 23 Feb 2026 17:01:48 +0000 Subject: [PATCH 17/22] Address comments Signed-off-by: Raven Black --- .../filters/http/file_server/config.cc | 9 ++++---- .../filters/http/file_server/file_streamer.h | 2 +- .../filters/http/file_server/filter.cc | 10 +++++++-- .../filters/http/file_server/filter.h | 8 ++----- .../filters/http/file_server/filter_test.cc | 22 +++++++++++++++---- 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/source/extensions/filters/http/file_server/config.cc b/source/extensions/filters/http/file_server/config.cc index 550afa273b724..50183c988f4e5 100644 --- a/source/extensions/filters/http/file_server/config.cc +++ b/source/extensions/filters/http/file_server/config.cc @@ -20,15 +20,14 @@ namespace FileServer { namespace { absl::Status validateProto(const ProtoFileServerConfig& config) { absl::flat_hash_set seen; - bool inserted; for (const auto& mapping : config.path_mappings()) { - std::tie(std::ignore, inserted) = seen.emplace(mapping.request_path_prefix()); + auto [_, inserted] = seen.emplace(mapping.request_path_prefix()); if (!inserted) { return absl::InvalidArgumentError( absl::StrCat("duplicate request_path_prefix: ", mapping.request_path_prefix())); } } - seen.erase(seen.begin(), seen.end()); + seen.clear(); bool directory_tried = false; static const absl::string_view directory_options = "default_file or list"; for (const auto& directory_behavior : config.directory_behaviors()) { @@ -41,7 +40,7 @@ absl::Status validateProto(const ProtoFileServerConfig& config) { absl::StrCat("directory_behavior must have only one of ", directory_options)); } if (!directory_behavior.default_file().empty()) { - std::tie(std::ignore, inserted) = seen.emplace(directory_behavior.default_file()); + auto [_, inserted] = seen.emplace(directory_behavior.default_file()); if (!inserted) { return absl::InvalidArgumentError(absl::StrCat( "duplicate default_file in directory_behaviors: ", directory_behavior.default_file())); @@ -76,7 +75,7 @@ absl::StatusOr FileServerFilterFactory::createFilterFacto } return [fsc = std::move(file_server_config.value())]( Http::FilterChainFactoryCallbacks& callbacks) -> void { - callbacks.addStreamDecoderFilter(FileServerFilter::createShared(fsc)); + callbacks.addStreamDecoderFilter(std::make_unique(fsc)); }; } diff --git a/source/extensions/filters/http/file_server/file_streamer.h b/source/extensions/filters/http/file_server/file_streamer.h index d937aee77164b..d0f47e28cb250 100644 --- a/source/extensions/filters/http/file_server/file_streamer.h +++ b/source/extensions/filters/http/file_server/file_streamer.h @@ -53,7 +53,7 @@ class FileStreamer { Event::Dispatcher* dispatcher_; FileStreamerClient& client_; std::filesystem::path file_path_; - uint64_t pos_; + uint64_t pos_ = 0; // If zero, fetches entire file. // To get the last byte, end_ must be the size of the file, not the inclusive last byte // like a range request uses. diff --git a/source/extensions/filters/http/file_server/filter.cc b/source/extensions/filters/http/file_server/filter.cc index a06e2e1557ef8..df88fc92b1eda 100644 --- a/source/extensions/filters/http/file_server/filter.cc +++ b/source/extensions/filters/http/file_server/filter.cc @@ -70,7 +70,7 @@ void FileServerFilter::onAboveWriteBufferHighWatermark() { file_streamer_.pause( void FileServerFilter::onBelowWriteBufferLowWatermark() { file_streamer_.unpause(); } Http::FilterHeadersStatus FileServerFilter::decodeHeaders(RequestHeaderMap& headers, - bool end_stream ABSL_ATTRIBUTE_UNUSED) { + bool end_stream) { if (!decoder_callbacks_->route() || !headers.Path()) { return Http::FilterHeadersStatus::Continue; } @@ -79,7 +79,7 @@ Http::FilterHeadersStatus FileServerFilter::decodeHeaders(RequestHeaderMap& head if (!config) { config = file_server_config_.get(); } - std::string path = PercentEncoding::decode(headers.Path()->value().getStringView()); + const std::string path = PercentEncoding::decode(headers.Path()->value().getStringView()); std::shared_ptr mapping = config->pathMapping(path); if (!mapping) { // If the request didn't match a mapping, skip this filter. @@ -105,6 +105,12 @@ Http::FilterHeadersStatus FileServerFilter::decodeHeaders(RequestHeaderMap& head absl::nullopt, "file_server_rejected_method"); return Http::FilterHeadersStatus::StopIteration; } + if (!end_stream) { + decoder_callbacks_->sendLocalReply(Http::Code::BadRequest, + CodeUtility::toString(Http::Code::BadRequest), nullptr, + absl::nullopt, "file_server_rejected_not_end_stream"); + return Http::FilterHeadersStatus::StopIteration; + } if (config->asyncFileManager() == nullptr) { decoder_callbacks_->sendLocalReply( Http::Code::InternalServerError, CodeUtility::toString(Http::Code::InternalServerError), diff --git a/source/extensions/filters/http/file_server/filter.h b/source/extensions/filters/http/file_server/filter.h index 2ea680dab914f..f4108ca984f7a 100644 --- a/source/extensions/filters/http/file_server/filter.h +++ b/source/extensions/filters/http/file_server/filter.h @@ -22,10 +22,8 @@ class FileServerFilter : public Http::PassThroughDecoderFilter, public FileStreamerClient, public Http::DownstreamWatermarkCallbacks { public: - static std::shared_ptr - createShared(std::shared_ptr file_server_config) { - return std::shared_ptr(new FileServerFilter(std::move(file_server_config))); - } + explicit FileServerFilter(std::shared_ptr file_server_config) + : file_server_config_(file_server_config), file_streamer_(*this) {} static const std::string& filterName(); @@ -42,8 +40,6 @@ class FileServerFilter : public Http::PassThroughDecoderFilter, void onBelowWriteBufferLowWatermark() override; private: - explicit FileServerFilter(std::shared_ptr file_server_config) - : file_server_config_(file_server_config), file_streamer_(*this) {} std::shared_ptr file_server_config_; friend class FileServerConfigTest; // Allow test access to file_server_config_. FileStreamer file_streamer_; diff --git a/test/extensions/filters/http/file_server/filter_test.cc b/test/extensions/filters/http/file_server/filter_test.cc index bb28f0994b475..ff5209715f728 100644 --- a/test/extensions/filters/http/file_server/filter_test.cc +++ b/test/extensions/filters/http/file_server/filter_test.cc @@ -48,7 +48,7 @@ class FileServerFilterTest : public testing::Test { .WillRepeatedly(ReturnRef(*dispatcher_)); } std::shared_ptr testFilter() { - auto filter = FileServerFilter::createShared(configFromYaml(R"( + auto filter = std::make_shared(configFromYaml(R"( path_mappings: - request_path_prefix: /path1 file_path_prefix: fs1 @@ -94,7 +94,7 @@ TEST_F(FileServerFilterTest, InternalServerErrorIfNoFileManagerConfigured) { ProtoFileServerConfig proto_config; TestUtility::loadFromYaml(yaml, proto_config); // Create with nullptr for AsyncFileManager. - auto filter = FileServerFilter::createShared( + auto filter = std::make_shared( std::make_shared(proto_config, nullptr, nullptr)); initFilter(*filter); Http::TestRequestHeaderMapImpl request_headers{ @@ -188,6 +188,20 @@ TEST_F(FileServerFilterTest, MethodNotAllowedIfMatchedPathAndUnsupportedMethod) EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); } +TEST_F(FileServerFilterTest, BadRequestIfHeadersDoNotEndStream) { + auto filter = testFilter(); + Http::TestRequestHeaderMapImpl request_headers{ + {":path", "/path1/foo"}, + {":method", "GET"}, + {":host", "test.host"}, + {":scheme", "https"}, + }; + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::BadRequest, _, _, _, + "file_server_rejected_not_end_stream")); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter->decodeHeaders(request_headers, false)); +} + TEST_F(FileServerFilterTest, FileStatFailedNotFoundRespondsNotFound) { auto filter = testFilter(); Http::TestRequestHeaderMapImpl request_headers{ @@ -223,7 +237,7 @@ TEST_F(FileServerFilterTest, FilterOnDestroyWhileFileActionInFlightAbortsRespons } TEST_F(FileServerFilterTest, ErrorsOnDirectoryWithNoConfiguredBehavior) { - auto filter = FileServerFilter::createShared(configFromYaml(R"( + auto filter = std::make_shared(configFromYaml(R"( path_mappings: - request_path_prefix: /path1 file_path_prefix: fs1 @@ -249,7 +263,7 @@ TEST_F(FileServerFilterTest, ErrorsOnDirectoryWithNoConfiguredBehavior) { } TEST_F(FileServerFilterTest, ErrorsOnDirectoryWithImpossiblyConfiguredBehaviorForCoverage) { - auto filter = FileServerFilter::createShared(configFromYaml(R"( + auto filter = std::make_shared(configFromYaml(R"( path_mappings: - request_path_prefix: /path1 file_path_prefix: fs1 From d2ef56cdcb6b120e76a34ffabf958b8c8f14915c Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 23 Feb 2026 17:10:32 +0000 Subject: [PATCH 18/22] Add wbpcode as CODEOWNER Signed-off-by: Raven Black --- CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index 8b6fab815756e..dac191bd3a99a 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -264,7 +264,7 @@ extensions/upstreams/tcp @ggreenway @mattklein123 /*/extensions/config/validators/minimum_clusters @adisuissa @yanavlasov # File system based extensions /*/extensions/common/async_files @mattklein123 @ravenblackx -/*/extensions/filters/http/file_server @ravenblackx @phlax +/*/extensions/filters/http/file_server @ravenblackx @wbpcode @phlax /*/extensions/filters/http/file_system_buffer @mattklein123 @ravenblackx /*/extensions/http/cache/file_system_http_cache @ggreenway @UNOWNED /*/extensions/http/cache_v2/file_system_http_cache @ggreenway @ravenblackx From 5cb575bc5720d1c1e41236894abf609b7e61b502 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 23 Feb 2026 17:24:43 +0000 Subject: [PATCH 19/22] Make manager_config mandatory Signed-off-by: Raven Black --- .../http/file_server/v3/file_server.proto | 6 ++-- .../filters/http/file_server/filter.cc | 6 ---- .../filters/http/file_server/filter_config.cc | 22 ++++++--------- .../filters/http/file_server/config_test.cc | 28 +++++++++---------- .../filters/http/file_server/filter_test.cc | 23 --------------- 5 files changed, 25 insertions(+), 60 deletions(-) diff --git a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto index f1439a3e639d0..c14adfa6b41e7 100644 --- a/api/envoy/extensions/filters/http/file_server/v3/file_server.proto +++ b/api/envoy/extensions/filters/http/file_server/v3/file_server.proto @@ -54,10 +54,8 @@ message FileServerConfig { } // A configuration for the AsyncFileManager to be used to read from the filesystem. - // - // This may be unset to allow for per-route-only configs. If this is unset when - // a ``path_mapping`` is matched, a 500 Internal Server Error response is served. - common.async_files.v3.AsyncFileManagerConfig manager_config = 1; + common.async_files.v3.AsyncFileManagerConfig manager_config = 1 + [(validate.rules).message = {required: true}]; // The longest matching path_mapping takes precedence. repeated PathMapping path_mappings = 2; diff --git a/source/extensions/filters/http/file_server/filter.cc b/source/extensions/filters/http/file_server/filter.cc index df88fc92b1eda..c314dcf058fb3 100644 --- a/source/extensions/filters/http/file_server/filter.cc +++ b/source/extensions/filters/http/file_server/filter.cc @@ -111,12 +111,6 @@ Http::FilterHeadersStatus FileServerFilter::decodeHeaders(RequestHeaderMap& head absl::nullopt, "file_server_rejected_not_end_stream"); return Http::FilterHeadersStatus::StopIteration; } - if (config->asyncFileManager() == nullptr) { - decoder_callbacks_->sendLocalReply( - Http::Code::InternalServerError, CodeUtility::toString(Http::Code::InternalServerError), - nullptr, absl::nullopt, "file_server_no_file_manager_configured"); - return Http::FilterHeadersStatus::StopIteration; - } // Parse range header, if present, into start and end (otherwise or on error, 0,0) auto [start, end] = parseRangeHeader(headers); is_head_ = headers.Method()->value() == Http::Headers::get().MethodValues.Head; diff --git a/source/extensions/filters/http/file_server/filter_config.cc b/source/extensions/filters/http/file_server/filter_config.cc index 64a0cc13ea882..02a1f02b5dc34 100644 --- a/source/extensions/filters/http/file_server/filter_config.cc +++ b/source/extensions/filters/http/file_server/filter_config.cc @@ -31,20 +31,16 @@ absl::StatusOr> FileServerConfig::create(const ProtoFileServerConfig& config, Envoy::Server::Configuration::ServerFactoryContext& context) { auto factory = AsyncFileManagerFactory::singleton(&context.singletonManager()); - if (config.manager_config().manager_type_case() != - envoy::extensions::common::async_files::v3::AsyncFileManagerConfig::MANAGER_TYPE_NOT_SET) { - TRY_ASSERT_MAIN_THREAD { - // TODO(ravenblack): make getAsyncFileManager use StatusOr instead of throw. - auto async_file_manager = factory->getAsyncFileManager(config.manager_config()); - return std::make_shared(config, std::move(factory), - std::move(async_file_manager)); - } - END_TRY - catch (const EnvoyException& e) { - return absl::InvalidArgumentError(e.what()); - } + TRY_ASSERT_MAIN_THREAD { + // TODO(ravenblack): make getAsyncFileManager use StatusOr instead of throw. + auto async_file_manager = factory->getAsyncFileManager(config.manager_config()); + return std::make_shared(config, std::move(factory), + std::move(async_file_manager)); + } + END_TRY + catch (const EnvoyException& e) { + return absl::InvalidArgumentError(e.what()); } - return std::make_shared(config, nullptr, nullptr); } FileServerConfig::FileServerConfig(const ProtoFileServerConfig& config, diff --git a/test/extensions/filters/http/file_server/config_test.cc b/test/extensions/filters/http/file_server/config_test.cc index d96076cd1db21..e19bcb9b9c4f5 100644 --- a/test/extensions/filters/http/file_server/config_test.cc +++ b/test/extensions/filters/http/file_server/config_test.cc @@ -82,6 +82,8 @@ class FileServerConfigTest : public testing::Test { TEST_F(FileServerConfigTest, EmptyDirectoryBehaviorIsConfigError) { auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +manager_config: + thread_pool: {} directory_behaviors: - {} )"), @@ -92,6 +94,8 @@ TEST_F(FileServerConfigTest, EmptyDirectoryBehaviorIsConfigError) { TEST_F(FileServerConfigTest, OverpopulatedDirectoryBehaviorIsConfigError) { auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +manager_config: + thread_pool: {} directory_behaviors: - default_file: "index.html" list: {} @@ -103,6 +107,8 @@ TEST_F(FileServerConfigTest, OverpopulatedDirectoryBehaviorIsConfigError) { TEST_F(FileServerConfigTest, DuplicateDirectoryFilesIsConfigError) { auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +manager_config: + thread_pool: {} directory_behaviors: - default_file: "index.html" - default_file: "index.html" @@ -113,6 +119,8 @@ TEST_F(FileServerConfigTest, DuplicateDirectoryFilesIsConfigError) { TEST_F(FileServerConfigTest, DuplicateDirectoryListIsConfigError) { auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +manager_config: + thread_pool: {} directory_behaviors: - list: {} - list: {} @@ -124,6 +132,8 @@ TEST_F(FileServerConfigTest, DuplicateDirectoryListIsConfigError) { TEST_F(FileServerConfigTest, DuplicateRequestPathPrefixIsConfigError) { auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +manager_config: + thread_pool: {} path_mappings: - request_path_prefix: "/banana" file_path_prefix: "/banana" @@ -136,6 +146,8 @@ TEST_F(FileServerConfigTest, DuplicateRequestPathPrefixIsConfigError) { TEST_F(FileServerConfigTest, SuffixForContentTypeContainingPeriodIsError) { auto status_or = factory()->createFilterFactoryFromProto(configFromYaml(R"( +manager_config: + thread_pool: {} content_types: "txt": "text/plain" ".html": "text/html" @@ -144,13 +156,6 @@ TEST_F(FileServerConfigTest, SuffixForContentTypeContainingPeriodIsError) { EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr(".html"))); } -TEST_F(FileServerConfigTest, EmptyConfigIsOk) { - std::shared_ptr config = captureConfigFromProto(emptyConfig()); - EXPECT_THAT(config->contentTypeForPath("xxx.txt"), Eq("")); - EXPECT_THAT(config->asyncFileManager(), IsNull()); - EXPECT_THAT(config->directoryBehavior(0), Eq(absl::nullopt)); -} - TEST_F(FileServerConfigTest, ValidConfigPopulatesConfigObjectAppropriately) { std::shared_ptr config = captureConfigFromProto(configFromYaml(R"( manager_config: @@ -212,6 +217,8 @@ default_content_type: "application/octet-stream" TEST_F(FileServerConfigTest, DuplicateDirectoryFilesIsConfigErrorInRouteConfig) { auto status_or = factory()->createRouteSpecificFilterConfig( configFromYaml(R"( +manager_config: + thread_pool: {} directory_behaviors: - default_file: "index.html" - default_file: "index.html" @@ -220,13 +227,6 @@ TEST_F(FileServerConfigTest, DuplicateDirectoryFilesIsConfigErrorInRouteConfig) EXPECT_THAT(status_or, HasStatus(absl::StatusCode::kInvalidArgument, HasSubstr("index.html"))); } -TEST_F(FileServerConfigTest, EmptyConfigIsOkInRouteConfig) { - std::shared_ptr config = makeRouteConfig(emptyConfig()); - EXPECT_THAT(config->contentTypeForPath("xxx.txt"), Eq("")); - EXPECT_THAT(config->asyncFileManager(), IsNull()); - EXPECT_THAT(config->directoryBehavior(0), Eq(absl::nullopt)); -} - TEST_F(FileServerConfigTest, InvalidFileManagerConfigFailsInRouteConfig) { auto mismatched_config = factory()->createRouteSpecificFilterConfig( configFromYaml(R"( diff --git a/test/extensions/filters/http/file_server/filter_test.cc b/test/extensions/filters/http/file_server/filter_test.cc index ff5209715f728..f2f79d2b30920 100644 --- a/test/extensions/filters/http/file_server/filter_test.cc +++ b/test/extensions/filters/http/file_server/filter_test.cc @@ -85,29 +85,6 @@ default_content_type: "application/octet-stream" Event::DispatcherPtr dispatcher_ = api_->allocateDispatcher("test_thread"); }; -TEST_F(FileServerFilterTest, InternalServerErrorIfNoFileManagerConfigured) { - std::string yaml(R"( -path_mappings: - - request_path_prefix: /path1 - file_path_prefix: fs1 -)"); - ProtoFileServerConfig proto_config; - TestUtility::loadFromYaml(yaml, proto_config); - // Create with nullptr for AsyncFileManager. - auto filter = std::make_shared( - std::make_shared(proto_config, nullptr, nullptr)); - initFilter(*filter); - Http::TestRequestHeaderMapImpl request_headers{ - {":path", "/path1/banana.html"}, - {":host", "test.host"}, - {":method", "GET"}, - {":scheme", "https"}, - }; - EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::InternalServerError, _, _, _, - "file_server_no_file_manager_configured")); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter->decodeHeaders(request_headers, true)); -} - TEST_F(FileServerFilterTest, PassThroughIfNoPath) { auto filter = testFilter(); Http::TestRequestHeaderMapImpl request_headers{ From 66e9e626fdf400a51a92e80205809c383dbad86d Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 23 Feb 2026 20:48:33 +0000 Subject: [PATCH 20/22] Fuzz tester limitation Signed-off-by: Raven Black --- test/extensions/filters/http/common/fuzz/BUILD | 1 + .../filters/http/common/fuzz/uber_per_filter.cc | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/test/extensions/filters/http/common/fuzz/BUILD b/test/extensions/filters/http/common/fuzz/BUILD index a3cce0ee3b7e9..71e3feec10c5a 100644 --- a/test/extensions/filters/http/common/fuzz/BUILD +++ b/test/extensions/filters/http/common/fuzz/BUILD @@ -57,6 +57,7 @@ envoy_cc_test_library( "//test/proto:bookstore_proto_cc_proto", "//test/test_common:registry_lib", "//test/test_common:test_runtime_lib", + "@envoy_api//envoy/extensions/filters/http/file_server/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/http/file_system_buffer/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/http/grpc_json_reverse_transcoder/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/http/grpc_json_transcoder/v3:pkg_cc_proto", diff --git a/test/extensions/filters/http/common/fuzz/uber_per_filter.cc b/test/extensions/filters/http/common/fuzz/uber_per_filter.cc index a92706be435ce..829308e424645 100644 --- a/test/extensions/filters/http/common/fuzz/uber_per_filter.cc +++ b/test/extensions/filters/http/common/fuzz/uber_per_filter.cc @@ -1,3 +1,4 @@ +#include "envoy/extensions/filters/http/file_server/v3/file_server.pb.h" #include "envoy/extensions/filters/http/file_system_buffer/v3/file_system_buffer.pb.h" #include "envoy/extensions/filters/http/grpc_json_reverse_transcoder/v3/transcoder.pb.h" #include "envoy/extensions/filters/http/grpc_json_transcoder/v3/transcoder.pb.h" @@ -139,6 +140,18 @@ void cleanFileSystemBufferConfig(Protobuf::Message* message) { } } +void cleanFileServerConfig(Protobuf::Message* message) { + envoy::extensions::filters::http::file_server::v3::FileServerConfig& config = + *Envoy::Protobuf::DynamicCastMessage< + envoy::extensions::filters::http::file_server::v3::FileServerConfig>(message); + if (config.manager_config().thread_pool().thread_count() > kMaxAsyncFileManagerThreadCount) { + throw EnvoyException(fmt::format( + "received input exceeding the allowed number of threads ({} > {}) for " + "FileServerConfig.AsyncFileManager", + config.manager_config().thread_pool().thread_count(), kMaxAsyncFileManagerThreadCount)); + } +} + void UberFilterFuzzer::cleanFuzzedConfig(absl::string_view filter_name, Protobuf::Message* message) { // Map filter name to clean-up function. @@ -154,6 +167,9 @@ void UberFilterFuzzer::cleanFuzzedConfig(absl::string_view filter_name, } else if (filter_name == "envoy.filters.http.file_system_buffer") { // Limit the number of threads to create to kMaxAsyncFileManagerThreadCount cleanFileSystemBufferConfig(message); + } else if (filter_name == "envoy.filters.http.file_server") { + // Limit the number of threads to create to kMaxAsyncFileManagerThreadCount + cleanFileServerConfig(message); } } From 834c57af71ff8c75ce24db0af63513bb4757a390 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 23 Feb 2026 21:24:06 +0000 Subject: [PATCH 21/22] Ensure config destruction happens in the same thread Signed-off-by: Raven Black --- source/extensions/filters/http/file_server/filter.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/http/file_server/filter.cc b/source/extensions/filters/http/file_server/filter.cc index c314dcf058fb3..a2174e322d90f 100644 --- a/source/extensions/filters/http/file_server/filter.cc +++ b/source/extensions/filters/http/file_server/filter.cc @@ -140,7 +140,10 @@ void FileServerFilter::errorFromFile(Http::Code code, absl::string_view log_mess } } -void FileServerFilter::onDestroy() { file_streamer_.abort(); } +void FileServerFilter::onDestroy() { + file_streamer_.abort(); + file_server_config_.clear(); +} } // namespace FileServer } // namespace HttpFilters From eed266238044095add00afa93c40985b66e48a46 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 23 Feb 2026 21:39:07 +0000 Subject: [PATCH 22/22] The right delete! Signed-off-by: Raven Black --- source/extensions/filters/http/file_server/filter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/http/file_server/filter.cc b/source/extensions/filters/http/file_server/filter.cc index a2174e322d90f..0350aae4e204d 100644 --- a/source/extensions/filters/http/file_server/filter.cc +++ b/source/extensions/filters/http/file_server/filter.cc @@ -142,7 +142,7 @@ void FileServerFilter::errorFromFile(Http::Code code, absl::string_view log_mess void FileServerFilter::onDestroy() { file_streamer_.abort(); - file_server_config_.clear(); + file_server_config_.reset(); } } // namespace FileServer