From 44762f64ce3252b7a151915d9abfa44d95b7a008 Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Tue, 5 Nov 2019 22:22:24 +0100 Subject: [PATCH 01/15] Added api proposition Signed-off-by: Lukasz Dziedziak --- .../filter/accesslog/v2/accesslog.proto | 2 + .../filter/accesslog/v3alpha/accesslog.proto | 2 + .../v2/http_connection_manager.proto | 33 +++++++++++++- .../v3alpha/http_connection_manager.proto | 33 +++++++++++++- api/envoy/type/format.proto | 43 +++++++++++++++++++ api/envoy/type/v3alpha/format.proto | 43 +++++++++++++++++++ docs/root/api-v2/types/types.rst | 1 + .../observability/access_log.rst | 2 + tools/spelling_dictionary.txt | 1 + 9 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 api/envoy/type/format.proto create mode 100644 api/envoy/type/v3alpha/format.proto diff --git a/api/envoy/config/filter/accesslog/v2/accesslog.proto b/api/envoy/config/filter/accesslog/v2/accesslog.proto index b336fb0b1673f..16a51a23eb131 100644 --- a/api/envoy/config/filter/accesslog/v2/accesslog.proto +++ b/api/envoy/config/filter/accesslog/v2/accesslog.proto @@ -45,6 +45,8 @@ message AccessLog { } } +// [#next-major-version: In the v3 API, we should consider renaming +// it to more generic filter] // [#next-free-field: 12] message AccessLogFilter { oneof filter_specifier { diff --git a/api/envoy/config/filter/accesslog/v3alpha/accesslog.proto b/api/envoy/config/filter/accesslog/v3alpha/accesslog.proto index 6c21668b593ac..e83377399db50 100644 --- a/api/envoy/config/filter/accesslog/v3alpha/accesslog.proto +++ b/api/envoy/config/filter/accesslog/v3alpha/accesslog.proto @@ -47,6 +47,8 @@ message AccessLog { } } +// [#next-major-version: In the v3 API, we should consider renaming +// it to more generic filter] // [#next-free-field: 12] message AccessLogFilter { oneof filter_specifier { diff --git a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto index 19f3fe3dd37d7..d1123f3dec940 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto @@ -11,6 +11,7 @@ import "envoy/api/v2/core/protocol.proto"; import "envoy/api/v2/rds.proto"; import "envoy/api/v2/srds.proto"; import "envoy/config/filter/accesslog/v2/accesslog.proto"; +import "envoy/type/format.proto"; import "envoy/type/percent.proto"; import "google/protobuf/any.proto"; @@ -23,7 +24,7 @@ import "validate/validate.proto"; // [#protodoc-title: HTTP connection manager] // HTTP connection manager :ref:`configuration overview `. -// [#next-free-field: 36] +// [#next-free-field: 37] message HttpConnectionManager { enum CodecType { // For every new connection, the connection manager will determine which @@ -467,6 +468,36 @@ message HttpConnectionManager { // with `prefix` match set to `/dir`. Defaults to `false`. Note that slash merging is not part of // `HTTP spec ` and is provided for convenience. bool merge_slashes = 33; + + // [#not-implemented-hide:] + // Configuration of local reply returned by Envoy. Allows to specify mappings and format of + // response. + LocalReplyConfig local_reply_config = 36; +} + +message LocalReplyConfig { + // Configuration of list of mappers which allows to filter and change local response. + // The client will iterate through mappers until first match. + repeated ResponseMapper mapper = 1; + + // Allows to define custom format of local reply. + // It is allowed to use :ref:`command operators ` + // which will be resolved to actual data. + type.StringOrJson format = 2; +} + +message ResponseMapper { + // Filter is used to determine if the response should be changed. + accesslog.v2.AccessLogFilter filter = 1 [(validate.rules).message = {required: true}]; + + // Rewriter defines which values in local reply should be changed. + ResponseRewriter rewriter = 2 [(validate.rules).message = {required: true}]; +} + +// Configuration of new value for matched local response. +message ResponseRewriter { + // Status code for matched response. + google.protobuf.UInt32Value status_code = 1; } message Rds { diff --git a/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto index f96b590d7130a..4c7543eec7cba 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto @@ -11,6 +11,7 @@ import "envoy/api/v3alpha/core/protocol.proto"; import "envoy/api/v3alpha/rds.proto"; import "envoy/api/v3alpha/srds.proto"; import "envoy/config/filter/accesslog/v3alpha/accesslog.proto"; +import "envoy/type/v3alpha/format.proto"; import "envoy/type/v3alpha/percent.proto"; import "google/protobuf/any.proto"; @@ -23,7 +24,7 @@ import "validate/validate.proto"; // [#protodoc-title: HTTP connection manager] // HTTP connection manager :ref:`configuration overview `. -// [#next-free-field: 36] +// [#next-free-field: 37] message HttpConnectionManager { enum CodecType { // For every new connection, the connection manager will determine which @@ -454,6 +455,36 @@ message HttpConnectionManager { // with `prefix` match set to `/dir`. Defaults to `false`. Note that slash merging is not part of // `HTTP spec ` and is provided for convenience. bool merge_slashes = 33; + + // [#not-implemented-hide:] + // Configuration of local reply returned by Envoy. Allows to specify mappings and format of + // response. + LocalReplyConfig local_reply_config = 36; +} + +message LocalReplyConfig { + // Configuration of list of mappers which allows to filter and change local response. + // The client will iterate through mappers until first match. + repeated ResponseMapper mapper = 1; + + // Allows to define custom format of local reply. + // It is allowed to use :ref:`command operators ` + // which will be resolved to actual data. + type.v3alpha.StringOrJson format = 2; +} + +message ResponseMapper { + // Filter is used to determine if the response should be changed. + accesslog.v3alpha.AccessLogFilter filter = 1 [(validate.rules).message = {required: true}]; + + // Rewriter defines which values in local reply should be changed. + ResponseRewriter rewriter = 2 [(validate.rules).message = {required: true}]; +} + +// Configuration of new value for matched local response. +message ResponseRewriter { + // Status code for matched response. + google.protobuf.UInt32Value status_code = 1; } message Rds { diff --git a/api/envoy/type/format.proto b/api/envoy/type/format.proto new file mode 100644 index 0000000000000..4918b18816f13 --- /dev/null +++ b/api/envoy/type/format.proto @@ -0,0 +1,43 @@ +syntax = "proto3"; + +package envoy.type; + +option java_outer_classname = "FormatProto"; +option java_multiple_files = true; +option java_package = "io.envoyproxy.envoy.type"; + +import "google/protobuf/struct.proto"; + +// [#protodoc-title: Format] + +// Allows to define one of format: flat plain text or structured json. +message StringOrJson { + oneof format { + // Allows to specify flat plain text format. + // + // .. code-block:: yaml + // + // string_format: "My custom message" + // + string string_format = 1; + + // Allows to specify structured data as json. + // + // .. code-block:: yaml + // + // json_format: + // protocol: "HTTP/1.1" + // message: "My message" + // + // The following JSON object would be created: + // + // .. code-block:: json + // + // { + // "protocol": "HTTP/1.1", + // "message": "My message" + // } + // + google.protobuf.Struct json_format = 2; + } +} diff --git a/api/envoy/type/v3alpha/format.proto b/api/envoy/type/v3alpha/format.proto new file mode 100644 index 0000000000000..cc0ee55745611 --- /dev/null +++ b/api/envoy/type/v3alpha/format.proto @@ -0,0 +1,43 @@ +syntax = "proto3"; + +package envoy.type.v3alpha; + +option java_outer_classname = "FormatProto"; +option java_multiple_files = true; +option java_package = "io.envoyproxy.envoy.type.v3alpha"; + +import "google/protobuf/struct.proto"; + +// [#protodoc-title: Format] + +// Allows to define one of format: flat plain text or structured json. +message StringOrJson { + oneof format { + // Allows to specify flat plain text format. + // + // .. code-block:: yaml + // + // string_format: "My custom message" + // + string string_format = 1; + + // Allows to specify structured data as json. + // + // .. code-block:: yaml + // + // json_format: + // protocol: "HTTP/1.1" + // message: "My message" + // + // The following JSON object would be created: + // + // .. code-block:: json + // + // { + // "protocol": "HTTP/1.1", + // "message": "My message" + // } + // + google.protobuf.Struct json_format = 2; + } +} diff --git a/docs/root/api-v2/types/types.rst b/docs/root/api-v2/types/types.rst index 4d6c8ef2edd6a..a78cf589c6912 100644 --- a/docs/root/api-v2/types/types.rst +++ b/docs/root/api-v2/types/types.rst @@ -5,6 +5,7 @@ Types :glob: :maxdepth: 2 + ../type/format.proto ../type/hash_policy.proto ../type/http.proto ../type/http_status.proto diff --git a/docs/root/configuration/observability/access_log.rst b/docs/root/configuration/observability/access_log.rst index 9639ce3b6af11..eb2fb3cdd28cc 100644 --- a/docs/root/configuration/observability/access_log.rst +++ b/docs/root/configuration/observability/access_log.rst @@ -91,6 +91,8 @@ Format dictionaries have the following restrictions: * The dictionary must map strings to strings (specifically, strings to command operators). Nesting is not currently supported. +.. _config_access_log_command_operators: + Command Operators ----------------- diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index 76a9f05aadc17..a343d5ac5d39d 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -873,6 +873,7 @@ resync ret retriable retriggers +rewriter rollout roundtrip rpcs From 2d14f71adff7199c6316079c044286bc2525e194 Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Tue, 5 Nov 2019 22:22:58 +0100 Subject: [PATCH 02/15] Basic implementation with current test working Signed-off-by: Lukasz Dziedziak --- include/envoy/access_log/access_log.h | 6 +- .../common/access_log/access_log_formatter.cc | 50 ++- .../common/access_log/access_log_formatter.h | 48 ++- source/common/http/BUILD | 1 + source/common/http/conn_manager_config.h | 6 + source/common/http/conn_manager_impl.cc | 29 ++ source/common/http/utility.cc | 34 +- source/common/http/utility.h | 26 ++ source/common/local_reply/BUILD | 19 ++ source/common/local_reply/local_reply.cc | 68 ++++ source/common/local_reply/local_reply.h | 116 +++++++ source/common/router/header_formatter.cc | 4 +- .../file/file_access_log_impl.cc | 4 +- .../network/http_connection_manager/BUILD | 3 + .../network/http_connection_manager/config.cc | 74 ++++ .../network/http_connection_manager/config.h | 3 + source/server/http/admin.h | 1 + .../access_log_formatter_fuzz_test.cc | 7 +- .../access_log_formatter_speed_test.cc | 3 +- .../access_log/access_log_formatter_test.cc | 315 ++++++++++-------- .../http/conn_manager_impl_fuzz_test.cc | 5 + test/common/http/conn_manager_impl_test.cc | 7 + test/common/http/conn_manager_utility_test.cc | 7 + 23 files changed, 660 insertions(+), 176 deletions(-) create mode 100644 source/common/local_reply/BUILD create mode 100644 source/common/local_reply/local_reply.cc create mode 100644 source/common/local_reply/local_reply.h diff --git a/include/envoy/access_log/access_log.h b/include/envoy/access_log/access_log.h index 394df8f26ccb7..be8fc2c9213e1 100644 --- a/include/envoy/access_log/access_log.h +++ b/include/envoy/access_log/access_log.h @@ -114,7 +114,8 @@ class Formatter { virtual std::string format(const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers, const Http::HeaderMap& response_trailers, - const StreamInfo::StreamInfo& stream_info) const PURE; + const StreamInfo::StreamInfo& stream_info, + const absl::string_view& response_body) const PURE; }; using FormatterPtr = std::unique_ptr; @@ -138,7 +139,8 @@ class FormatterProvider { virtual std::string format(const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers, const Http::HeaderMap& response_trailers, - const StreamInfo::StreamInfo& stream_info) const PURE; + const StreamInfo::StreamInfo& stream_info, + const absl::string_view& response_body) const PURE; }; using FormatterProviderPtr = std::unique_ptr; diff --git a/source/common/access_log/access_log_formatter.cc b/source/common/access_log/access_log_formatter.cc index 7a20c3a36b673..b07b68ac194c7 100644 --- a/source/common/access_log/access_log_formatter.cc +++ b/source/common/access_log/access_log_formatter.cc @@ -104,12 +104,14 @@ FormatterImpl::FormatterImpl(const std::string& format) { std::string FormatterImpl::format(const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers, const Http::HeaderMap& response_trailers, - const StreamInfo::StreamInfo& stream_info) const { + const StreamInfo::StreamInfo& stream_info, + const absl::string_view& response_body) const { std::string log_line; log_line.reserve(256); for (const FormatterProviderPtr& provider : providers_) { - log_line += provider->format(request_headers, response_headers, response_trailers, stream_info); + log_line += provider->format(request_headers, response_headers, response_trailers, stream_info, + response_body); } return log_line; @@ -125,8 +127,10 @@ JsonFormatterImpl::JsonFormatterImpl(std::unordered_map JsonFormatterImpl::toMap( const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers, - const Http::HeaderMap& response_trailers, const StreamInfo::StreamInfo& stream_info) const { + const Http::HeaderMap& response_trailers, const StreamInfo::StreamInfo& stream_info, + const absl::string_view& body) const { std::unordered_map output; for (const auto& pair : json_output_format_) { output.emplace(pair.first, pair.second->format(request_headers, response_headers, - response_trailers, stream_info)); + response_trailers, stream_info, body)); } return output; } @@ -271,6 +276,8 @@ std::vector AccessLogFormatParser::parse(const std::string formatters.emplace_back(FormatterProviderPtr{ new ResponseTrailerFormatter(main_header, alternative_header, max_length)}); + } else if (absl::StartsWith(token, "RESP_BODY")) { + formatters.emplace_back(FormatterProviderPtr{new BodyFormatter()}); } else if (absl::StartsWith(token, DYNAMIC_META_TOKEN)) { std::string filter_namespace; absl::optional max_length; @@ -505,18 +512,27 @@ StreamInfoFormatter::StreamInfoFormatter(const std::string& field_name) { std::string StreamInfoFormatter::format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo& stream_info) const { + const StreamInfo::StreamInfo& stream_info, + const absl::string_view&) const { return field_extractor_(stream_info); } PlainStringFormatter::PlainStringFormatter(const std::string& str) : str_(str) {} std::string PlainStringFormatter::format(const Http::HeaderMap&, const Http::HeaderMap&, - const Http::HeaderMap&, - const StreamInfo::StreamInfo&) const { + const Http::HeaderMap&, const StreamInfo::StreamInfo&, + const absl::string_view&) const { return str_; } +BodyFormatter::BodyFormatter() {} + +std::string BodyFormatter::format(const Http::HeaderMap&, const Http::HeaderMap&, + const Http::HeaderMap&, const StreamInfo::StreamInfo&, + const absl::string_view& response_body) const { + return response_body.data(); +} + HeaderFormatter::HeaderFormatter(const std::string& main_header, const std::string& alternative_header, absl::optional max_length) @@ -550,8 +566,8 @@ ResponseHeaderFormatter::ResponseHeaderFormatter(const std::string& main_header, std::string ResponseHeaderFormatter::format(const Http::HeaderMap&, const Http::HeaderMap& response_headers, - const Http::HeaderMap&, - const StreamInfo::StreamInfo&) const { + const Http::HeaderMap&, const StreamInfo::StreamInfo&, + const absl::string_view&) const { return HeaderFormatter::format(response_headers); } @@ -562,7 +578,8 @@ RequestHeaderFormatter::RequestHeaderFormatter(const std::string& main_header, std::string RequestHeaderFormatter::format(const Http::HeaderMap& request_headers, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo&) const { + const StreamInfo::StreamInfo&, + const absl::string_view&) const { return HeaderFormatter::format(request_headers); } @@ -573,7 +590,8 @@ ResponseTrailerFormatter::ResponseTrailerFormatter(const std::string& main_heade std::string ResponseTrailerFormatter::format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap& response_trailers, - const StreamInfo::StreamInfo&) const { + const StreamInfo::StreamInfo&, + const absl::string_view&) const { return HeaderFormatter::format(response_trailers); } @@ -615,7 +633,8 @@ DynamicMetadataFormatter::DynamicMetadataFormatter(const std::string& filter_nam std::string DynamicMetadataFormatter::format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo& stream_info) const { + const StreamInfo::StreamInfo& stream_info, + const absl::string_view&) const { return MetadataFormatter::format(stream_info.dynamicMetadata()); } @@ -623,7 +642,8 @@ StartTimeFormatter::StartTimeFormatter(const std::string& format) : date_formatt std::string StartTimeFormatter::format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo& stream_info) const { + const StreamInfo::StreamInfo& stream_info, + const absl::string_view&) const { if (date_formatter_.formatString().empty()) { return AccessLogDateTimeFormatter::fromTime(stream_info.startTime()); } else { diff --git a/source/common/access_log/access_log_formatter.h b/source/common/access_log/access_log_formatter.h index 103657dcb40db..5b9a45320a845 100644 --- a/source/common/access_log/access_log_formatter.h +++ b/source/common/access_log/access_log_formatter.h @@ -93,7 +93,8 @@ class FormatterImpl : public Formatter { std::string format(const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers, const Http::HeaderMap& response_trailers, - const StreamInfo::StreamInfo& stream_info) const override; + const StreamInfo::StreamInfo& stream_info, + const absl::string_view& response_body) const override; private: std::vector providers_; @@ -107,15 +108,18 @@ class JsonFormatterImpl : public Formatter { std::string format(const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers, const Http::HeaderMap& response_trailers, - const StreamInfo::StreamInfo& stream_info) const override; + const StreamInfo::StreamInfo& stream_info, + const absl::string_view& response_body) const override; private: std::vector providers_; std::map json_output_format_; - std::unordered_map - toMap(const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers, - const Http::HeaderMap& response_trailers, const StreamInfo::StreamInfo& stream_info) const; + std::unordered_map toMap(const Http::HeaderMap& request_headers, + const Http::HeaderMap& response_headers, + const Http::HeaderMap& response_trailers, + const StreamInfo::StreamInfo& stream_info, + const absl::string_view& body) const; }; /** @@ -128,12 +132,26 @@ class PlainStringFormatter : public FormatterProvider { // Formatter::format std::string format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo&) const override; + const StreamInfo::StreamInfo&, const absl::string_view&) const override; private: std::string str_; }; +/** + * Formatter for string literal. It ignores headers and stream info and returns string by which it + * was initialized. + */ +class BodyFormatter : public FormatterProvider { +public: + BodyFormatter(); + + // Formatter::format + std::string format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, + const StreamInfo::StreamInfo&, + const absl::string_view& response_body) const override; +}; + class HeaderFormatter { public: HeaderFormatter(const std::string& main_header, const std::string& alternative_header, @@ -157,7 +175,8 @@ class RequestHeaderFormatter : public FormatterProvider, HeaderFormatter { // Formatter::format std::string format(const Http::HeaderMap& request_headers, const Http::HeaderMap&, - const Http::HeaderMap&, const StreamInfo::StreamInfo&) const override; + const Http::HeaderMap&, const StreamInfo::StreamInfo&, + const absl::string_view&) const override; }; /** @@ -170,7 +189,8 @@ class ResponseHeaderFormatter : public FormatterProvider, HeaderFormatter { // Formatter::format std::string format(const Http::HeaderMap&, const Http::HeaderMap& response_headers, - const Http::HeaderMap&, const StreamInfo::StreamInfo&) const override; + const Http::HeaderMap&, const StreamInfo::StreamInfo&, + const absl::string_view&) const override; }; /** @@ -183,8 +203,8 @@ class ResponseTrailerFormatter : public FormatterProvider, HeaderFormatter { // Formatter::format std::string format(const Http::HeaderMap&, const Http::HeaderMap&, - const Http::HeaderMap& response_trailers, - const StreamInfo::StreamInfo&) const override; + const Http::HeaderMap& response_trailers, const StreamInfo::StreamInfo&, + const absl::string_view&) const override; }; /** @@ -196,7 +216,8 @@ class StreamInfoFormatter : public FormatterProvider { // Formatter::format std::string format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo& stream_info) const override; + const StreamInfo::StreamInfo& stream_info, + const absl::string_view&) const override; using FieldExtractor = std::function; @@ -230,7 +251,8 @@ class DynamicMetadataFormatter : public FormatterProvider, MetadataFormatter { // FormatterProvider::format std::string format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo& stream_info) const override; + const StreamInfo::StreamInfo& stream_info, + const absl::string_view&) const override; }; /** @@ -240,7 +262,7 @@ class StartTimeFormatter : public FormatterProvider { public: StartTimeFormatter(const std::string& format); std::string format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo&) const override; + const StreamInfo::StreamInfo&, const absl::string_view&) const override; private: const Envoy::DateFormatter date_formatter_; diff --git a/source/common/http/BUILD b/source/common/http/BUILD index d317963b33c1c..fd99743c1e9c8 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -122,6 +122,7 @@ envoy_cc_library( "//include/envoy/config:config_provider_interface", "//include/envoy/http:filter_interface", "//include/envoy/router:rds_interface", + "//source/common/local_reply:local_reply_lib", "//source/common/network:utility_lib", "@envoy_api//envoy/config/filter/network/http_connection_manager/v2:pkg_cc_proto", ], diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index 5a3221f358cba..4efae31ccdcb6 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -7,6 +7,7 @@ #include "envoy/stats/scope.h" #include "common/http/date_provider.h" +#include "common/local_reply/local_reply.h" #include "common/network/utility.h" namespace Envoy { @@ -389,6 +390,11 @@ class ConnectionManagerConfig { * one. */ virtual bool shouldMergeSlashes() const PURE; + + /** + * @return LocalReply configuration which supplies mapping for local reply generated by Envoy. + */ + virtual LocalReply::LocalReply* localReply() const PURE; }; } // namespace Http } // namespace Envoy diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index aaa5c77c13d74..f27c704bde33c 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -1350,6 +1350,7 @@ void ConnectionManagerImpl::ActiveStream::sendLocalReply( createFilterChain(); } stream_info_.setResponseCodeDetails(details); + Utility::sendLocalReply( is_grpc_request, [this, modify_headers](HeaderMapPtr&& headers, bool end_stream) -> void { @@ -1366,6 +1367,20 @@ void ConnectionManagerImpl::ActiveStream::sendLocalReply( // request instead. encodeData(nullptr, data, end_stream, FilterIterationStartState::CanStartFromCurrent); }, + [this](Code& code) -> void { + connection_manager_.config_.localReply()->matchAndRewrite( + request_headers_.get(), response_headers_.get(), response_trailers_.get(), stream_info_, + code); + }, + [this](HeaderMapPtr&& headers, absl::string_view& body_text) -> std::string { + std::string formatted_body = connection_manager_.config_.localReply()->format( + request_headers_.get(), response_headers_.get(), response_trailers_.get(), stream_info_, + body_text); + + connection_manager_.config_.localReply()->insertContentHeaders(formatted_body, + headers.get()); + return formatted_body; + }, state_.destroyed_, code, body, grpc_status, is_head_request); } @@ -2301,6 +2316,20 @@ void ConnectionManagerImpl::ActiveStreamEncoderFilter::responseDataTooLarge() { parent_.response_encoder_->encodeData(data, end_stream); parent_.state_.local_complete_ = end_stream; }, + [&](Code& code) -> void { + parent_.connection_manager_.config_.localReply()->matchAndRewrite( + parent_.request_headers_.get(), parent_.response_headers_.get(), + parent_.response_trailers_.get(), parent_.stream_info_, code); + }, + [&](HeaderMapPtr&& headers, absl::string_view body_text) -> std::string { + std::string formatted_body = parent_.connection_manager_.config_.localReply()->format( + parent_.request_headers_.get(), parent_.response_headers_.get(), + parent_.response_trailers_.get(), parent_.stream_info_, body_text); + + parent_.connection_manager_.config_.localReply()->insertContentHeaders(formatted_body, + headers.get()); + return formatted_body; + }, parent_.state_.destroyed_, Http::Code::InternalServerError, CodeUtility::toString(Http::Code::InternalServerError), absl::nullopt, parent_.is_head_request_); diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 5b93c9b4faade..ac7429140c42e 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -306,8 +306,32 @@ void Utility::sendLocalReply( std::function encode_data, const bool& is_reset, Code response_code, absl::string_view body_text, const absl::optional grpc_status, bool is_head_request) { + sendLocalReply( + is_grpc, encode_headers, encode_data, [&](Code&) -> void {}, + [&](HeaderMapPtr&& response_headers, absl::string_view& body) -> std::string { + if (!body.empty()) { + response_headers->insertContentLength().value(body.size()); + response_headers->insertContentType().value(Headers::get().ContentTypeValues.Text); + return body.data(); + } + return {}; + }, + is_reset, response_code, body_text, grpc_status, is_head_request); +} + +void Utility::sendLocalReply( + bool is_grpc, std::function encode_headers, + std::function encode_data, + std::function rewrite_response, + std::function + format_response, + const bool& is_reset, Code response_code, absl::string_view body_text, + const absl::optional grpc_status, bool is_head_request) { // encode_headers() may reset the stream, so the stream must not be reset before calling it. ASSERT(!is_reset); + + // rewrite_response will rewrite response code (more might be added in future) + rewrite_response(response_code); // Respond with a gRPC trailers-only response if the request is gRPC if (is_grpc) { HeaderMapPtr response_headers{new HeaderMapImpl{ @@ -328,9 +352,9 @@ void Utility::sendLocalReply( HeaderMapPtr response_headers{ new HeaderMapImpl{{Headers::get().Status, std::to_string(enumToInt(response_code))}}}; + std::string formatted_body{}; if (!body_text.empty()) { - response_headers->insertContentLength().value(body_text.size()); - response_headers->insertContentType().value(Headers::get().ContentTypeValues.Text); + formatted_body = format_response(std::move(response_headers), body_text); } if (is_head_request) { @@ -338,10 +362,10 @@ void Utility::sendLocalReply( return; } - encode_headers(std::move(response_headers), body_text.empty()); + encode_headers(std::move(response_headers), formatted_body.empty()); // encode_headers()) may have changed the referenced is_reset so we need to test it - if (!body_text.empty() && !is_reset) { - Buffer::OwnedImpl buffer(body_text); + if (!formatted_body.empty() && !is_reset) { + Buffer::OwnedImpl buffer(formatted_body); encode_data(buffer, true); } } diff --git a/source/common/http/utility.h b/source/common/http/utility.h index fef7b08e11b6d..1b24f9046d786 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -213,6 +213,32 @@ void sendLocalReply(bool is_grpc, const absl::optional grpc_status, bool is_head_request = false); +/** + * Create a locally generated response using the provided lambdas with rewrite function. + * @param is_grpc tells if this is a response to a gRPC request. + * @param encode_headers supplies the function to encode response headers. + * @param encode_data supplies the function to encode the response body. + * @param rewrite_response supplies the function to rewrite locally generated response + * e.g. change status code + * @param format_response supplies the function to change format of locally generated reply + * @param is_reset boolean reference that indicates whether a stream has been reset. It is the + * responsibility of the caller to ensure that this is set to false if onDestroy() + * is invoked in the context of sendLocalReply(). + * @param response_code supplies the HTTP response code. + * @param body_text supplies the optional body text which is sent using the text/plain content + * type. + * @param grpc_status the gRPC status code to override the httpToGrpcStatus mapping with. + */ +void sendLocalReply(bool is_grpc, + std::function encode_headers, + std::function encode_data, + std::function rewrite_response, + std::function + format_response, + const bool& is_reset, Code response_code, absl::string_view body_text, + const absl::optional grpc_status, + bool is_head_request = false); + struct GetLastAddressFromXffInfo { // Last valid address pulled from the XFF header. Network::Address::InstanceConstSharedPtr address_; diff --git a/source/common/local_reply/BUILD b/source/common/local_reply/BUILD new file mode 100644 index 0000000000000..eafc9db02a793 --- /dev/null +++ b/source/common/local_reply/BUILD @@ -0,0 +1,19 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "local_reply_lib", + srcs = ["local_reply.cc"], + hdrs = ["local_reply.h"], + deps = [ + "//include/envoy/http:codes_interface", + "//source/common/access_log:access_log_formatter_lib", + ], +) diff --git a/source/common/local_reply/local_reply.cc b/source/common/local_reply/local_reply.cc new file mode 100644 index 0000000000000..9bc1b30047ded --- /dev/null +++ b/source/common/local_reply/local_reply.cc @@ -0,0 +1,68 @@ +#include "common/local_reply/local_reply.h" + +#include +#include + +#include "common/access_log/access_log_formatter.h" + +namespace Envoy { +namespace LocalReply { + +ResponseRewriter::ResponseRewriter(absl::optional response_code) + : response_code_(response_code) {} + +void ResponseRewriter::rewrite(Http::Code& code) { + code = static_cast(response_code_.value()); +} + +ResponseMapper::ResponseMapper(AccessLog::FilterPtr&& filter, ResponseRewriterPtr&& rewriter) { + filter_ = std::move(filter); + rewriter_ = std::move(rewriter); +} + +bool ResponseMapper::match(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + const Http::HeaderMap* response_trailers, + const StreamInfo::StreamInfo& stream_info) { + return filter_->evaluate(stream_info, *request_headers, *response_headers, *response_trailers); +} + +void ResponseMapper::rewrite(Http::Code& status_code) { rewriter_->rewrite(status_code); } + +LocalReply::LocalReply(std::list mappers, AccessLog::FormatterPtr&& formatter, + std::string content_type) { + mappers_ = std::move(mappers); + formatter_ = std::move(formatter); + content_type_ = content_type; +} + +void LocalReply::matchAndRewrite(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + const Http::HeaderMap* response_trailers, + const StreamInfo::StreamInfo& stream_info, Http::Code& code) { + for (auto& mapper : mappers_) { + if (mapper->match(request_headers, response_headers, response_trailers, stream_info)) { + mapper->rewrite(code); + break; + } + } +} + +std::string LocalReply::format(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + const Http::HeaderMap* response_trailers, + const StreamInfo::StreamInfo& stream_info, + const absl::string_view& body) { + return formatter_->format(*request_headers, *response_headers, *response_trailers, stream_info, + body); +} + +void LocalReply::insertContentHeaders(const absl::string_view& body, Http::HeaderMap* headers) { + if (!body.empty()) { + headers->insertContentLength().value(body.size()); + headers->insertContentType().value(content_type_); + } +} + +} // namespace LocalReply +} // namespace Envoy diff --git a/source/common/local_reply/local_reply.h b/source/common/local_reply/local_reply.h new file mode 100644 index 0000000000000..3f816eb8a0c6a --- /dev/null +++ b/source/common/local_reply/local_reply.h @@ -0,0 +1,116 @@ +#pragma once + +#include +#include + +#include "envoy/http/codes.h" + +#include "common/access_log/access_log_formatter.h" + +namespace Envoy { +namespace LocalReply { + +/** + * Configuration of response rewriter which contains status code to which matched local response + * should be mapped. + */ +class ResponseRewriter { +public: + ResponseRewriter(absl::optional response_code); + + /** + * Change given status code to one defined during instance creation. + * @param status_code supplies reference to status code. + */ + void rewrite(Http::Code& code); + +private: + absl::optional response_code_; +}; + +using ResponseRewriterPtr = std::unique_ptr; + +/** + * Configuration of response mapper which contains pair of filter and definiton of rewriter. + */ +class ResponseMapper { +public: + ResponseMapper(AccessLog::FilterPtr&& filter, ResponseRewriterPtr&& rewriter); + + /** + * Check if given request matches defined filter. + * @param request_headers supplies the information about request headers required by filters + * @param stream_info supplies the information about streams required by filters. + * @return true if matches filter. + */ + bool match(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_headers, + const Http::HeaderMap* response_trailers, const StreamInfo::StreamInfo& stream_info); + + /** + * Call rewriter method from ResponseRewriter. + * @param status_code supplies status code. + */ + void rewrite(Http::Code& status_code); + +private: + AccessLog::FilterPtr filter_; + ResponseRewriterPtr rewriter_; +}; + +using ResponseMapperPtr = std::unique_ptr; + +/** + * Configuration of local reply which contains list of ResponseMapper and formatter with proper + * content type. + */ +class LocalReply { +public: + LocalReply(std::list mappers, AccessLog::FormatterPtr&& formatter, + std::string content_type); + + /** + * Run through defined in configuration filters and for first matched filter rewrite local + * response. + * @param request_headers supplies the information about request headers required by filters. + * @param response_headers supplies response headers. + * @param response_trailers supplies response trailers. + * @param stream_info supplies the information about streams required by filters. + * @param code local reply status code. + */ + void matchAndRewrite(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + const Http::HeaderMap* response_trailers, + const StreamInfo::StreamInfo& stream_info, Http::Code& code); + + /** + * Run AccessLogFormatter format method which reformat data to structure defined in configuration. + * @param request_headers supplies the information about request headers required by filters. + * @param request_headers supplies the incoming request headers after filtering. + * @param response_headers supplies response headers. + * @param response_trailers supplies response trailers. + * @param stream_info supplies the information about streams required by filters. + * @param body original response body. + * @return std::string formatted response body. + */ + std::string format(const Http::HeaderMap* request_headers, + const Http::HeaderMap* response_headers, + const Http::HeaderMap* response_trailers, + const StreamInfo::StreamInfo& stream_info, const absl::string_view& body); + + /** + * Insert proper content-length and content-type headers for configured format. + * @param body supplies the body of response. + * @param headers supplies the pointer to header to which new one will be added. + */ + void insertContentHeaders(const absl::string_view& body, Http::HeaderMap* headers); + +private: + std::list mappers_; + AccessLog::FormatterPtr formatter_; + std::string content_type_; +}; + +using LocalReplyPtr = std::unique_ptr; + +} // namespace LocalReply +} // namespace Envoy diff --git a/source/common/router/header_formatter.cc b/source/common/router/header_formatter.cc index 0ee5f56586d76..a939a44b717c2 100644 --- a/source/common/router/header_formatter.cc +++ b/source/common/router/header_formatter.cc @@ -292,8 +292,8 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam Http::HeaderMapImpl empty_map; std::string formatted; for (const auto& formatter : formatters) { - absl::StrAppend(&formatted, - formatter->format(empty_map, empty_map, empty_map, stream_info)); + absl::StrAppend(&formatted, formatter->format(empty_map, empty_map, empty_map, stream_info, + std::string())); } return formatted; }; diff --git a/source/extensions/access_loggers/file/file_access_log_impl.cc b/source/extensions/access_loggers/file/file_access_log_impl.cc index 410204d3ad86f..a0ae76b4e754c 100644 --- a/source/extensions/access_loggers/file/file_access_log_impl.cc +++ b/source/extensions/access_loggers/file/file_access_log_impl.cc @@ -16,8 +16,8 @@ void FileAccessLog::emitLog(const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers, const Http::HeaderMap& response_trailers, const StreamInfo::StreamInfo& stream_info) { - log_file_->write( - formatter_->format(request_headers, response_headers, response_trailers, stream_info)); + log_file_->write(formatter_->format(request_headers, response_headers, response_trailers, + stream_info, std::string())); } } // namespace File diff --git a/source/extensions/filters/network/http_connection_manager/BUILD b/source/extensions/filters/network/http_connection_manager/BUILD index eaacb34f0fb07..559d10c661451 100644 --- a/source/extensions/filters/network/http_connection_manager/BUILD +++ b/source/extensions/filters/network/http_connection_manager/BUILD @@ -25,15 +25,18 @@ envoy_cc_library( "//include/envoy/server:admin_interface", "//include/envoy/server:options_interface", "//include/envoy/stats:stats_interface", + "//source/common/access_log:access_log_formatter_lib", "//source/common/access_log:access_log_lib", "//source/common/common:minimal_logger_lib", "//source/common/config:utility_lib", "//source/common/http:conn_manager_lib", "//source/common/http:default_server_string_lib", + "//source/common/http:headers_lib", "//source/common/http:utility_lib", "//source/common/http/http1:codec_lib", "//source/common/http/http2:codec_lib", "//source/common/json:json_loader_lib", + "//source/common/local_reply:local_reply_lib", "//source/common/protobuf:utility_lib", "//source/common/router:rds_lib", "//source/common/router:scoped_rds_lib", diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 0db976a5dcb78..472f3652edc8a 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -10,15 +10,18 @@ #include "envoy/server/admin.h" #include "envoy/tracing/http_tracer.h" +#include "common/access_log/access_log_formatter.h" #include "common/access_log/access_log_impl.h" #include "common/common/fmt.h" #include "common/config/utility.h" #include "common/http/conn_manager_utility.h" #include "common/http/date_provider_impl.h" #include "common/http/default_server_string.h" +#include "common/http/headers.h" #include "common/http/http1/codec_impl.h" #include "common/http/http2/codec_impl.h" #include "common/http/utility.h" +#include "common/local_reply/local_reply.h" #include "common/protobuf/utility.h" #include "common/router/rds_impl.h" #include "common/router/scoped_rds.h" @@ -54,6 +57,53 @@ FilterFactoryMap::const_iterator findUpgradeCaseInsensitive(const FilterFactoryM return upgrade_map.end(); } +std::unordered_map +convertJsonFormatToMap(ProtobufWkt::Struct json_format) { + std::unordered_map output; + for (const auto& pair : json_format.fields()) { + if (pair.second.kind_case() != ProtobufWkt::Value::kStringValue) { + throw EnvoyException("Only string values are supported in the JSON access log format."); + } + output.emplace(pair.first, pair.second.string_value()); + } + return output; +} + +AccessLog::FormatterPtr createFormatter(const Protobuf::Message& config, + Server::Configuration::FactoryContext& context) { + const auto& local_reply_config = MessageUtil::downcastAndValidate< + const envoy::config::filter::network::http_connection_manager::v2::LocalReplyConfig&>( + config, context.messageValidationVisitor()); + AccessLog::FormatterPtr formatter; + + if (local_reply_config.has_format()) { + if (local_reply_config.format().format_case() == envoy::type::StringOrJson::kStringFormat) { + formatter = std::make_unique( + local_reply_config.format().string_format()); + } else if (local_reply_config.format().format_case() == + envoy::type::StringOrJson::kJsonFormat) { + auto json_format_map = convertJsonFormatToMap(local_reply_config.format().json_format()); + formatter = std::make_unique(json_format_map); + } else { + formatter = std::make_unique("%RESP_BODY%"); + } + } else { + formatter = std::make_unique("%RESP_BODY%"); + } + + return formatter; +} + +std::string getContentType( + const envoy::config::filter::network::http_connection_manager::v2::LocalReplyConfig& config) { + if (config.has_format() && + config.format().format_case() == envoy::type::StringOrJson::kJsonFormat) { + return Http::Headers::get().ContentTypeValues.Json; + } else { + return Http::Headers::get().ContentTypeValues.Text; + } +} + std::unique_ptr createInternalAddressConfig( const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& config) { @@ -376,6 +426,30 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( std::make_pair(name, FilterConfig{std::move(factories), enabled})); } } + + // local_reply + std::list mappers; + if (config.has_local_reply_config() && !config.local_reply_config().mapper().empty()) { + + for (auto& mapper : config.local_reply_config().mapper()) { + if (mapper.has_filter() && mapper.has_rewriter()) { + AccessLog::FilterPtr filter = AccessLog::FilterFactory::fromProto( + mapper.filter(), context.runtime(), context.random(), + context.messageValidationVisitor()); + + LocalReply::ResponseRewriterPtr rewriter = std::make_unique( + LocalReply::ResponseRewriter{PROTOBUF_GET_WRAPPED_OR_DEFAULT( + mapper.rewriter(), status_code, absl::optional())}); + + LocalReply::ResponseMapperPtr mapper = std::make_unique( + LocalReply::ResponseMapper{std::move(filter), std::move(rewriter)}); + mappers.emplace_back(std::move(mapper)); + } + } + } + local_reply_ = std::make_unique(LocalReply::LocalReply{ + std::move(mappers), createFormatter(config.local_reply_config(), context), + getContentType(config.local_reply_config())}); } void HttpConnectionManagerConfig::processFilter( diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index 4bce5086ad2bc..970cebe1324f9 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -15,6 +15,7 @@ #include "common/common/logger.h" #include "common/http/conn_manager_impl.h" #include "common/json/json_loader.h" +#include "common/local_reply/local_reply.h" #include "extensions/filters/network/common/factory_base.h" #include "extensions/filters/network/well_known_names.h" @@ -143,6 +144,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, bool shouldNormalizePath() const override { return normalize_path_; } bool shouldMergeSlashes() const override { return merge_slashes_; } std::chrono::milliseconds delayedCloseTimeout() const override { return delayed_close_timeout_; } + LocalReply::LocalReply* localReply() const override { return local_reply_.get(); } private: enum class CodecType { HTTP1, HTTP2, HTTP3, AUTO }; @@ -191,6 +193,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, std::chrono::milliseconds delayed_close_timeout_; const bool normalize_path_; const bool merge_slashes_; + LocalReply::LocalReplyPtr local_reply_; // Default idle timeout is 5 minutes if nothing is specified in the HCM config. static const uint64_t StreamIdleTimeoutMs = 5 * 60 * 1000; diff --git a/source/server/http/admin.h b/source/server/http/admin.h index 3ace506ef6efd..f4b904238bf66 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -153,6 +153,7 @@ class AdminImpl : public Admin, const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return true; } bool shouldMergeSlashes() const override { return true; } + LocalReply::LocalReply* localReply() const override { return nullptr; }; Http::Code request(absl::string_view path_and_query, absl::string_view method, Http::HeaderMap& response_headers, std::string& body) override; void closeSocket(); diff --git a/test/common/access_log/access_log_formatter_fuzz_test.cc b/test/common/access_log/access_log_formatter_fuzz_test.cc index 1df65dd1736e6..22f470f94b7e7 100644 --- a/test/common/access_log/access_log_formatter_fuzz_test.cc +++ b/test/common/access_log/access_log_formatter_fuzz_test.cc @@ -13,9 +13,10 @@ DEFINE_PROTO_FUZZER(const test::common::access_log::TestCase& input) { std::vector formatters = AccessLog::AccessLogFormatParser::parse(input.format()); for (const auto& it : formatters) { - it->format( - Fuzz::fromHeaders(input.request_headers()), Fuzz::fromHeaders(input.response_headers()), - Fuzz::fromHeaders(input.response_trailers()), Fuzz::fromStreamInfo(input.stream_info())); + it->format(Fuzz::fromHeaders(input.request_headers()), + Fuzz::fromHeaders(input.response_headers()), + Fuzz::fromHeaders(input.response_trailers()), + Fuzz::fromStreamInfo(input.stream_info()), std::string{}); } ENVOY_LOG_MISC(trace, "Success"); } catch (const EnvoyException& e) { diff --git a/test/common/access_log/access_log_formatter_speed_test.cc b/test/common/access_log/access_log_formatter_speed_test.cc index 595f6e1f5c87c..e3d49701713ff 100644 --- a/test/common/access_log/access_log_formatter_speed_test.cc +++ b/test/common/access_log/access_log_formatter_speed_test.cc @@ -20,9 +20,10 @@ static void BM_AccessLogFormatter(benchmark::State& state) { Http::TestHeaderMapImpl request_headers; Http::TestHeaderMapImpl response_headers; Http::TestHeaderMapImpl response_trailers; + std::string body{}; for (auto _ : state) { output_bytes += - formatter->format(request_headers, response_headers, response_trailers, *stream_info) + formatter->format(request_headers, response_headers, response_trailers, *stream_info, body) .length(); } benchmark::DoNotOptimize(output_bytes); diff --git a/test/common/access_log/access_log_formatter_test.cc b/test/common/access_log/access_log_formatter_test.cc index 73f2e50db962e..9f03c0c8bc8e0 100644 --- a/test/common/access_log/access_log_formatter_test.cc +++ b/test/common/access_log/access_log_formatter_test.cc @@ -37,8 +37,9 @@ TEST(AccessLogFormatterTest, plainStringFormatter) { PlainStringFormatter formatter("plain"); Http::TestHeaderMapImpl header{{":method", "GET"}, {":path", "/"}}; StreamInfo::MockStreamInfo stream_info; + std::string body{}; - EXPECT_EQ("plain", formatter.format(header, header, header, stream_info)); + EXPECT_EQ("plain", formatter.format(header, header, header, stream_info, body)); } TEST(AccessLogFormatterTest, streamInfoFormatter) { @@ -46,33 +47,34 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { NiceMock stream_info; Http::TestHeaderMapImpl header{{":method", "GET"}, {":path", "/"}}; + std::string body{}; { StreamInfoFormatter request_duration_format("REQUEST_DURATION"); absl::optional dur = std::chrono::nanoseconds(5000000); EXPECT_CALL(stream_info, lastDownstreamRxByteReceived()).WillOnce(Return(dur)); - EXPECT_EQ("5", request_duration_format.format(header, header, header, stream_info)); + EXPECT_EQ("5", request_duration_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter request_duration_format("REQUEST_DURATION"); absl::optional dur; EXPECT_CALL(stream_info, lastDownstreamRxByteReceived()).WillOnce(Return(dur)); - EXPECT_EQ("-", request_duration_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", request_duration_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter response_duration_format("RESPONSE_DURATION"); absl::optional dur = std::chrono::nanoseconds(10000000); EXPECT_CALL(stream_info, firstUpstreamRxByteReceived()).WillRepeatedly(Return(dur)); - EXPECT_EQ("10", response_duration_format.format(header, header, header, stream_info)); + EXPECT_EQ("10", response_duration_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter response_duration_format("RESPONSE_DURATION"); absl::optional dur; EXPECT_CALL(stream_info, firstUpstreamRxByteReceived()).WillRepeatedly(Return(dur)); - EXPECT_EQ("-", response_duration_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", response_duration_format.format(header, header, header, stream_info, body)); } { @@ -83,7 +85,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { absl::optional dur_downstream = std::chrono::nanoseconds(25000000); EXPECT_CALL(stream_info, lastDownstreamTxByteSent()).WillRepeatedly(Return(dur_downstream)); - EXPECT_EQ("15", ttlb_duration_format.format(header, header, header, stream_info)); + EXPECT_EQ("15", ttlb_duration_format.format(header, header, header, stream_info, body)); } { @@ -94,122 +96,123 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { absl::optional dur_downstream; EXPECT_CALL(stream_info, lastDownstreamTxByteSent()).WillRepeatedly(Return(dur_downstream)); - EXPECT_EQ("-", ttlb_duration_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", ttlb_duration_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter bytes_received_format("BYTES_RECEIVED"); EXPECT_CALL(stream_info, bytesReceived()).WillOnce(Return(1)); - EXPECT_EQ("1", bytes_received_format.format(header, header, header, stream_info)); + EXPECT_EQ("1", bytes_received_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter protocol_format("PROTOCOL"); absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillOnce(Return(protocol)); - EXPECT_EQ("HTTP/1.1", protocol_format.format(header, header, header, stream_info)); + EXPECT_EQ("HTTP/1.1", protocol_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter response_format("RESPONSE_CODE"); absl::optional response_code{200}; EXPECT_CALL(stream_info, responseCode()).WillRepeatedly(Return(response_code)); - EXPECT_EQ("200", response_format.format(header, header, header, stream_info)); + EXPECT_EQ("200", response_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter response_code_format("RESPONSE_CODE"); absl::optional response_code; EXPECT_CALL(stream_info, responseCode()).WillRepeatedly(Return(response_code)); - EXPECT_EQ("0", response_code_format.format(header, header, header, stream_info)); + EXPECT_EQ("0", response_code_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter response_format("RESPONSE_CODE_DETAILS"); absl::optional rc_details; EXPECT_CALL(stream_info, responseCodeDetails()).WillRepeatedly(ReturnRef(rc_details)); - EXPECT_EQ("-", response_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", response_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter response_code_format("RESPONSE_CODE_DETAILS"); absl::optional rc_details{"via_upstream"}; EXPECT_CALL(stream_info, responseCodeDetails()).WillRepeatedly(ReturnRef(rc_details)); - EXPECT_EQ("via_upstream", response_code_format.format(header, header, header, stream_info)); + EXPECT_EQ("via_upstream", + response_code_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter bytes_sent_format("BYTES_SENT"); EXPECT_CALL(stream_info, bytesSent()).WillOnce(Return(1)); - EXPECT_EQ("1", bytes_sent_format.format(header, header, header, stream_info)); + EXPECT_EQ("1", bytes_sent_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter duration_format("DURATION"); absl::optional dur = std::chrono::nanoseconds(15000000); EXPECT_CALL(stream_info, requestComplete()).WillRepeatedly(Return(dur)); - EXPECT_EQ("15", duration_format.format(header, header, header, stream_info)); + EXPECT_EQ("15", duration_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter response_flags_format("RESPONSE_FLAGS"); ON_CALL(stream_info, hasResponseFlag(StreamInfo::ResponseFlag::LocalReset)) .WillByDefault(Return(true)); - EXPECT_EQ("LR", response_flags_format.format(header, header, header, stream_info)); + EXPECT_EQ("LR", response_flags_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("UPSTREAM_HOST"); - EXPECT_EQ("10.0.0.1:443", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("10.0.0.1:443", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("UPSTREAM_CLUSTER"); const std::string upstream_cluster_name = "cluster_name"; EXPECT_CALL(stream_info.host_->cluster_, name()).WillOnce(ReturnRef(upstream_cluster_name)); - EXPECT_EQ("cluster_name", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("cluster_name", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("UPSTREAM_HOST"); EXPECT_CALL(stream_info, upstreamHost()).WillOnce(Return(nullptr)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("UPSTREAM_CLUSTER"); EXPECT_CALL(stream_info, upstreamHost()).WillOnce(Return(nullptr)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_ADDRESS"); - EXPECT_EQ("127.0.0.2:0", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("127.0.0.2:0", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_ADDRESS_WITHOUT_PORT"); - EXPECT_EQ("127.0.0.2", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("127.0.0.2", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT"); - EXPECT_EQ("127.0.0.1", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("127.0.0.1", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_REMOTE_ADDRESS"); - EXPECT_EQ("127.0.0.1:0", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("127.0.0.1:0", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_DIRECT_REMOTE_ADDRESS_WITHOUT_PORT"); - EXPECT_EQ("127.0.0.1", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("127.0.0.1", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_DIRECT_REMOTE_ADDRESS"); - EXPECT_EQ("127.0.0.1:0", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("127.0.0.1:0", upstream_format.format(header, header, header, stream_info, body)); } { @@ -217,7 +220,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { std::string requested_server_name = "stub_server"; EXPECT_CALL(stream_info, requestedServerName()) .WillRepeatedly(ReturnRef(requested_server_name)); - EXPECT_EQ("stub_server", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("stub_server", upstream_format.format(header, header, header, stream_info, body)); } { @@ -225,7 +228,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { std::string requested_server_name; EXPECT_CALL(stream_info, requestedServerName()) .WillRepeatedly(ReturnRef(requested_server_name)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_URI_SAN"); @@ -233,7 +236,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { const std::vector sans{"san"}; EXPECT_CALL(*connection_info, uriSanPeerCertificate()).WillRepeatedly(Return(sans)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("san", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("san", upstream_format.format(header, header, header, stream_info, body)); } { @@ -242,7 +245,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { const std::vector sans{"san1", "san2"}; EXPECT_CALL(*connection_info, uriSanPeerCertificate()).WillRepeatedly(Return(sans)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("san1,san2", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("san1,san2", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_URI_SAN"); @@ -250,12 +253,12 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, uriSanPeerCertificate()) .WillRepeatedly(Return(std::vector())); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_URI_SAN"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_URI_SAN"); @@ -263,7 +266,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { const std::vector sans{"san"}; EXPECT_CALL(*connection_info, uriSanLocalCertificate()).WillRepeatedly(Return(sans)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("san", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("san", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_URI_SAN"); @@ -271,7 +274,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { const std::vector sans{"san1", "san2"}; EXPECT_CALL(*connection_info, uriSanLocalCertificate()).WillRepeatedly(Return(sans)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("san1,san2", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("san1,san2", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_URI_SAN"); @@ -279,12 +282,12 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, uriSanLocalCertificate()) .WillRepeatedly(Return(std::vector())); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_URI_SAN"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_SUBJECT"); @@ -293,7 +296,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, subjectLocalCertificate()) .WillRepeatedly(ReturnRef(subject_local)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("subject", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("subject", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_SUBJECT"); @@ -301,12 +304,12 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, subjectLocalCertificate()) .WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_SUBJECT"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SUBJECT"); @@ -314,19 +317,19 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { const std::string subject_peer = "subject"; EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(ReturnRef(subject_peer)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("subject", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("subject", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SUBJECT"); auto connection_info = std::make_shared(); EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SUBJECT"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_SESSION_ID"); @@ -334,19 +337,19 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { const std::string session_id = "deadbeef"; EXPECT_CALL(*connection_info, sessionId()).WillRepeatedly(ReturnRef(session_id)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("deadbeef", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("deadbeef", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_SESSION_ID"); auto connection_info = std::make_shared(); EXPECT_CALL(*connection_info, sessionId()).WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_SESSION_ID"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_CIPHER"); @@ -355,19 +358,19 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { .WillRepeatedly(Return("TLS_DHE_RSA_WITH_AES_256_GCM_SHA384")); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("TLS_DHE_RSA_WITH_AES_256_GCM_SHA384", - upstream_format.format(header, header, header, stream_info)); + upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_CIPHER"); auto connection_info = std::make_shared(); EXPECT_CALL(*connection_info, ciphersuiteString()).WillRepeatedly(Return("")); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_CIPHER"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_VERSION"); @@ -375,19 +378,19 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { std::string tlsVersion = "TLSv1.2"; EXPECT_CALL(*connection_info, tlsVersion()).WillRepeatedly(ReturnRef(tlsVersion)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("TLSv1.2", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("TLSv1.2", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_VERSION"); auto connection_info = std::make_shared(); EXPECT_CALL(*connection_info, tlsVersion()).WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_VERSION"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_FINGERPRINT_256"); @@ -396,7 +399,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, sha256PeerCertificateDigest()) .WillRepeatedly(ReturnRef(expected_sha)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ(expected_sha, upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ(expected_sha, upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_FINGERPRINT_256"); @@ -405,12 +408,12 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, sha256PeerCertificateDigest()) .WillRepeatedly(ReturnRef(expected_sha)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_FINGERPRINT_256"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SERIAL"); @@ -419,7 +422,8 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, serialNumberPeerCertificate()) .WillRepeatedly(ReturnRef(serial_number)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("b8b5ecc898f2124a", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("b8b5ecc898f2124a", + upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SERIAL"); @@ -427,12 +431,12 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, serialNumberPeerCertificate()) .WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SERIAL"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_ISSUER"); @@ -442,19 +446,19 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, issuerPeerCertificate()).WillRepeatedly(ReturnRef(issuer_peer)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("CN=Test CA,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US", - upstream_format.format(header, header, header, stream_info)); + upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_ISSUER"); auto connection_info = std::make_shared(); EXPECT_CALL(*connection_info, issuerPeerCertificate()).WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_ISSUER"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SUBJECT"); @@ -464,19 +468,19 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(ReturnRef(subject_peer)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("CN=Test Server,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US", - upstream_format.format(header, header, header, stream_info)); + upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SUBJECT"); auto connection_info = std::make_shared(); EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SUBJECT"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_CERT"); @@ -485,7 +489,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, urlEncodedPemEncodedPeerCertificate()) .WillRepeatedly(ReturnRef(expected_cert)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ(expected_cert, upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ(expected_cert, upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_CERT"); @@ -494,12 +498,12 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, urlEncodedPemEncodedPeerCertificate()) .WillRepeatedly(ReturnRef(expected_cert)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_CERT"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_CERT_V_START"); @@ -510,19 +514,19 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, validFromPeerCertificate()).WillRepeatedly(Return(startTime)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("2018-12-18T01:50:34.000Z", - upstream_format.format(header, header, header, stream_info)); + upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_CERT_V_START"); auto connection_info = std::make_shared(); EXPECT_CALL(*connection_info, validFromPeerCertificate()).WillRepeatedly(Return(absl::nullopt)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_CERT_V_START"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_CERT_V_END"); @@ -533,7 +537,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, expirationPeerCertificate()).WillRepeatedly(Return(endTime)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("2020-12-17T01:50:34.000Z", - upstream_format.format(header, header, header, stream_info)); + upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_CERT_V_END"); @@ -541,19 +545,19 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { EXPECT_CALL(*connection_info, expirationPeerCertificate()) .WillRepeatedly(Return(absl::nullopt)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(nullptr)); StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_CERT_V_END"); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } { StreamInfoFormatter upstream_format("UPSTREAM_TRANSPORT_FAILURE_REASON"); std::string upstream_transport_failure_reason = "SSL error"; EXPECT_CALL(stream_info, upstreamTransportFailureReason()) .WillRepeatedly(ReturnRef(upstream_transport_failure_reason)); - EXPECT_EQ("SSL error", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("SSL error", upstream_format.format(header, header, header, stream_info, body)); } { @@ -561,7 +565,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { std::string upstream_transport_failure_reason; EXPECT_CALL(stream_info, upstreamTransportFailureReason()) .WillRepeatedly(ReturnRef(upstream_transport_failure_reason)); - EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); + EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info, body)); } } @@ -570,29 +574,30 @@ TEST(AccessLogFormatterTest, requestHeaderFormatter) { Http::TestHeaderMapImpl request_header{{":method", "GET"}, {":path", "/"}}; Http::TestHeaderMapImpl response_header{{":method", "PUT"}}; Http::TestHeaderMapImpl response_trailer{{":method", "POST"}, {"test-2", "test-2"}}; + std::string body{}; { RequestHeaderFormatter formatter(":Method", "", absl::optional()); - EXPECT_EQ("GET", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("GET", formatter.format(request_header, response_header, response_trailer, + stream_info, body)); } { RequestHeaderFormatter formatter(":path", ":method", absl::optional()); - EXPECT_EQ("/", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("/", formatter.format(request_header, response_header, response_trailer, stream_info, + body)); } { RequestHeaderFormatter formatter(":TEST", ":METHOD", absl::optional()); - EXPECT_EQ("GET", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("GET", formatter.format(request_header, response_header, response_trailer, + stream_info, body)); } { RequestHeaderFormatter formatter("does_not_exist", "", absl::optional()); - EXPECT_EQ("-", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("-", formatter.format(request_header, response_header, response_trailer, stream_info, + body)); } } @@ -601,29 +606,30 @@ TEST(AccessLogFormatterTest, responseHeaderFormatter) { Http::TestHeaderMapImpl request_header{{":method", "GET"}, {":path", "/"}}; Http::TestHeaderMapImpl response_header{{":method", "PUT"}, {"test", "test"}}; Http::TestHeaderMapImpl response_trailer{{":method", "POST"}, {"test-2", "test-2"}}; + std::string body{}; { ResponseHeaderFormatter formatter(":method", "", absl::optional()); - EXPECT_EQ("PUT", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("PUT", formatter.format(request_header, response_header, response_trailer, + stream_info, body)); } { ResponseHeaderFormatter formatter("test", ":method", absl::optional()); - EXPECT_EQ("test", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("test", formatter.format(request_header, response_header, response_trailer, + stream_info, body)); } { ResponseHeaderFormatter formatter(":path", ":method", absl::optional()); - EXPECT_EQ("PUT", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("PUT", formatter.format(request_header, response_header, response_trailer, + stream_info, body)); } { ResponseHeaderFormatter formatter("does_not_exist", "", absl::optional()); - EXPECT_EQ("-", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("-", formatter.format(request_header, response_header, response_trailer, stream_info, + body)); } } @@ -632,29 +638,30 @@ TEST(AccessLogFormatterTest, responseTrailerFormatter) { Http::TestHeaderMapImpl request_header{{":method", "GET"}, {":path", "/"}}; Http::TestHeaderMapImpl response_header{{":method", "PUT"}, {"test", "test"}}; Http::TestHeaderMapImpl response_trailer{{":method", "POST"}, {"test-2", "test-2"}}; + std::string body{}; { ResponseTrailerFormatter formatter(":method", "", absl::optional()); - EXPECT_EQ("POST", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("POST", formatter.format(request_header, response_header, response_trailer, + stream_info, body)); } { ResponseTrailerFormatter formatter("test-2", ":method", absl::optional()); - EXPECT_EQ("test-2", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("test-2", formatter.format(request_header, response_header, response_trailer, + stream_info, body)); } { ResponseTrailerFormatter formatter(":path", ":method", absl::optional()); - EXPECT_EQ("POST", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("POST", formatter.format(request_header, response_header, response_trailer, + stream_info, body)); } { ResponseTrailerFormatter formatter("does_not_exist", "", absl::optional()); - EXPECT_EQ("-", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("-", formatter.format(request_header, response_header, response_trailer, stream_info, + body)); } } @@ -726,13 +733,14 @@ TEST(AccessLogFormatterTest, dynamicMetadataFormatter) { TEST(AccessLogFormatterTest, startTimeFormatter) { NiceMock stream_info; Http::TestHeaderMapImpl header{{":method", "GET"}, {":path", "/"}}; + std::string body{}; { StartTimeFormatter start_time_format("%Y/%m/%d"); time_t test_epoch = 1522280158; SystemTime time = std::chrono::system_clock::from_time_t(test_epoch); EXPECT_CALL(stream_info, startTime()).WillOnce(Return(time)); - EXPECT_EQ("2018/03/28", start_time_format.format(header, header, header, stream_info)); + EXPECT_EQ("2018/03/28", start_time_format.format(header, header, header, stream_info, body)); } { @@ -740,7 +748,18 @@ TEST(AccessLogFormatterTest, startTimeFormatter) { SystemTime time; EXPECT_CALL(stream_info, startTime()).WillOnce(Return(time)); EXPECT_EQ(AccessLogDateTimeFormatter::fromTime(time), - start_time_format.format(header, header, header, stream_info)); + start_time_format.format(header, header, header, stream_info, body)); + } +} + +TEST(AccessLogFormatterTest, bodyFormatterTest) { + NiceMock stream_info; + Http::TestHeaderMapImpl header{{":method", "GET"}, {":path", "/"}}; + std::string body = "My test message"; + + { + BodyFormatter body_formatter{}; + EXPECT_EQ("My test message", body_formatter.format(header, header, header, stream_info, body)); } } @@ -764,6 +783,7 @@ TEST(AccessLogFormatterTest, JsonFormatterPlainStringTest) { Http::TestHeaderMapImpl request_header; Http::TestHeaderMapImpl response_header; Http::TestHeaderMapImpl response_trailer; + std::string body{}; envoy::api::v2::core::Metadata metadata; populateMetadataTestData(metadata); @@ -777,8 +797,9 @@ TEST(AccessLogFormatterTest, JsonFormatterPlainStringTest) { {"plain_string", "plain_string_value"}}; JsonFormatterImpl formatter(key_mapping); - verifyJsonOutput(formatter.format(request_header, response_header, response_trailer, stream_info), - expected_json_map); + verifyJsonOutput( + formatter.format(request_header, response_header, response_trailer, stream_info, body), + expected_json_map); } TEST(AccessLogFormatterTest, JsonFormatterSingleOperatorTest) { @@ -786,6 +807,7 @@ TEST(AccessLogFormatterTest, JsonFormatterSingleOperatorTest) { Http::TestHeaderMapImpl request_header; Http::TestHeaderMapImpl response_header; Http::TestHeaderMapImpl response_trailer; + std::string body{}; envoy::api::v2::core::Metadata metadata; populateMetadataTestData(metadata); @@ -797,8 +819,9 @@ TEST(AccessLogFormatterTest, JsonFormatterSingleOperatorTest) { std::unordered_map key_mapping = {{"protocol", "%PROTOCOL%"}}; JsonFormatterImpl formatter(key_mapping); - verifyJsonOutput(formatter.format(request_header, response_header, response_trailer, stream_info), - expected_json_map); + verifyJsonOutput( + formatter.format(request_header, response_header, response_trailer, stream_info, body), + expected_json_map); } TEST(AccessLogFormatterTest, JsonFormatterNonExistentHeaderTest) { @@ -806,6 +829,7 @@ TEST(AccessLogFormatterTest, JsonFormatterNonExistentHeaderTest) { Http::TestHeaderMapImpl request_header{{"some_request_header", "SOME_REQUEST_HEADER"}}; Http::TestHeaderMapImpl response_header{{"some_response_header", "SOME_RESPONSE_HEADER"}}; Http::TestHeaderMapImpl response_trailer; + std::string body{}; std::unordered_map expected_json_map = { {"protocol", "HTTP/1.1"}, @@ -823,8 +847,9 @@ TEST(AccessLogFormatterTest, JsonFormatterNonExistentHeaderTest) { absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyJsonOutput(formatter.format(request_header, response_header, response_trailer, stream_info), - expected_json_map); + verifyJsonOutput( + formatter.format(request_header, response_header, response_trailer, stream_info, body), + expected_json_map); } TEST(AccessLogFormatterTest, JsonFormatterAlternateHeaderTest) { @@ -832,6 +857,7 @@ TEST(AccessLogFormatterTest, JsonFormatterAlternateHeaderTest) { Http::TestHeaderMapImpl request_header{{"request_present_header", "REQUEST_PRESENT_HEADER"}}; Http::TestHeaderMapImpl response_header{{"response_present_header", "RESPONSE_PRESENT_HEADER"}}; Http::TestHeaderMapImpl response_trailer; + std::string body{}; std::unordered_map expected_json_map = { {"request_present_header_or_request_absent_header", "REQUEST_PRESENT_HEADER"}, @@ -853,8 +879,9 @@ TEST(AccessLogFormatterTest, JsonFormatterAlternateHeaderTest) { absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyJsonOutput(formatter.format(request_header, response_header, response_trailer, stream_info), - expected_json_map); + verifyJsonOutput( + formatter.format(request_header, response_header, response_trailer, stream_info, body), + expected_json_map); } TEST(AccessLogFormatterTest, JsonFormatterDynamicMetadataTest) { @@ -862,6 +889,7 @@ TEST(AccessLogFormatterTest, JsonFormatterDynamicMetadataTest) { Http::TestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; Http::TestHeaderMapImpl response_trailer{{"third", "POST"}, {"test-2", "test-2"}}; + std::string body{}; envoy::api::v2::core::Metadata metadata; populateMetadataTestData(metadata); @@ -880,8 +908,9 @@ TEST(AccessLogFormatterTest, JsonFormatterDynamicMetadataTest) { JsonFormatterImpl formatter(key_mapping); - verifyJsonOutput(formatter.format(request_header, response_header, response_trailer, stream_info), - expected_json_map); + verifyJsonOutput( + formatter.format(request_header, response_header, response_trailer, stream_info, body), + expected_json_map); } TEST(AccessLogFormatterTest, JsonFormatterStartTimeTest) { @@ -889,6 +918,7 @@ TEST(AccessLogFormatterTest, JsonFormatterStartTimeTest) { Http::TestHeaderMapImpl request_header; Http::TestHeaderMapImpl response_header; Http::TestHeaderMapImpl response_trailer; + std::string body{}; time_t expected_time_in_epoch = 1522280158; SystemTime time = std::chrono::system_clock::from_time_t(expected_time_in_epoch); @@ -909,8 +939,9 @@ TEST(AccessLogFormatterTest, JsonFormatterStartTimeTest) { {"all_zeroes", "%START_TIME(%f.%1f.%2f.%3f)%"}}; JsonFormatterImpl formatter(key_mapping); - verifyJsonOutput(formatter.format(request_header, response_header, response_trailer, stream_info), - expected_json_map); + verifyJsonOutput( + formatter.format(request_header, response_header, response_trailer, stream_info, body), + expected_json_map); } TEST(AccessLogFormatterTest, JsonFormatterMultiTokenTest) { @@ -919,6 +950,7 @@ TEST(AccessLogFormatterTest, JsonFormatterMultiTokenTest) { Http::TestHeaderMapImpl request_header{{"some_request_header", "SOME_REQUEST_HEADER"}}; Http::TestHeaderMapImpl response_header{{"some_response_header", "SOME_RESPONSE_HEADER"}}; Http::TestHeaderMapImpl response_trailer; + std::string body{}; std::unordered_map expected_json_map = { {"multi_token_field", "HTTP/1.1 plainstring SOME_REQUEST_HEADER SOME_RESPONSE_HEADER"}}; @@ -932,7 +964,7 @@ TEST(AccessLogFormatterTest, JsonFormatterMultiTokenTest) { EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); const auto parsed = Json::Factory::loadFromString( - formatter.format(request_header, response_header, response_trailer, stream_info)); + formatter.format(request_header, response_header, response_trailer, stream_info, body)); for (const auto& pair : expected_json_map) { EXPECT_EQ(parsed->getString(pair.first), pair.second); } @@ -944,6 +976,7 @@ TEST(AccessLogFormatterTest, CompositeFormatterSuccess) { Http::TestHeaderMapImpl request_header{{"first", "GET"}, {":path", "/"}}; Http::TestHeaderMapImpl response_header{{"second", "PUT"}, {"test", "test"}}; Http::TestHeaderMapImpl response_trailer{{"third", "POST"}, {"test-2", "test-2"}}; + std::string body = "Test body"; { const std::string format = "{{%PROTOCOL%}} %RESP(not exist)%++%RESP(test)% " @@ -954,16 +987,26 @@ TEST(AccessLogFormatterTest, CompositeFormatterSuccess) { absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - EXPECT_EQ("{{HTTP/1.1}} -++test GET PUT\t@POST@\ttest-2[]", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ( + "{{HTTP/1.1}} -++test GET PUT\t@POST@\ttest-2[]", + formatter.format(request_header, response_header, response_trailer, stream_info, body)); } { const std::string format = "{}*JUST PLAIN string]"; FormatterImpl formatter(format); - EXPECT_EQ(format, - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ(format, formatter.format(request_header, response_header, response_trailer, + stream_info, body)); + } + + { + const std::string format = "%RESP_BODY% - with response body"; + FormatterImpl formatter(format); + + EXPECT_EQ( + "Test body - with response body", + formatter.format(request_header, response_header, response_trailer, stream_info, body)); } { @@ -972,8 +1015,8 @@ TEST(AccessLogFormatterTest, CompositeFormatterSuccess) { FormatterImpl formatter(format); - EXPECT_EQ("GET|G|PU|GET|POS", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ("GET|G|PU|GET|POS", formatter.format(request_header, response_header, + response_trailer, stream_info, body)); } { @@ -985,8 +1028,9 @@ TEST(AccessLogFormatterTest, CompositeFormatterSuccess) { "test_obj)%|%DYNAMIC_METADATA(com.test:test_obj:inner_key)%"; FormatterImpl formatter(format); - EXPECT_EQ("\"test_value\"|{\"inner_key\":\"inner_value\"}|\"inner_value\"", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ( + "\"test_value\"|{\"inner_key\":\"inner_value\"}|\"inner_value\"", + formatter.format(request_header, response_header, response_trailer, stream_info, body)); } { @@ -998,9 +1042,10 @@ TEST(AccessLogFormatterTest, CompositeFormatterSuccess) { EXPECT_CALL(stream_info, startTime()).WillRepeatedly(Return(time)); FormatterImpl formatter(format); - EXPECT_EQ(fmt::format("2018/03/28|{}|bad_format|2018-03-28T23:35:58.000Z|000000000.0.00.000", - expected_time_in_epoch), - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ( + fmt::format("2018/03/28|{}|bad_format|2018-03-28T23:35:58.000Z|000000000.0.00.000", + expected_time_in_epoch), + formatter.format(request_header, response_header, response_trailer, stream_info, body)); } { @@ -1013,8 +1058,9 @@ TEST(AccessLogFormatterTest, CompositeFormatterSuccess) { EXPECT_CALL(stream_info, startTime()).WillRepeatedly(Return(time)); FormatterImpl formatter(format); - EXPECT_EQ("1970/01/01|0|bad_format|1970-01-01T00:00:00.000Z|000000000.0.00.000", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ( + "1970/01/01|0|bad_format|1970-01-01T00:00:00.000Z|000000000.0.00.000", + formatter.format(request_header, response_header, response_trailer, stream_info, body)); } { @@ -1024,8 +1070,9 @@ TEST(AccessLogFormatterTest, CompositeFormatterSuccess) { const SystemTime start_time(std::chrono::microseconds(1522796769123456)); EXPECT_CALL(stream_info, startTime()).WillRepeatedly(Return(start_time)); FormatterImpl formatter(format); - EXPECT_EQ("1522796769.123|1522796769.1234|1522796769.12345|1522796769.123456", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ( + "1522796769.123|1522796769.1234|1522796769.12345|1522796769.123456", + formatter.format(request_header, response_header, response_trailer, stream_info, body)); } { @@ -1034,9 +1081,10 @@ TEST(AccessLogFormatterTest, CompositeFormatterSuccess) { const SystemTime start_time(std::chrono::microseconds(1522796769123456)); EXPECT_CALL(stream_info, startTime()).WillRepeatedly(Return(start_time)); FormatterImpl formatter(format); - EXPECT_EQ("segment1:1522796769.123|segment2:1522796769.1234|seg3:1522796769.123456|1522796769-" - "123-asdf-123456000|.1234560:segm5:2018", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ( + "segment1:1522796769.123|segment2:1522796769.1234|seg3:1522796769.123456|1522796769-" + "123-asdf-123456000|.1234560:segm5:2018", + formatter.format(request_header, response_header, response_trailer, stream_info, body)); } { @@ -1046,8 +1094,9 @@ TEST(AccessLogFormatterTest, CompositeFormatterSuccess) { const SystemTime start_time(std::chrono::microseconds(1522796769123456)); EXPECT_CALL(stream_info, startTime()).WillOnce(Return(start_time)); FormatterImpl formatter(format); - EXPECT_EQ("%%|%%123456000|1522796769%%123|1%%1522796769", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ( + "%%|%%123456000|1522796769%%123|1%%1522796769", + formatter.format(request_header, response_header, response_trailer, stream_info, body)); } } diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index d64e24b3f1b78..94b392bf3a29d 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -130,6 +130,7 @@ class FuzzConfig : public ConnectionManagerConfig { const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return false; } bool shouldMergeSlashes() const override { return false; } + LocalReply::LocalReply* localReply() const override { return local_reply_.get(); } const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager config_; std::list access_logs_; @@ -167,6 +168,10 @@ class FuzzConfig : public ConnectionManagerConfig { Http::Http1Settings http1_settings_; Http::DefaultInternalAddressConfig internal_address_config_; bool normalize_path_{true}; + LocalReply::LocalReplyPtr local_reply_ = std::make_unique( + LocalReply::LocalReply{{}, + std::make_unique("%RESP_BODY%"), + Http::Headers::get().ContentTypeValues.Text}); }; // Internal representation of stream state. Encapsulates the stream state, mocks diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index f137879b20c46..df26feed2787a 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -128,6 +128,11 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan false, 256}); } + + local_reply_ = std::make_unique( + LocalReply::LocalReply{{}, + std::make_unique("%RESP_BODY%"), + Http::Headers::get().ContentTypeValues.Text}); } void setupFilterChain(int num_decoder_filters, int num_encoder_filters) { @@ -315,6 +320,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return normalize_path_; } bool shouldMergeSlashes() const override { return merge_slashes_; } + LocalReply::LocalReply* localReply() const override { return local_reply_.get(); } DangerousDeprecatedTestTime test_time_; NiceMock route_config_provider_; @@ -369,6 +375,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan Http::Http1Settings http1_settings_; bool normalize_path_ = false; bool merge_slashes_ = false; + LocalReply::LocalReplyPtr local_reply_; NiceMock upstream_conn_; // for websocket tests NiceMock conn_pool_; // for websocket tests diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 315e35938a068..1825c991ab613 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -91,6 +91,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_CONST_METHOD0(http1Settings, const Http::Http1Settings&()); MOCK_CONST_METHOD0(shouldNormalizePath, bool()); MOCK_CONST_METHOD0(shouldMergeSlashes, bool()); + MOCK_CONST_METHOD0(localReply, LocalReply::LocalReply*()); std::unique_ptr internal_address_config_ = std::make_unique(); @@ -108,7 +109,12 @@ class ConnectionManagerUtilityTest : public testing::Test { percent2.set_denominator(envoy::type::FractionalPercent::TEN_THOUSAND); tracing_config_ = { Tracing::OperationName::Ingress, {}, percent1, percent2, percent1, false, 256}; + local_reply_ = std::make_unique( + LocalReply::LocalReply{{}, + std::make_unique("%RESP_BODY%"), + Http::Headers::get().ContentTypeValues.Text}); ON_CALL(config_, tracingConfig()).WillByDefault(Return(&tracing_config_)); + ON_CALL(config_, localReply()).WillByDefault(Return(local_reply_.get())); ON_CALL(config_, via()).WillByDefault(ReturnRef(via_)); } @@ -145,6 +151,7 @@ class ConnectionManagerUtilityTest : public testing::Test { NiceMock runtime_; Http::TracingConnectionManagerConfig tracing_config_; NiceMock local_info_; + LocalReply::LocalReplyPtr local_reply_; std::string canary_node_{"canary"}; std::string empty_node_; std::string via_; From 54f3ff48fed49147cc118c0b8722f80da7a08094 Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Mon, 11 Nov 2019 17:57:19 +0100 Subject: [PATCH 03/15] Added tests and fixed comments from code review Signed-off-by: Lukasz Dziedziak --- .../filter/accesslog/v2/accesslog.proto | 3 +- .../filter/accesslog/v3alpha/accesslog.proto | 3 +- .../v2/http_connection_manager.proto | 1 - .../v3alpha/http_connection_manager.proto | 1 - include/envoy/access_log/access_log.h | 1 + source/common/http/async_client_impl.h | 21 +- source/common/http/conn_manager_impl.cc | 122 ++--- source/common/http/utility.cc | 108 ++-- source/common/http/utility.h | 91 ++-- source/common/local_reply/local_reply.cc | 17 +- .../access_log/access_log_formatter_test.cc | 27 +- test/common/http/utility_test.cc | 39 +- test/common/local_reply/BUILD | 20 + test/common/local_reply/local_reply_test.cc | 82 +++ test/config/utility.cc | 11 + test/config/utility.h | 3 + test/integration/BUILD | 12 + .../local_reply_integration_test.cc | 465 ++++++++++++++++++ test/mocks/http/mocks.cc | 21 +- test/test_common/utility.h | 21 + 20 files changed, 837 insertions(+), 232 deletions(-) create mode 100644 test/common/local_reply/BUILD create mode 100644 test/common/local_reply/local_reply_test.cc create mode 100644 test/integration/local_reply_integration_test.cc diff --git a/api/envoy/config/filter/accesslog/v2/accesslog.proto b/api/envoy/config/filter/accesslog/v2/accesslog.proto index 16a51a23eb131..c13f09fd3a264 100644 --- a/api/envoy/config/filter/accesslog/v2/accesslog.proto +++ b/api/envoy/config/filter/accesslog/v2/accesslog.proto @@ -45,8 +45,7 @@ message AccessLog { } } -// [#next-major-version: In the v3 API, we should consider renaming -// it to more generic filter] +// [#next-major-version: In the v3 API, we should consider renaming it to more generic filter] // [#next-free-field: 12] message AccessLogFilter { oneof filter_specifier { diff --git a/api/envoy/config/filter/accesslog/v3alpha/accesslog.proto b/api/envoy/config/filter/accesslog/v3alpha/accesslog.proto index e83377399db50..a2bd71ddc62de 100644 --- a/api/envoy/config/filter/accesslog/v3alpha/accesslog.proto +++ b/api/envoy/config/filter/accesslog/v3alpha/accesslog.proto @@ -47,8 +47,7 @@ message AccessLog { } } -// [#next-major-version: In the v3 API, we should consider renaming -// it to more generic filter] +// [#next-major-version: In the v3 API, we should consider renaming it to more generic filter] // [#next-free-field: 12] message AccessLogFilter { oneof filter_specifier { diff --git a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto index d1123f3dec940..7b93ef5caad8e 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto @@ -469,7 +469,6 @@ message HttpConnectionManager { // `HTTP spec ` and is provided for convenience. bool merge_slashes = 33; - // [#not-implemented-hide:] // Configuration of local reply returned by Envoy. Allows to specify mappings and format of // response. LocalReplyConfig local_reply_config = 36; diff --git a/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto index 4c7543eec7cba..6c205f8ffee66 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto @@ -456,7 +456,6 @@ message HttpConnectionManager { // `HTTP spec ` and is provided for convenience. bool merge_slashes = 33; - // [#not-implemented-hide:] // Configuration of local reply returned by Envoy. Allows to specify mappings and format of // response. LocalReplyConfig local_reply_config = 36; diff --git a/include/envoy/access_log/access_log.h b/include/envoy/access_log/access_log.h index be8fc2c9213e1..4cafc44005ef0 100644 --- a/include/envoy/access_log/access_log.h +++ b/include/envoy/access_log/access_log.h @@ -134,6 +134,7 @@ class FormatterProvider { * @param response_headers supplies the response headers. * @param response_trailers supplies the response trailers. * @param stream_info supplies the stream info. + * @param response_body supplies the response body. * @return std::string containing a single value extracted from the given headers/trailers/stream. */ virtual std::string format(const Http::HeaderMap& request_headers, diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 5f45456045692..55c74b94785c0 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -345,15 +345,18 @@ class AsyncStreamImpl : public AsyncClient::Stream, absl::string_view details) override { stream_info_.setResponseCodeDetails(details); Utility::sendLocalReply( - is_grpc_request_, - [this, modify_headers](HeaderMapPtr&& headers, bool end_stream) -> void { - if (modify_headers != nullptr) { - modify_headers(*headers); - } - encodeHeaders(std::move(headers), end_stream); - }, - [this](Buffer::Instance& data, bool end_stream) -> void { encodeData(data, end_stream); }, - remote_closed_, code, body, grpc_status, is_head_request_); + remote_closed_, + Utility::EncodeFunctions{ + [this, modify_headers](HeaderMapPtr&& headers, bool end_stream) -> void { + if (modify_headers != nullptr) { + modify_headers(*headers); + } + encodeHeaders(std::move(headers), end_stream); + }, + [this](Buffer::Instance& data, bool end_stream) -> void { + encodeData(data, end_stream); + }}, + Utility::LocalReplyData{is_grpc_request_, code, body, grpc_status, is_head_request_}); } // The async client won't pause if sending an Expect: 100-Continue so simply // swallows any incoming encode100Continue. diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index f27c704bde33c..cca066fea7cd0 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -1352,36 +1352,38 @@ void ConnectionManagerImpl::ActiveStream::sendLocalReply( stream_info_.setResponseCodeDetails(details); Utility::sendLocalReply( - is_grpc_request, - [this, modify_headers](HeaderMapPtr&& headers, bool end_stream) -> void { - if (modify_headers != nullptr) { - modify_headers(*headers); - } - response_headers_ = std::move(headers); - // TODO: Start encoding from the last decoder filter that saw the - // request instead. - encodeHeaders(nullptr, *response_headers_, end_stream); - }, - [this](Buffer::Instance& data, bool end_stream) -> void { - // TODO: Start encoding from the last decoder filter that saw the - // request instead. - encodeData(nullptr, data, end_stream, FilterIterationStartState::CanStartFromCurrent); - }, - [this](Code& code) -> void { - connection_manager_.config_.localReply()->matchAndRewrite( - request_headers_.get(), response_headers_.get(), response_trailers_.get(), stream_info_, - code); - }, - [this](HeaderMapPtr&& headers, absl::string_view& body_text) -> std::string { - std::string formatted_body = connection_manager_.config_.localReply()->format( - request_headers_.get(), response_headers_.get(), response_trailers_.get(), stream_info_, - body_text); - - connection_manager_.config_.localReply()->insertContentHeaders(formatted_body, - headers.get()); - return formatted_body; - }, - state_.destroyed_, code, body, grpc_status, is_head_request); + state_.destroyed_, + Utility::EncodeFunctions{ + [this, modify_headers](HeaderMapPtr&& headers, bool end_stream) -> void { + if (modify_headers != nullptr) { + modify_headers(*headers); + } + response_headers_ = std::move(headers); + // TODO: Start encoding from the last decoder filter that saw the + // request instead. + encodeHeaders(nullptr, *response_headers_, end_stream); + }, + [this](Buffer::Instance& data, bool end_stream) -> void { + // TODO: Start encoding from the last decoder filter that saw the + // request instead. + encodeData(nullptr, data, end_stream, FilterIterationStartState::CanStartFromCurrent); + }}, + Utility::RewriteReplyFunctions{ + [this](Code& code) -> void { + connection_manager_.config_.localReply()->matchAndRewrite( + request_headers_.get(), response_headers_.get(), response_trailers_.get(), + stream_info_, code); + }, + [this](HeaderMapPtr&& headers, absl::string_view& body_text) -> std::string { + std::string formatted_body = connection_manager_.config_.localReply()->format( + request_headers_.get(), response_headers_.get(), response_trailers_.get(), + stream_info_, body_text); + + connection_manager_.config_.localReply()->insertContentHeaders(formatted_body, + headers.get()); + return formatted_body; + }}, + Utility::LocalReplyData{is_grpc_request, code, body, grpc_status, is_head_request}); } void ConnectionManagerImpl::ActiveStream::encode100ContinueHeaders( @@ -2305,34 +2307,38 @@ void ConnectionManagerImpl::ActiveStreamEncoderFilter::responseDataTooLarge() { parent_.stream_info_.setResponseCodeDetails( StreamInfo::ResponseCodeDetails::get().RequestHeadersTooLarge); Http::Utility::sendLocalReply( - Grpc::Common::hasGrpcContentType(*parent_.request_headers_), - [&](HeaderMapPtr&& response_headers, bool end_stream) -> void { - parent_.chargeStats(*response_headers); - parent_.response_headers_ = std::move(response_headers); - parent_.response_encoder_->encodeHeaders(*parent_.response_headers_, end_stream); - parent_.state_.local_complete_ = end_stream; - }, - [&](Buffer::Instance& data, bool end_stream) -> void { - parent_.response_encoder_->encodeData(data, end_stream); - parent_.state_.local_complete_ = end_stream; - }, - [&](Code& code) -> void { - parent_.connection_manager_.config_.localReply()->matchAndRewrite( - parent_.request_headers_.get(), parent_.response_headers_.get(), - parent_.response_trailers_.get(), parent_.stream_info_, code); - }, - [&](HeaderMapPtr&& headers, absl::string_view body_text) -> std::string { - std::string formatted_body = parent_.connection_manager_.config_.localReply()->format( - parent_.request_headers_.get(), parent_.response_headers_.get(), - parent_.response_trailers_.get(), parent_.stream_info_, body_text); - - parent_.connection_manager_.config_.localReply()->insertContentHeaders(formatted_body, - headers.get()); - return formatted_body; - }, - parent_.state_.destroyed_, Http::Code::InternalServerError, - CodeUtility::toString(Http::Code::InternalServerError), absl::nullopt, - parent_.is_head_request_); + parent_.state_.destroyed_, + Utility::EncodeFunctions{[&](HeaderMapPtr&& response_headers, bool end_stream) -> void { + parent_.chargeStats(*response_headers); + parent_.response_headers_ = std::move(response_headers); + parent_.response_encoder_->encodeHeaders( + *parent_.response_headers_, end_stream); + parent_.state_.local_complete_ = end_stream; + }, + [&](Buffer::Instance& data, bool end_stream) -> void { + parent_.response_encoder_->encodeData(data, end_stream); + parent_.state_.local_complete_ = end_stream; + }}, + Utility::RewriteReplyFunctions{ + [&](Code& code) -> void { + parent_.connection_manager_.config_.localReply()->matchAndRewrite( + parent_.request_headers_.get(), parent_.response_headers_.get(), + parent_.response_trailers_.get(), parent_.stream_info_, code); + }, + [&](HeaderMapPtr&& headers, absl::string_view body_text) -> std::string { + std::string formatted_body = + parent_.connection_manager_.config_.localReply()->format( + parent_.request_headers_.get(), parent_.response_headers_.get(), + parent_.response_trailers_.get(), parent_.stream_info_, body_text); + + parent_.connection_manager_.config_.localReply()->insertContentHeaders( + formatted_body, headers.get()); + return formatted_body; + }}, + Utility::LocalReplyData{Grpc::Common::hasGrpcContentType(*parent_.request_headers_), + Http::Code::InternalServerError, + CodeUtility::toString(Http::Code::InternalServerError), + absl::nullopt, parent_.is_head_request_}); parent_.maybeEndEncode(parent_.state_.local_complete_); } else { ENVOY_STREAM_LOG( diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index ac7429140c42e..d5e27debf1f6c 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -286,87 +286,81 @@ Utility::parseHttp1Settings(const envoy::api::v2::core::Http1ProtocolOptions& co return ret; } -void Utility::sendLocalReply(bool is_grpc, StreamDecoderFilterCallbacks& callbacks, - const bool& is_reset, Code response_code, absl::string_view body_text, - const absl::optional grpc_status, - bool is_head_request) { - sendLocalReply( - is_grpc, - [&](HeaderMapPtr&& headers, bool end_stream) -> void { - callbacks.encodeHeaders(std::move(headers), end_stream); - }, - [&](Buffer::Instance& data, bool end_stream) -> void { - callbacks.encodeData(data, end_stream); - }, - is_reset, response_code, body_text, grpc_status, is_head_request); -} - -void Utility::sendLocalReply( - bool is_grpc, std::function encode_headers, - std::function encode_data, const bool& is_reset, - Code response_code, absl::string_view body_text, - const absl::optional grpc_status, bool is_head_request) { - sendLocalReply( - is_grpc, encode_headers, encode_data, [&](Code&) -> void {}, - [&](HeaderMapPtr&& response_headers, absl::string_view& body) -> std::string { - if (!body.empty()) { - response_headers->insertContentLength().value(body.size()); - response_headers->insertContentType().value(Headers::get().ContentTypeValues.Text); - return body.data(); - } - return {}; - }, - is_reset, response_code, body_text, grpc_status, is_head_request); -} - -void Utility::sendLocalReply( - bool is_grpc, std::function encode_headers, - std::function encode_data, - std::function rewrite_response, - std::function - format_response, - const bool& is_reset, Code response_code, absl::string_view body_text, - const absl::optional grpc_status, bool is_head_request) { +void Utility::sendLocalReply(const bool& is_reset, StreamDecoderFilterCallbacks& callbacks, + LocalReplyData local_reply_data) { + sendLocalReply(is_reset, + Utility::EncodeFunctions{[&](HeaderMapPtr&& headers, bool end_stream) -> void { + callbacks.encodeHeaders(std::move(headers), end_stream); + }, + [&](Buffer::Instance& data, bool end_stream) -> void { + callbacks.encodeData(data, end_stream); + }}, + local_reply_data); +} + +void Utility::sendLocalReply(const bool& is_reset, EncodeFunctions encode_functions, + LocalReplyData local_reply_data) { + sendLocalReply(is_reset, encode_functions, + RewriteReplyFunctions{ + [&](Code&) -> void {}, + [&](HeaderMapPtr&& response_headers, absl::string_view& body) -> std::string { + if (!body.empty()) { + response_headers->insertContentLength().value(body.size()); + response_headers->insertContentType().value( + Headers::get().ContentTypeValues.Text); + return body.data(); + } + return {}; + }}, + local_reply_data); +} + +void Utility::sendLocalReply(const bool& is_reset, EncodeFunctions encode_functions, + RewriteReplyFunctions rewrite_reply_functions, + LocalReplyData local_reply_data) { // encode_headers() may reset the stream, so the stream must not be reset before calling it. ASSERT(!is_reset); // rewrite_response will rewrite response code (more might be added in future) - rewrite_response(response_code); + rewrite_reply_functions.rewrite_response_(local_reply_data.response_code_); // Respond with a gRPC trailers-only response if the request is gRPC - if (is_grpc) { + if (local_reply_data.is_grpc_) { HeaderMapPtr response_headers{new HeaderMapImpl{ {Headers::get().Status, std::to_string(enumToInt(Code::OK))}, {Headers::get().ContentType, Headers::get().ContentTypeValues.Grpc}, {Headers::get().GrpcStatus, - std::to_string( - enumToInt(grpc_status ? grpc_status.value() - : Grpc::Utility::httpToGrpcStatus(enumToInt(response_code))))}}}; - if (!body_text.empty() && !is_head_request) { + std::to_string(enumToInt( + local_reply_data.grpc_status_ + ? local_reply_data.grpc_status_.value() + : Grpc::Utility::httpToGrpcStatus(enumToInt(local_reply_data.response_code_))))}}}; + if (!local_reply_data.body_text_.empty() && !local_reply_data.is_head_request_) { // TODO(dio): Probably it is worth to consider caching the encoded message based on gRPC // status. - response_headers->insertGrpcMessage().value(PercentEncoding::encode(body_text)); + response_headers->insertGrpcMessage().value( + PercentEncoding::encode(local_reply_data.body_text_)); } - encode_headers(std::move(response_headers), true); // Trailers only response + encode_functions.encode_headers_(std::move(response_headers), true); // Trailers only response return; } - HeaderMapPtr response_headers{ - new HeaderMapImpl{{Headers::get().Status, std::to_string(enumToInt(response_code))}}}; + HeaderMapPtr response_headers{new HeaderMapImpl{ + {Headers::get().Status, std::to_string(enumToInt(local_reply_data.response_code_))}}}; std::string formatted_body{}; - if (!body_text.empty()) { - formatted_body = format_response(std::move(response_headers), body_text); + if (!local_reply_data.body_text_.empty()) { + formatted_body = rewrite_reply_functions.format_response_(std::move(response_headers), + local_reply_data.body_text_); } - if (is_head_request) { - encode_headers(std::move(response_headers), true); + if (local_reply_data.is_head_request_) { + encode_functions.encode_headers_(std::move(response_headers), true); return; } - encode_headers(std::move(response_headers), formatted_body.empty()); + encode_functions.encode_headers_(std::move(response_headers), formatted_body.empty()); // encode_headers()) may have changed the referenced is_reset so we need to test it if (!formatted_body.empty() && !is_reset) { Buffer::OwnedImpl buffer(formatted_body); - encode_data(buffer, true); + encode_functions.encode_data_(buffer, true); } } diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 1b24f9046d786..3262d641dce22 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -175,69 +175,68 @@ Http2Settings parseHttp2Settings(const envoy::api::v2::core::Http2ProtocolOption */ Http1Settings parseHttp1Settings(const envoy::api::v2::core::Http1ProtocolOptions& config); +struct EncodeFunctions { + // Function to encode response headers. + std::function encode_headers_; + // Function to encode the response body. + std::function encode_data_; +}; + +struct RewriteReplyFunctions { + // Function to rewrite locally generated response e.g. change status code. + std::function rewrite_response_; + // Function to change format of locally generated reply. + std::function format_response_; +}; + +struct LocalReplyData { + // Tells if this is a response to a gRPC request. + bool is_grpc_; + // Supplies the HTTP response code. + Code response_code_; + // Supplies the optional body text which is returned. + absl::string_view body_text_; + // gRPC status code to override the httpToGrpcStatus mapping with. + const absl::optional grpc_status_; + // Tells if this is a response to a HEAD request. + bool is_head_request_ = false; +}; + /** * Create a locally generated response using filter callbacks. - * @param is_grpc tells if this is a response to a gRPC request. - * @param callbacks supplies the filter callbacks to use. * @param is_reset boolean reference that indicates whether a stream has been reset. It is the - * responsibility of the caller to ensure that this is set to false if onDestroy() - * is invoked in the context of sendLocalReply(). - * @param response_code supplies the HTTP response code. - * @param body_text supplies the optional body text which is sent using the text/plain content - * type. - * @param grpc_status the gRPC status code to override the httpToGrpcStatus mapping with. - * @param is_head_request tells if this is a response to a HEAD request + * responsibility of the caller to ensure that this is set to false if onDestroy() + * is invoked in the context of sendLocalReply(). + * @param callbacks supplies the filter callbacks to use. + * @param local_reply_data struct which keeps data related to generate reply. */ -void sendLocalReply(bool is_grpc, StreamDecoderFilterCallbacks& callbacks, const bool& is_reset, - Code response_code, absl::string_view body_text, - const absl::optional grpc_status, - bool is_head_request); +void sendLocalReply(const bool& is_reset, StreamDecoderFilterCallbacks& callbacks, + LocalReplyData local_reply_data); /** * Create a locally generated response using the provided lambdas. - * @param is_grpc tells if this is a response to a gRPC request. - * @param encode_headers supplies the function to encode response headers. - * @param encode_data supplies the function to encode the response body. + * @param is_reset boolean reference that indicates whether a stream has been reset. It is the * responsibility of the caller to ensure that this is set to false if onDestroy() * is invoked in the context of sendLocalReply(). - * @param response_code supplies the HTTP response code. - * @param body_text supplies the optional body text which is sent using the text/plain content - * type. - * @param grpc_status the gRPC status code to override the httpToGrpcStatus mapping with. + * @param encode_functions supplies the functions to encode response body and headers. + * @param local_reply_data struct which keeps data related to generate reply. */ -void sendLocalReply(bool is_grpc, - std::function encode_headers, - std::function encode_data, - const bool& is_reset, Code response_code, absl::string_view body_text, - const absl::optional grpc_status, - bool is_head_request = false); +void sendLocalReply(const bool& is_reset, EncodeFunctions encode_data, + LocalReplyData local_reply_data); /** * Create a locally generated response using the provided lambdas with rewrite function. - * @param is_grpc tells if this is a response to a gRPC request. - * @param encode_headers supplies the function to encode response headers. - * @param encode_data supplies the function to encode the response body. - * @param rewrite_response supplies the function to rewrite locally generated response - * e.g. change status code - * @param format_response supplies the function to change format of locally generated reply * @param is_reset boolean reference that indicates whether a stream has been reset. It is the * responsibility of the caller to ensure that this is set to false if onDestroy() * is invoked in the context of sendLocalReply(). - * @param response_code supplies the HTTP response code. - * @param body_text supplies the optional body text which is sent using the text/plain content - * type. - * @param grpc_status the gRPC status code to override the httpToGrpcStatus mapping with. - */ -void sendLocalReply(bool is_grpc, - std::function encode_headers, - std::function encode_data, - std::function rewrite_response, - std::function - format_response, - const bool& is_reset, Code response_code, absl::string_view body_text, - const absl::optional grpc_status, - bool is_head_request = false); + * @param encode_functions supplies the functions to encode response body and headers. + * @param rewrite_reply_functions supplies the function to rewrite and change format of locally + * generated reply. + * @param local_reply_data struct which keeps data related to generate reply. + */ +void sendLocalReply(const bool& is_reset, EncodeFunctions encode_functions, + RewriteReplyFunctions rewrite_reply_functions, LocalReplyData local_reply_data); struct GetLastAddressFromXffInfo { // Last valid address pulled from the XFF header. diff --git a/source/common/local_reply/local_reply.cc b/source/common/local_reply/local_reply.cc index 9bc1b30047ded..18eee8e7d0b28 100644 --- a/source/common/local_reply/local_reply.cc +++ b/source/common/local_reply/local_reply.cc @@ -12,13 +12,13 @@ ResponseRewriter::ResponseRewriter(absl::optional response_code) : response_code_(response_code) {} void ResponseRewriter::rewrite(Http::Code& code) { - code = static_cast(response_code_.value()); + if (response_code_.has_value()) { + code = static_cast(response_code_.value()); + } } -ResponseMapper::ResponseMapper(AccessLog::FilterPtr&& filter, ResponseRewriterPtr&& rewriter) { - filter_ = std::move(filter); - rewriter_ = std::move(rewriter); -} +ResponseMapper::ResponseMapper(AccessLog::FilterPtr&& filter, ResponseRewriterPtr&& rewriter) + : filter_(std::move(filter)), rewriter_(std::move(rewriter)) {} bool ResponseMapper::match(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_headers, @@ -30,11 +30,8 @@ bool ResponseMapper::match(const Http::HeaderMap* request_headers, void ResponseMapper::rewrite(Http::Code& status_code) { rewriter_->rewrite(status_code); } LocalReply::LocalReply(std::list mappers, AccessLog::FormatterPtr&& formatter, - std::string content_type) { - mappers_ = std::move(mappers); - formatter_ = std::move(formatter); - content_type_ = content_type; -} + std::string content_type) + : mappers_(std::move(mappers)), formatter_(std::move(formatter)), content_type_(content_type) {} void LocalReply::matchAndRewrite(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_headers, diff --git a/test/common/access_log/access_log_formatter_test.cc b/test/common/access_log/access_log_formatter_test.cc index 9f03c0c8bc8e0..5f48f68edcfd7 100644 --- a/test/common/access_log/access_log_formatter_test.cc +++ b/test/common/access_log/access_log_formatter_test.cc @@ -763,21 +763,6 @@ TEST(AccessLogFormatterTest, bodyFormatterTest) { } } -void verifyJsonOutput(std::string json_string, - std::unordered_map expected_map) { - const auto parsed = Json::Factory::loadFromString(json_string); - - // Every json log line should have only one newline character, and it should be the last character - // in the string - const auto newline_pos = json_string.find('\n'); - EXPECT_NE(newline_pos, std::string::npos); - EXPECT_EQ(newline_pos, json_string.length() - 1); - - for (const auto& pair : expected_map) { - EXPECT_EQ(parsed->getString(pair.first), pair.second); - } -} - TEST(AccessLogFormatterTest, JsonFormatterPlainStringTest) { StreamInfo::MockStreamInfo stream_info; Http::TestHeaderMapImpl request_header; @@ -797,7 +782,7 @@ TEST(AccessLogFormatterTest, JsonFormatterPlainStringTest) { {"plain_string", "plain_string_value"}}; JsonFormatterImpl formatter(key_mapping); - verifyJsonOutput( + TestUtility::verifyJsonOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } @@ -819,7 +804,7 @@ TEST(AccessLogFormatterTest, JsonFormatterSingleOperatorTest) { std::unordered_map key_mapping = {{"protocol", "%PROTOCOL%"}}; JsonFormatterImpl formatter(key_mapping); - verifyJsonOutput( + TestUtility::verifyJsonOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } @@ -847,7 +832,7 @@ TEST(AccessLogFormatterTest, JsonFormatterNonExistentHeaderTest) { absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyJsonOutput( + TestUtility::verifyJsonOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } @@ -879,7 +864,7 @@ TEST(AccessLogFormatterTest, JsonFormatterAlternateHeaderTest) { absl::optional protocol = Http::Protocol::Http11; EXPECT_CALL(stream_info, protocol()).WillRepeatedly(Return(protocol)); - verifyJsonOutput( + TestUtility::verifyJsonOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } @@ -908,7 +893,7 @@ TEST(AccessLogFormatterTest, JsonFormatterDynamicMetadataTest) { JsonFormatterImpl formatter(key_mapping); - verifyJsonOutput( + TestUtility::verifyJsonOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } @@ -939,7 +924,7 @@ TEST(AccessLogFormatterTest, JsonFormatterStartTimeTest) { {"all_zeroes", "%START_TIME(%f.%1f.%2f.%3f)%"}}; JsonFormatterImpl formatter(key_mapping); - verifyJsonOutput( + TestUtility::verifyJsonOutput( formatter.format(request_header, response_header, response_trailer, stream_info, body), expected_json_map); } diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index a9f96f315aebd..54a1548182070 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -444,8 +444,9 @@ TEST(HttpUtility, SendLocalReply) { EXPECT_CALL(callbacks, encodeHeaders_(_, false)); EXPECT_CALL(callbacks, encodeData(_, true)); - Utility::sendLocalReply(false, callbacks, is_reset, Http::Code::PayloadTooLarge, "large", - absl::nullopt, false); + Utility::sendLocalReply( + is_reset, callbacks, + Utility::LocalReplyData{false, Http::Code::PayloadTooLarge, "large", absl::nullopt, false}); } TEST(HttpUtility, SendLocalGrpcReply) { @@ -461,8 +462,9 @@ TEST(HttpUtility, SendLocalGrpcReply) { EXPECT_NE(headers.GrpcMessage(), nullptr); EXPECT_EQ(headers.GrpcMessage()->value().getStringView(), "large"); })); - Utility::sendLocalReply(true, callbacks, is_reset, Http::Code::PayloadTooLarge, "large", - absl::nullopt, false); + Utility::sendLocalReply( + is_reset, callbacks, + Utility::LocalReplyData{true, Http::Code::PayloadTooLarge, "large", absl::nullopt, false}); } TEST(HttpUtility, SendLocalGrpcReplyWithUpstreamJsonPayload) { @@ -488,8 +490,9 @@ TEST(HttpUtility, SendLocalGrpcReplyWithUpstreamJsonPayload) { const auto& encoded = Utility::PercentEncoding::encode(json); EXPECT_EQ(headers.GrpcMessage()->value().getStringView(), encoded); })); - Utility::sendLocalReply(true, callbacks, is_reset, Http::Code::Unauthorized, json, absl::nullopt, - false); + Utility::sendLocalReply( + is_reset, callbacks, + Utility::LocalReplyData{true, Http::Code::Unauthorized, json, absl::nullopt, false}); } TEST(HttpUtility, RateLimitedGrpcStatus) { @@ -501,8 +504,9 @@ TEST(HttpUtility, RateLimitedGrpcStatus) { EXPECT_EQ(headers.GrpcStatus()->value().getStringView(), std::to_string(enumToInt(Grpc::Status::GrpcStatus::Unavailable))); })); - Utility::sendLocalReply(true, callbacks, false, Http::Code::TooManyRequests, "", absl::nullopt, - false); + Utility::sendLocalReply( + false, callbacks, + Utility::LocalReplyData{true, Http::Code::TooManyRequests, "", absl::nullopt, false}); EXPECT_CALL(callbacks, encodeHeaders_(_, true)) .WillOnce(Invoke([&](const HeaderMap& headers, bool) -> void { @@ -510,10 +514,11 @@ TEST(HttpUtility, RateLimitedGrpcStatus) { EXPECT_EQ(headers.GrpcStatus()->value().getStringView(), std::to_string(enumToInt(Grpc::Status::GrpcStatus::ResourceExhausted))); })); - Utility::sendLocalReply( - true, callbacks, false, Http::Code::TooManyRequests, "", - absl::make_optional(Grpc::Status::GrpcStatus::ResourceExhausted), - false); + Utility::sendLocalReply(false, callbacks, + Utility::LocalReplyData{true, Http::Code::TooManyRequests, "", + absl::make_optional( + Grpc::Status::GrpcStatus::ResourceExhausted), + false}); } TEST(HttpUtility, SendLocalReplyDestroyedEarly) { @@ -524,8 +529,9 @@ TEST(HttpUtility, SendLocalReplyDestroyedEarly) { is_reset = true; })); EXPECT_CALL(callbacks, encodeData(_, true)).Times(0); - Utility::sendLocalReply(false, callbacks, is_reset, Http::Code::PayloadTooLarge, "large", - absl::nullopt, false); + Utility::sendLocalReply( + is_reset, callbacks, + Utility::LocalReplyData{false, Http::Code::PayloadTooLarge, "large", absl::nullopt, false}); } TEST(HttpUtility, SendLocalReplyHeadRequest) { @@ -536,8 +542,9 @@ TEST(HttpUtility, SendLocalReplyHeadRequest) { EXPECT_EQ(headers.ContentLength()->value().getStringView(), fmt::format("{}", strlen("large"))); })); - Utility::sendLocalReply(false, callbacks, is_reset, Http::Code::PayloadTooLarge, "large", - absl::nullopt, true); + Utility::sendLocalReply( + is_reset, callbacks, + Utility::LocalReplyData{false, Http::Code::PayloadTooLarge, "large", absl::nullopt, true}); } TEST(HttpUtility, TestExtractHostPathFromUri) { diff --git a/test/common/local_reply/BUILD b/test/common/local_reply/BUILD new file mode 100644 index 0000000000000..1b0b22d9ff62e --- /dev/null +++ b/test/common/local_reply/BUILD @@ -0,0 +1,20 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +envoy_package() + +envoy_cc_test( + name = "local_reply_test", + srcs = ["local_reply_test.cc"], + deps = [ + "//source/common/common:assert_lib", + "//source/common/local_reply:local_reply_lib", + "//test/mocks/http:http_mocks", + "//test/mocks/stream_info:stream_info_mocks", + ], +) diff --git a/test/common/local_reply/local_reply_test.cc b/test/common/local_reply/local_reply_test.cc new file mode 100644 index 0000000000000..9c280aabfa525 --- /dev/null +++ b/test/common/local_reply/local_reply_test.cc @@ -0,0 +1,82 @@ +#include "envoy/http/codes.h" + +#include "common/local_reply/local_reply.h" + +#include "test/mocks/http/mocks.h" + +#include "gmock/gmock.h" + +using testing::_; + +namespace Envoy { +namespace LocalReply { + +class ResponseRewriterTest : public testing::Test { +public: + ResponseRewriterTest() {} +}; + +TEST_F(ResponseRewriterTest, ShouldChangeResponseCode) { + // given + absl::optional response_code(504); + ResponseRewriterPtr response_rewriter = std::make_unique(response_code); + Http::Code code{Http::Code::OK}; + + // when + response_rewriter->rewrite(code); + + // then + EXPECT_EQ(code, Http::Code::GatewayTimeout); +} + +TEST_F(ResponseRewriterTest, ShouldNotChangeResponseCode) { + // given + ResponseRewriterPtr response_rewriter = std::make_unique(absl::nullopt); + Http::Code code{Http::Code::OK}; + + // when + response_rewriter->rewrite(code); + + // then + EXPECT_EQ(code, Http::Code::OK); +} + +class LocalReplyTest : public testing::Test { +public: + LocalReplyTest() { formatter_ = std::make_unique("plain"); } + + Envoy::AccessLog::FormatterPtr formatter_; + NiceMock stream_info_; + Http::TestHeaderMapImpl headers_; + Http::Code code_; +}; + +TEST_F(LocalReplyTest, ShouldInsertContentLengthAndTypeHeaders) { + // given + LocalReplyPtr local_reply = std::make_unique( + LocalReply{{}, std::move(formatter_), Http::Headers::get().ContentTypeValues.Text}); + absl::string_view body = "test body"; + + // when + local_reply->insertContentHeaders(body, &headers_); + // then + + EXPECT_EQ(headers_.ContentLength()->value().getStringView(), "9"); + EXPECT_EQ(headers_.ContentType()->value().getStringView(), "text/plain"); +} + +TEST_F(LocalReplyTest, ShouldNotInsertContentHeadersForEmptyBody) { + // given + LocalReplyPtr local_reply = std::make_unique( + LocalReply{{}, std::move(formatter_), Http::Headers::get().ContentTypeValues.Text}); + absl::string_view body = {}; + + // when + local_reply->insertContentHeaders(body, &headers_); + + // then + EXPECT_TRUE(headers_.ContentLength() == nullptr); + EXPECT_TRUE(headers_.ContentType() == nullptr); +} +} // namespace LocalReply +} // namespace Envoy \ No newline at end of file diff --git a/test/config/utility.cc b/test/config/utility.cc index 29717682558e6..b716751b865c0 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -745,6 +745,17 @@ void ConfigHelper::setOutboundFramesLimits(uint32_t max_all_frames, uint32_t max } } +void ConfigHelper::setLocalReply( + envoy::config::filter::network::http_connection_manager::v2::LocalReplyConfig& config) { + auto filter = getFilterFromListener("envoy.http_connection_manager"); + if (filter) { + envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager hcm_config; + loadHttpConnectionManager(hcm_config); + hcm_config.mutable_local_reply_config()->MergeFrom(config); + storeHttpConnectionManager(hcm_config); + } +} + CdsHelper::CdsHelper() : cds_path_(TestEnvironment::writeStringToFileForTest("cds.pb_text", "")) {} void CdsHelper::setCds(const std::vector& clusters) { diff --git a/test/config/utility.h b/test/config/utility.h index 8359fa0edeb1b..e535d06f38010 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -169,6 +169,9 @@ class ConfigHelper { // configuration generated in ConfigHelper::finalize. void skipPortUsageValidation() { skip_port_usage_validation_ = true; } + void setLocalReply( + envoy::config::filter::network::http_connection_manager::v2::LocalReplyConfig& config); + private: // Load the first HCM struct from the first listener into a parsed proto. bool loadHttpConnectionManager( diff --git a/test/integration/BUILD b/test/integration/BUILD index 8ed3846c21b5b..7c88e4741ecb3 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -922,3 +922,15 @@ envoy_cc_test( "//test/test_common:utility_lib", ], ) + +envoy_cc_test( + name = "local_reply_integration_test", + srcs = [ + "local_reply_integration_test.cc", + ], + deps = [ + ":http_integration_lib", + ":http_protocol_integration_lib", + "//test/test_common:utility_lib", + ], +) diff --git a/test/integration/local_reply_integration_test.cc b/test/integration/local_reply_integration_test.cc new file mode 100644 index 0000000000000..8c48f62e980d3 --- /dev/null +++ b/test/integration/local_reply_integration_test.cc @@ -0,0 +1,465 @@ +#include "test/integration/http_protocol_integration.h" + +namespace Envoy { + +class LocalReplyIntegrationTest : public HttpProtocolIntegrationTest { +public: + void initialize() override { HttpProtocolIntegrationTest::initialize(); } + + void setLocalReplyConfig(const std::string& yaml) { + envoy::config::filter::network::http_connection_manager::v2::LocalReplyConfig + local_reply_config; + TestUtility::loadFromYaml(yaml, local_reply_config); + config_helper_.setLocalReply(local_reply_config); + } +}; + +INSTANTIATE_TEST_SUITE_P(Protocols, LocalReplyIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams()), + HttpProtocolIntegrationTest::protocolTestParamsToString); + +TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson) { + const std::string yaml = R"EOF( +mapper: + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value + rewriter: + status_code: 550 +format: + json_format: + level: TRACE + user_agent: "%REQ(USER-AGENT)%" + response_body: "%RESP_BODY%" + )EOF"; + setLocalReplyConfig(yaml); + initialize(); + + std::unordered_map expected_json_map = { + {"level", "TRACE"}, + {"user_agent", "-"}, + {"response_body", "upstream connect error or disconnect/reset before headers. reset reason: " + "connection termination"}}; + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"test-header", "exact-match-value"}}); + auto response = std::move(encoder_decoder.second); + + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + response->waitForEndStream(); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + } else { + codec_client_->close(); + } + + EXPECT_FALSE(upstream_request_->complete()); + EXPECT_EQ(0U, upstream_request_->bodyLength()); + + EXPECT_TRUE(response->complete()); + EXPECT_EQ("application/json", response->headers().ContentType()->value().getStringView()); + EXPECT_EQ("149", response->headers().ContentLength()->value().getStringView()); + EXPECT_EQ("550", response->headers().Status()->value().getStringView()); + // Check if returned json is same as expected + TestUtility::verifyJsonOutput(response->body(), expected_json_map); +} + +// Should map to first matching filter in this case it will be 2nd filter +TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJsonForFirstMatchingFilter) { + const std::string yaml = R"EOF( +mapper: + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value-1 + rewriter: + status_code: 550 + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value + rewriter: + status_code: 551 + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value + rewriter: + status_code: 552 +format: + json_format: + level: TRACE + response_flags: "%RESPONSE_FLAGS%" + response_body: "%RESP_BODY%" + )EOF"; + setLocalReplyConfig(yaml); + initialize(); + + std::unordered_map expected_json_map = { + {"level", "TRACE"}, + {"response_flags", "UC"}, + {"response_body", "upstream connect error or disconnect/reset before headers. reset reason: " + "connection termination"}}; + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"test-header", "exact-match-value"}}); + auto response = std::move(encoder_decoder.second); + + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + response->waitForEndStream(); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + } else { + codec_client_->close(); + } + + EXPECT_FALSE(upstream_request_->complete()); + EXPECT_EQ(0U, upstream_request_->bodyLength()); + + EXPECT_TRUE(response->complete()); + EXPECT_EQ("application/json", response->headers().ContentType()->value().getStringView()); + EXPECT_EQ("154", response->headers().ContentLength()->value().getStringView()); + EXPECT_EQ("551", response->headers().Status()->value().getStringView()); + // Check if returned json is same as expected + TestUtility::verifyJsonOutput(response->body(), expected_json_map); +} + +TEST_P(LocalReplyIntegrationTest, ShouldNotMatchAnyFilter) { + const std::string yaml = R"EOF( +mapper: + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value-1 + rewriter: + status_code: 550 + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value-2 + rewriter: + status_code: 551 + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value-3 + rewriter: + status_code: 552 +format: + json_format: + level: TRACE + response_flags: "%RESPONSE_FLAGS%" + response_body: "%RESP_BODY%" + )EOF"; + setLocalReplyConfig(yaml); + initialize(); + + std::unordered_map expected_json_map = { + {"level", "TRACE"}, + {"response_flags", "UC"}, + {"response_body", "upstream connect error or disconnect/reset before headers. reset reason: " + "connection termination"}}; + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"test-header", "exact-match-value"}}); + auto response = std::move(encoder_decoder.second); + + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + response->waitForEndStream(); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + } else { + codec_client_->close(); + } + + EXPECT_FALSE(upstream_request_->complete()); + EXPECT_EQ(0U, upstream_request_->bodyLength()); + + EXPECT_TRUE(response->complete()); + EXPECT_EQ("application/json", response->headers().ContentType()->value().getStringView()); + EXPECT_EQ("154", response->headers().ContentLength()->value().getStringView()); + EXPECT_EQ("503", response->headers().Status()->value().getStringView()); + // Check if returned json is same as expected + TestUtility::verifyJsonOutput(response->body(), expected_json_map); +} + +// Should map response code and return response application/text without any changes. +TEST_P(LocalReplyIntegrationTest, ShouldMapResponseCodeAndMapToDefaultTextResponse) { + const std::string yaml = R"EOF( +mapper: + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value-1 + rewriter: + status_code: 550 + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value-2 + rewriter: + status_code: 551 + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value-3 + rewriter: + status_code: 552 + )EOF"; + setLocalReplyConfig(yaml); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"test-header", "exact-match-value-2"}}); + auto response = std::move(encoder_decoder.second); + + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + response->waitForEndStream(); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + } else { + codec_client_->close(); + } + + EXPECT_FALSE(upstream_request_->complete()); + EXPECT_EQ(0U, upstream_request_->bodyLength()); + + EXPECT_TRUE(response->complete()); + EXPECT_EQ("text/plain", response->headers().ContentType()->value().getStringView()); + EXPECT_EQ("95", response->headers().ContentLength()->value().getStringView()); + + EXPECT_EQ("551", response->headers().Status()->value().getStringView()); + + EXPECT_EQ(response->body(), "upstream connect error or disconnect/reset before headers. reset " + "reason: connection termination"); +} + +// Should map response code and return response text/plain with custom format. +TEST_P(LocalReplyIntegrationTest, ShouldMapResponseCodeAndMapToCustomTextResponse) { + const std::string yaml = R"EOF( +mapper: + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value-1 + rewriter: + status_code: 550 + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value-2 + rewriter: + status_code: 551 + - filter: + header_filter: + header: + name: test-header + exact_match: exact-match-value-3 + rewriter: + status_code: 552 +format: + string_format: "%RESPONSE_FLAGS% - %RESP_BODY% - custom response" +)EOF"; + setLocalReplyConfig(yaml); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"test-header", "exact-match-value-2"}}); + auto response = std::move(encoder_decoder.second); + + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + response->waitForEndStream(); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + } else { + codec_client_->close(); + } + + EXPECT_FALSE(upstream_request_->complete()); + EXPECT_EQ(0U, upstream_request_->bodyLength()); + + EXPECT_TRUE(response->complete()); + + EXPECT_EQ("text/plain", response->headers().ContentType()->value().getStringView()); + EXPECT_EQ("118", response->headers().ContentLength()->value().getStringView()); + + EXPECT_EQ("551", response->headers().Status()->value().getStringView()); + + EXPECT_EQ(response->body(), "UC - upstream connect error or disconnect/reset before headers. " + "reset reason: connection termination - custom response"); +} + +// Should return formatted text/plain response. +TEST_P(LocalReplyIntegrationTest, ShouldFormatResponseToCustomString) { + const std::string yaml = R"EOF( +format: + string_format: "%RESPONSE_FLAGS% - %RESP_BODY% - custom response" +)EOF"; + setLocalReplyConfig(yaml); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"test-header", "exact-match-value-2"}}); + auto response = std::move(encoder_decoder.second); + + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + response->waitForEndStream(); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + } else { + codec_client_->close(); + } + + EXPECT_FALSE(upstream_request_->complete()); + EXPECT_EQ(0U, upstream_request_->bodyLength()); + + EXPECT_TRUE(response->complete()); + + EXPECT_EQ("text/plain", response->headers().ContentType()->value().getStringView()); + EXPECT_EQ("118", response->headers().ContentLength()->value().getStringView()); + + EXPECT_EQ("503", response->headers().Status()->value().getStringView()); + + EXPECT_EQ(response->body(), "UC - upstream connect error or disconnect/reset before headers. " + "reset reason: connection termination - custom response"); +} + +// Should return formatted application/json response. +TEST_P(LocalReplyIntegrationTest, ShouldFormatResponseToJson) { + const std::string yaml = R"EOF( +format: + json_format: + level: TRACE + response_flags: "%RESPONSE_FLAGS%" + response_body: "%RESP_BODY%" + )EOF"; + setLocalReplyConfig(yaml); + initialize(); + + std::unordered_map expected_json_map = { + {"level", "TRACE"}, + {"response_flags", "UC"}, + {"response_body", "upstream connect error or disconnect/reset before headers. reset reason: " + "connection termination"}}; + + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}, + {"test-header", "exact-match-value"}}); + auto response = std::move(encoder_decoder.second); + + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + response->waitForEndStream(); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + } else { + codec_client_->close(); + } + + EXPECT_FALSE(upstream_request_->complete()); + EXPECT_EQ(0U, upstream_request_->bodyLength()); + + EXPECT_TRUE(response->complete()); + EXPECT_EQ("application/json", response->headers().ContentType()->value().getStringView()); + EXPECT_EQ("154", response->headers().ContentLength()->value().getStringView()); + EXPECT_EQ("503", response->headers().Status()->value().getStringView()); + // Check if returned json is same as expected + TestUtility::verifyJsonOutput(response->body(), expected_json_map); +} + +} // namespace Envoy \ No newline at end of file diff --git a/test/mocks/http/mocks.cc b/test/mocks/http/mocks.cc index 4fb7b6243768c..f4a63e25e43b6 100644 --- a/test/mocks/http/mocks.cc +++ b/test/mocks/http/mocks.cc @@ -77,15 +77,18 @@ void MockStreamDecoderFilterCallbacks::sendLocalReply_( const absl::optional grpc_status, absl::string_view details) { details_ = std::string(details); Utility::sendLocalReply( - is_grpc_request_, - [this, modify_headers](HeaderMapPtr&& headers, bool end_stream) -> void { - if (modify_headers != nullptr) { - modify_headers(*headers); - } - encodeHeaders(std::move(headers), end_stream); - }, - [this](Buffer::Instance& data, bool end_stream) -> void { encodeData(data, end_stream); }, - stream_destroyed_, code, body, grpc_status, is_head_request_); + stream_destroyed_, + Utility::EncodeFunctions{ + [this, modify_headers](HeaderMapPtr&& headers, bool end_stream) -> void { + if (modify_headers != nullptr) { + modify_headers(*headers); + } + encodeHeaders(std::move(headers), end_stream); + }, + [this](Buffer::Instance& data, bool end_stream) -> void { + encodeData(data, end_stream); + }}, + Utility::LocalReplyData{is_grpc_request_, code, body, grpc_status, is_head_request_}); } MockStreamEncoderFilterCallbacks::MockStreamEncoderFilterCallbacks() { diff --git a/test/test_common/utility.h b/test/test_common/utility.h index b84bdab195ad2..ed4faa2bbf00f 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -547,6 +547,27 @@ class TestUtility { MessageUtil::jsonConvert(source, tmp); MessageUtil::jsonConvert(tmp, ProtobufMessage::getStrictValidationVisitor(), dest); } + + /** + * Check that generated json as string is equal to expeceted response as unordered map. + * + * @param json_string response as json string. + * @param expected_map unordered map which contains expected data. + */ + static void verifyJsonOutput(std::string json_string, + std::unordered_map expected_map) { + const auto parsed = Json::Factory::loadFromString(json_string); + + // Every json log line should have only one newline character, and it should be the last + // character in the string + const auto newline_pos = json_string.find('\n'); + EXPECT_NE(newline_pos, std::string::npos); + EXPECT_EQ(newline_pos, json_string.length() - 1); + + for (const auto& pair : expected_map) { + EXPECT_EQ(parsed->getString(pair.first), pair.second); + } + } }; /** From 8d5bd74de9ca854eaf906baa6f7f26f472274838 Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Mon, 11 Nov 2019 21:38:42 +0100 Subject: [PATCH 04/15] Fixed format Signed-off-by: Lukasz Dziedziak --- source/common/local_reply/local_reply.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/local_reply/local_reply.h b/source/common/local_reply/local_reply.h index 3f816eb8a0c6a..ee409df078f93 100644 --- a/source/common/local_reply/local_reply.h +++ b/source/common/local_reply/local_reply.h @@ -31,7 +31,7 @@ class ResponseRewriter { using ResponseRewriterPtr = std::unique_ptr; /** - * Configuration of response mapper which contains pair of filter and definiton of rewriter. + * Configuration of response mapper which contains pair of filter and definition of rewriter. */ class ResponseMapper { public: From 525956eccf50e3be6c8ac201d56d737d7300aa3c Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Mon, 11 Nov 2019 23:10:14 +0100 Subject: [PATCH 05/15] Fixed spelling Signed-off-by: Lukasz Dziedziak --- test/test_common/utility.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_common/utility.h b/test/test_common/utility.h index ed4faa2bbf00f..bcafab9fbf107 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -549,7 +549,7 @@ class TestUtility { } /** - * Check that generated json as string is equal to expeceted response as unordered map. + * Check that generated json as string is equal to expected response as unordered map. * * @param json_string response as json string. * @param expected_map unordered map which contains expected data. From b71957b937b3884623441a257a9526bf2d7d8377 Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Tue, 12 Nov 2019 15:08:39 +0100 Subject: [PATCH 06/15] Fixed missing parameter in method after merge Signed-off-by: Lukasz Dziedziak --- source/common/access_log/access_log_formatter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/access_log/access_log_formatter.h b/source/common/access_log/access_log_formatter.h index ae159e05a623f..1f10184182000 100644 --- a/source/common/access_log/access_log_formatter.h +++ b/source/common/access_log/access_log_formatter.h @@ -264,7 +264,7 @@ class FilterStateFormatter : public FormatterProvider { // FormatterProvider::format std::string format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo& stream_info) const override; + const StreamInfo::StreamInfo& stream_info, const absl::string_view&) const override; private: std::string key_; From 84660a68a9941bd16244206aab273cf7c8e7dd3a Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Sun, 17 Nov 2019 13:48:44 +0100 Subject: [PATCH 07/15] Added docs and more tests Signed-off-by: Lukasz Dziedziak --- .../v2/http_connection_manager.proto | 4 +- .../http/http_conn_man/http_conn_man.rst | 1 + .../http/http_conn_man/local_reply.rst | 107 ++++++++++++++++++ include/envoy/router/BUILD | 1 - .../common/access_log/access_log_formatter.cc | 3 +- .../common/access_log/access_log_formatter.h | 3 +- source/extensions/quic_listeners/quiche/BUILD | 1 - test/common/local_reply/BUILD | 1 + test/common/local_reply/local_reply_test.cc | 100 +++++++++++++++- 9 files changed, 214 insertions(+), 7 deletions(-) create mode 100644 docs/root/configuration/http/http_conn_man/local_reply.rst diff --git a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto index d100484bf7242..b9e2da92c5bbe 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto @@ -487,10 +487,10 @@ message LocalReplyConfig { } message ResponseMapper { - // Filter is used to determine if the response should be changed. + // Filter is used to determine if the response should be rewritten. accesslog.v2.AccessLogFilter filter = 1 [(validate.rules).message = {required: true}]; - // Rewriter defines which values in local reply should be changed. + // Rewriter defines how to rewrite determined response. ResponseRewriter rewriter = 2 [(validate.rules).message = {required: true}]; } diff --git a/docs/root/configuration/http/http_conn_man/http_conn_man.rst b/docs/root/configuration/http/http_conn_man/http_conn_man.rst index 8414acc05fe7a..8f42239f81db1 100644 --- a/docs/root/configuration/http/http_conn_man/http_conn_man.rst +++ b/docs/root/configuration/http/http_conn_man/http_conn_man.rst @@ -12,6 +12,7 @@ HTTP connection manager header_casing headers header_sanitizing + local_reply stats runtime rds diff --git a/docs/root/configuration/http/http_conn_man/local_reply.rst b/docs/root/configuration/http/http_conn_man/local_reply.rst new file mode 100644 index 0000000000000..48659de8c0980 --- /dev/null +++ b/docs/root/configuration/http/http_conn_man/local_reply.rst @@ -0,0 +1,107 @@ +.. _config_http_conn_man_local_reply: + +Local reply modification +======================== + +The :ref:`HTTP connection manager ` supports modification of local reply which is response returned by Envoy itself. + +Features: + +* :ref:`Local reply content modification`. +* :ref:`Local reply format modification`. + +.. _config_http_conn_man_local_reply_modification: + +Local reply content modification +-------------------------------- + +There is support for modification of local replies. You can specify list of :ref:`mappers ` which contains +pairs of :ref:`filter ` and :ref:`rewriter `. Both elements in pair has to be +specified. If more than one pair is defined then first matching is used. + +Example how to change status code when local reply contains any of these response flags: + +.. code-block:: yaml + + mapper: + filter: + response_flag_filter: + flags: + - LH + - UH + rewriter: + status_code: 504 + +.. _config_http_conn_man_local_reply_format: + +Local reply format modification +------------------------------- + +Local reply format contains command operators that extract the relevant data and insert it. +They support two formats: :ref:`format strings ` and +:ref:`"format dictionaries" `. In both cases, the :ref:`command operators ` +are used to extract the relevant data, which is then inserted into the specified reply format. +Only one reply format may be specified at the time. + +.. _config_http_conn_man_local_reply_format_string: + +Format Strings +-------------- + +Format strings are plain strings, specified using the ``format`` key. They may contain +either :ref:`command operators ` or other characters interpreted as a plain string. +The access log formatter does not make any assumptions about a new line separator, so one +has to specified as part of the format string. + +.. code-block:: none + + %RESP_BODY% %RESPONSE_CODE% %RESPONSE_FLAGS% "My custom response" + +Example of custom Envoy local reply format: + +.. code-block:: none + + upstream connect error or disconnect/reset before headers. reset reason: connection failure 204 UH My custom response + + +If format isn't specified then :ref:`default format ` is used. + +.. _config_http_conn_man_local_reply_default_format: + +Default Format String +--------------------- + +If custom format string is not specified, Envoy uses the following default format: + +.. code-block:: none + + %RESP_BODY% + +Example of the default local reply format: + +.. code-block:: none + + upstream connect error or disconnect/reset before headers. reset reason: connection failure + +.. _config_http_conn_man_local_reply_dictionaries: + +Format Dictionaries +------------------- + +Format dictionaries are dictionaries that specify a structured local reply output format, +specified using the ``json_format`` key. This allows response to be returned in a structured format +such as JSON. + +More can be found in :ref:`configuration `. + +.. _config_http_conn_man_local_reply_command_operators: + +Command Operators +----------------- + +Local reply format reuse :ref:`access log operators `, so more information can be found there. +It is also possible to use new command operator provided only for local reply modification purpose. + +%RESP_BODY% + HTTP response body generated by Envoy. + diff --git a/include/envoy/router/BUILD b/include/envoy/router/BUILD index 17625722f7fbd..9e4b3599ada55 100644 --- a/include/envoy/router/BUILD +++ b/include/envoy/router/BUILD @@ -62,7 +62,6 @@ envoy_cc_library( "//include/envoy/upstream:retry_interface", "//source/common/protobuf", "//source/common/protobuf:utility_lib", - "@envoy_api//envoy/api/v2/core:pkg_cc_proto", ], ) diff --git a/source/common/access_log/access_log_formatter.cc b/source/common/access_log/access_log_formatter.cc index a5d770beb29a3..05d890d17da89 100644 --- a/source/common/access_log/access_log_formatter.cc +++ b/source/common/access_log/access_log_formatter.cc @@ -661,7 +661,8 @@ FilterStateFormatter::FilterStateFormatter(const std::string& key, std::string FilterStateFormatter::format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo& stream_info) const { + const StreamInfo::StreamInfo& stream_info, + const absl::string_view&) const { const StreamInfo::FilterState& filter_state = stream_info.filterState(); if (!filter_state.hasDataWithName(key_)) { return UnspecifiedValueString; diff --git a/source/common/access_log/access_log_formatter.h b/source/common/access_log/access_log_formatter.h index 1f10184182000..944d73471e696 100644 --- a/source/common/access_log/access_log_formatter.h +++ b/source/common/access_log/access_log_formatter.h @@ -264,7 +264,8 @@ class FilterStateFormatter : public FormatterProvider { // FormatterProvider::format std::string format(const Http::HeaderMap&, const Http::HeaderMap&, const Http::HeaderMap&, - const StreamInfo::StreamInfo& stream_info, const absl::string_view&) const override; + const StreamInfo::StreamInfo& stream_info, + const absl::string_view&) const override; private: std::string key_; diff --git a/source/extensions/quic_listeners/quiche/BUILD b/source/extensions/quic_listeners/quiche/BUILD index 7dc5605918202..69e199cca902d 100644 --- a/source/extensions/quic_listeners/quiche/BUILD +++ b/source/extensions/quic_listeners/quiche/BUILD @@ -232,7 +232,6 @@ envoy_cc_library( "//source/common/network:listener_lib", "//source/common/protobuf:utility_lib", "//source/server:connection_handler_lib", - "@envoy_api//envoy/api/v2/listener:pkg_cc_proto", ], ) diff --git a/test/common/local_reply/BUILD b/test/common/local_reply/BUILD index 1b0b22d9ff62e..794a8549589a2 100644 --- a/test/common/local_reply/BUILD +++ b/test/common/local_reply/BUILD @@ -14,6 +14,7 @@ envoy_cc_test( deps = [ "//source/common/common:assert_lib", "//source/common/local_reply:local_reply_lib", + "//test/mocks/access_log:access_log_mocks", "//test/mocks/http:http_mocks", "//test/mocks/stream_info:stream_info_mocks", ], diff --git a/test/common/local_reply/local_reply_test.cc b/test/common/local_reply/local_reply_test.cc index 9c280aabfa525..8cf50368b37d3 100644 --- a/test/common/local_reply/local_reply_test.cc +++ b/test/common/local_reply/local_reply_test.cc @@ -2,11 +2,14 @@ #include "common/local_reply/local_reply.h" +#include "test/mocks/access_log/mocks.h" #include "test/mocks/http/mocks.h" #include "gmock/gmock.h" +#include "gtest/gtest.h" using testing::_; +using testing::Return; namespace Envoy { namespace LocalReply { @@ -48,7 +51,7 @@ class LocalReplyTest : public testing::Test { Envoy::AccessLog::FormatterPtr formatter_; NiceMock stream_info_; Http::TestHeaderMapImpl headers_; - Http::Code code_; + Http::Code code_{Http::Code::OK}; }; TEST_F(LocalReplyTest, ShouldInsertContentLengthAndTypeHeaders) { @@ -78,5 +81,100 @@ TEST_F(LocalReplyTest, ShouldNotInsertContentHeadersForEmptyBody) { EXPECT_TRUE(headers_.ContentLength() == nullptr); EXPECT_TRUE(headers_.ContentType() == nullptr); } + +TEST_F(LocalReplyTest, ShouldMatchFirstFilterAndReturn) { + // given + std::list mappers; + // first mapper + absl::optional response_code1(504); + ResponseRewriterPtr response_rewriter1 = std::make_unique(response_code1); + AccessLog::MockFilter* filter1{new AccessLog::MockFilter()}; + ResponseMapperPtr mapper1 = std::make_unique( + ResponseMapper{AccessLog::FilterPtr{filter1}, std::move(response_rewriter1)}); + mappers.emplace_back(std::move(mapper1)); + EXPECT_CALL(*filter1, evaluate(_, _, _, _)).WillOnce(Return(true)); + + // second mapper + absl::optional response_code2(505); + ResponseRewriterPtr response_rewriter2 = std::make_unique(response_code2); + AccessLog::MockFilter* filter2{new AccessLog::MockFilter()}; + ResponseMapperPtr mapper2 = std::make_unique( + ResponseMapper{AccessLog::FilterPtr{filter2}, std::move(response_rewriter2)}); + mappers.emplace_back(std::move(mapper2)); + EXPECT_CALL(*filter2, evaluate(_, _, _, _)).Times(0); + + LocalReplyPtr local_reply = std::make_unique(LocalReply{ + std::move(mappers), std::move(formatter_), Http::Headers::get().ContentTypeValues.Text}); + + // when + local_reply->matchAndRewrite(&headers_, &headers_, &headers_, stream_info_, code_); + + // then + EXPECT_EQ(code_, Http::Code::GatewayTimeout); +} + +TEST_F(LocalReplyTest, ShouldMatchSecondFilterAndReturn) { + // given + std::list mappers; + + // first mapper + AccessLog::MockFilter* filter1{new AccessLog::MockFilter()}; + absl::optional response_code1(504); + ResponseRewriterPtr response_rewriter1 = std::make_unique(response_code1); + ResponseMapperPtr mapper1 = std::make_unique( + ResponseMapper{AccessLog::FilterPtr{filter1}, std::move(response_rewriter1)}); + mappers.emplace_back(std::move(mapper1)); + EXPECT_CALL(*filter1, evaluate(_, _, _, _)).WillOnce(Return(false)); + + // second mapper + AccessLog::MockFilter* filter2{new AccessLog::MockFilter()}; + absl::optional response_code2(505); + ResponseRewriterPtr response_rewriter2 = std::make_unique(response_code2); + ResponseMapperPtr mapper2 = std::make_unique( + ResponseMapper{AccessLog::FilterPtr{filter2}, std::move(response_rewriter2)}); + mappers.emplace_back(std::move(mapper2)); + EXPECT_CALL(*filter2, evaluate(_, _, _, _)).WillOnce(Return(true)); + + LocalReplyPtr local_reply = std::make_unique(LocalReply{ + std::move(mappers), std::move(formatter_), Http::Headers::get().ContentTypeValues.Text}); + + // when + local_reply->matchAndRewrite(&headers_, &headers_, &headers_, stream_info_, code_); + + // then + EXPECT_EQ(code_, Http::Code::HTTPVersionNotSupported); +} + +TEST_F(LocalReplyTest, ShuldNotMatchAnyFilter) { + // given + std::list mappers; + // first mapper + absl::optional response_code1(504); + ResponseRewriterPtr response_rewriter1 = std::make_unique(response_code1); + AccessLog::MockFilter* filter1{new AccessLog::MockFilter()}; + ResponseMapperPtr mapper1 = std::make_unique( + ResponseMapper{AccessLog::FilterPtr{filter1}, std::move(response_rewriter1)}); + mappers.emplace_back(std::move(mapper1)); + EXPECT_CALL(*filter1, evaluate(_, _, _, _)).WillOnce(Return(false)); + + // second mapper + absl::optional response_code2(505); + ResponseRewriterPtr response_rewriter2 = std::make_unique(response_code2); + AccessLog::MockFilter* filter2{new AccessLog::MockFilter()}; + ResponseMapperPtr mapper2 = std::make_unique( + ResponseMapper{AccessLog::FilterPtr{filter2}, std::move(response_rewriter2)}); + mappers.emplace_back(std::move(mapper2)); + EXPECT_CALL(*filter2, evaluate(_, _, _, _)).WillOnce(Return(false)); + + LocalReplyPtr local_reply = std::make_unique(LocalReply{ + std::move(mappers), std::move(formatter_), Http::Headers::get().ContentTypeValues.Text}); + + // when + local_reply->matchAndRewrite(&headers_, &headers_, &headers_, stream_info_, code_); + + // then + EXPECT_EQ(code_, Http::Code::OK); +} + } // namespace LocalReply } // namespace Envoy \ No newline at end of file From 3e0d621b9160e9b06de1c3c573f37e6fd94cd6e6 Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Sun, 17 Nov 2019 18:24:31 +0100 Subject: [PATCH 08/15] Fix import Signed-off-by: Lukasz Dziedziak --- .../v3alpha/http_connection_manager.proto | 4 ++-- include/envoy/router/BUILD | 1 + source/extensions/quic_listeners/quiche/BUILD | 1 + test/test_common/utility.h | 3 ++- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto b/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto index 5e21c038f91f8..f6f96a6cc4877 100644 --- a/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto +++ b/api/envoy/config/filter/network/http_connection_manager/v3alpha/http_connection_manager.proto @@ -474,10 +474,10 @@ message LocalReplyConfig { } message ResponseMapper { - // Filter is used to determine if the response should be changed. + // Filter is used to determine if the response should be rewritten. accesslog.v3alpha.AccessLogFilter filter = 1 [(validate.rules).message = {required: true}]; - // Rewriter defines which values in local reply should be changed. + // Rewriter defines how to rewrite determined response. ResponseRewriter rewriter = 2 [(validate.rules).message = {required: true}]; } diff --git a/include/envoy/router/BUILD b/include/envoy/router/BUILD index 9e4b3599ada55..17625722f7fbd 100644 --- a/include/envoy/router/BUILD +++ b/include/envoy/router/BUILD @@ -62,6 +62,7 @@ envoy_cc_library( "//include/envoy/upstream:retry_interface", "//source/common/protobuf", "//source/common/protobuf:utility_lib", + "@envoy_api//envoy/api/v2/core:pkg_cc_proto", ], ) diff --git a/source/extensions/quic_listeners/quiche/BUILD b/source/extensions/quic_listeners/quiche/BUILD index 69e199cca902d..7dc5605918202 100644 --- a/source/extensions/quic_listeners/quiche/BUILD +++ b/source/extensions/quic_listeners/quiche/BUILD @@ -232,6 +232,7 @@ envoy_cc_library( "//source/common/network:listener_lib", "//source/common/protobuf:utility_lib", "//source/server:connection_handler_lib", + "@envoy_api//envoy/api/v2/listener:pkg_cc_proto", ], ) diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 9e378117db07f..ea36d2c971c9e 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -18,6 +18,7 @@ #include "common/common/c_smart_ptr.h" #include "common/common/thread.h" #include "common/http/header_map_impl.h" +#include "common/json/json_loader.h" #include "common/protobuf/message_validator_impl.h" #include "common/protobuf/utility.h" #include "common/stats/fake_symbol_table_impl.h" @@ -549,7 +550,7 @@ class TestUtility { } /** - * Check that generated json as string is equal to expected response as unordered map. + * Check that generated json as string is equal unordered map. * * @param json_string response as json string. * @param expected_map unordered map which contains expected data. From bdcaa07f3181016a689457e0611be04e1c610ead Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Sun, 17 Nov 2019 22:06:46 +0100 Subject: [PATCH 09/15] Fixed format Signed-off-by: Lukasz Dziedziak --- test/common/access_log/access_log_formatter_test.cc | 5 +++-- test/common/http/utility_test.cc | 11 ++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/test/common/access_log/access_log_formatter_test.cc b/test/common/access_log/access_log_formatter_test.cc index 8333b2c780df2..4efc6c175c2fa 100644 --- a/test/common/access_log/access_log_formatter_test.cc +++ b/test/common/access_log/access_log_formatter_test.cc @@ -1041,8 +1041,9 @@ TEST(AccessLogFormatterTest, CompositeFormatterSuccess) { "%FILTER_STATE(testing):8%|%FILTER_STATE(nonexisting)%"; FormatterImpl formatter(format); - EXPECT_EQ("\"test_value\"|-|\"test_va|-", - formatter.format(request_header, response_header, response_trailer, stream_info)); + EXPECT_EQ( + "\"test_value\"|-|\"test_va|-", + formatter.format(request_header, response_header, response_trailer, stream_info, body)); } { diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index d9c505ceb01df..311325f53095e 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -514,11 +514,12 @@ TEST(HttpUtility, RateLimitedGrpcStatus) { EXPECT_EQ(headers.GrpcStatus()->value().getStringView(), std::to_string(enumToInt(Grpc::Status::WellKnownGrpcStatus::ResourceExhausted))); })); - Utility::sendLocalReply(false, callbacks, - Utility::LocalReplyData{true, Http::Code::TooManyRequests, "", - absl::make_optional( - Grpc::Status::GrpcStatus::ResourceExhausted), - false}); + Utility::sendLocalReply( + false, callbacks, + Utility::LocalReplyData{true, Http::Code::TooManyRequests, "", + absl::make_optional( + Grpc::Status::WellKnownGrpcStatus::ResourceExhausted), + false}); } TEST(HttpUtility, SendLocalReplyDestroyedEarly) { From ed817a5f9897b9cbf39565c2fbead1965da50fd1 Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Sun, 24 Nov 2019 23:48:26 +0100 Subject: [PATCH 10/15] Nullptr bcs of header not initialized Signed-off-by: Lukasz Dziedziak --- source/common/local_reply/local_reply.cc | 12 +++++++++--- source/common/local_reply/local_reply.h | 1 + source/extensions/transport_sockets/tap/BUILD | 3 --- test/server/BUILD | 1 - 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/source/common/local_reply/local_reply.cc b/source/common/local_reply/local_reply.cc index 18eee8e7d0b28..1f922054e99ec 100644 --- a/source/common/local_reply/local_reply.cc +++ b/source/common/local_reply/local_reply.cc @@ -37,8 +37,12 @@ void LocalReply::matchAndRewrite(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_headers, const Http::HeaderMap* response_trailers, const StreamInfo::StreamInfo& stream_info, Http::Code& code) { + for (auto& mapper : mappers_) { - if (mapper->match(request_headers, response_headers, response_trailers, stream_info)) { + if (mapper->match(request_headers == nullptr ? empty_headers_ : request_headers, + response_headers == nullptr ? empty_headers_ : response_headers, + response_trailers == nullptr ? empty_headers_ : response_trailers, + stream_info)) { mapper->rewrite(code); break; } @@ -50,8 +54,10 @@ std::string LocalReply::format(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_trailers, const StreamInfo::StreamInfo& stream_info, const absl::string_view& body) { - return formatter_->format(*request_headers, *response_headers, *response_trailers, stream_info, - body); + return formatter_->format(request_headers == nullptr ? *empty_headers_ : *request_headers, + response_headers == nullptr ? *empty_headers_ : *response_headers, + response_trailers == nullptr ? *empty_headers_ : *response_trailers, + stream_info, body); } void LocalReply::insertContentHeaders(const absl::string_view& body, Http::HeaderMap* headers) { diff --git a/source/common/local_reply/local_reply.h b/source/common/local_reply/local_reply.h index ee409df078f93..76d414a3adc08 100644 --- a/source/common/local_reply/local_reply.h +++ b/source/common/local_reply/local_reply.h @@ -108,6 +108,7 @@ class LocalReply { std::list mappers_; AccessLog::FormatterPtr formatter_; std::string content_type_; + const Http::HeaderMap* empty_headers_{}; }; using LocalReplyPtr = std::unique_ptr; diff --git a/source/extensions/transport_sockets/tap/BUILD b/source/extensions/transport_sockets/tap/BUILD index 81cd27b5a497e..9dad7be68f43c 100644 --- a/source/extensions/transport_sockets/tap/BUILD +++ b/source/extensions/transport_sockets/tap/BUILD @@ -40,8 +40,6 @@ envoy_cc_library( "//include/envoy/network:transport_socket_interface", "//source/common/buffer:buffer_lib", "//source/extensions/common/tap:extension_config_base", - "@envoy_api//envoy/config/transport_socket/tap/v2alpha:pkg_cc_proto", - "@envoy_api//envoy/data/tap/v2alpha:pkg_cc_proto", ], ) @@ -60,6 +58,5 @@ envoy_cc_extension( "//source/common/config:utility_lib", "//source/common/protobuf:utility_lib", "//source/extensions/transport_sockets:well_known_names", - "@envoy_api//envoy/config/transport_socket/tap/v2alpha:pkg_cc_proto", ], ) diff --git a/test/server/BUILD b/test/server/BUILD index 9aeabecac6c73..972a4f2412ef8 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -348,7 +348,6 @@ envoy_cc_test_library( deps = [ "//source/common/protobuf:utility_lib", "//test/test_common:utility_lib", - "@envoy_api//envoy/api/v2:pkg_cc_proto", ], ) From b4c82b67321d6977f8dc27a1029d86f596a9d7cc Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Mon, 25 Nov 2019 10:48:59 +0100 Subject: [PATCH 11/15] Fix format Signed-off-by: Lukasz Dziedziak --- source/extensions/transport_sockets/tap/BUILD | 3 +++ test/common/grpc/BUILD | 5 +---- test/common/stats/BUILD | 1 - test/server/BUILD | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/source/extensions/transport_sockets/tap/BUILD b/source/extensions/transport_sockets/tap/BUILD index 9dad7be68f43c..81cd27b5a497e 100644 --- a/source/extensions/transport_sockets/tap/BUILD +++ b/source/extensions/transport_sockets/tap/BUILD @@ -40,6 +40,8 @@ envoy_cc_library( "//include/envoy/network:transport_socket_interface", "//source/common/buffer:buffer_lib", "//source/extensions/common/tap:extension_config_base", + "@envoy_api//envoy/config/transport_socket/tap/v2alpha:pkg_cc_proto", + "@envoy_api//envoy/data/tap/v2alpha:pkg_cc_proto", ], ) @@ -58,5 +60,6 @@ envoy_cc_extension( "//source/common/config:utility_lib", "//source/common/protobuf:utility_lib", "//source/extensions/transport_sockets:well_known_names", + "@envoy_api//envoy/config/transport_socket/tap/v2alpha:pkg_cc_proto", ], ) diff --git a/test/common/grpc/BUILD b/test/common/grpc/BUILD index 523c1d1fd2662..f005647672dc9 100644 --- a/test/common/grpc/BUILD +++ b/test/common/grpc/BUILD @@ -168,8 +168,5 @@ envoy_cc_test_library( name = "utility_lib", hdrs = ["utility.h"], data = ["//test/config/integration/certs"], - deps = [ - "//test/test_common:environment_lib", - "@envoy_api//envoy/api/v2/core:pkg_cc_proto", - ], + deps = ["//test/test_common:environment_lib"], ) diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index b39c0b6243d5e..5d6c4cc3ee9ce 100644 --- a/test/common/stats/BUILD +++ b/test/common/stats/BUILD @@ -189,7 +189,6 @@ envoy_cc_test( "//test/mocks/thread_local:thread_local_mocks", "//test/test_common:logging_lib", "//test/test_common:utility_lib", - "@envoy_api//envoy/config/metrics/v2:pkg_cc_proto", ], ) diff --git a/test/server/BUILD b/test/server/BUILD index 972a4f2412ef8..9c3267b5a3fec 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -179,7 +179,6 @@ envoy_cc_test_library( "//test/test_common:simulated_time_system_lib", "//test/test_common:test_time_lib", "//test/test_common:threadsafe_singleton_injector_lib", - "@envoy_api//envoy/admin/v2alpha:pkg_cc_proto", ], ) @@ -348,6 +347,7 @@ envoy_cc_test_library( deps = [ "//source/common/protobuf:utility_lib", "//test/test_common:utility_lib", + "@envoy_api//envoy/api/v2:pkg_cc_proto", ], ) From 24075303945acde51295480cbe838a1f71429f61 Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Mon, 25 Nov 2019 11:11:55 +0100 Subject: [PATCH 12/15] Fix format Signed-off-by: Lukasz Dziedziak --- test/common/grpc/BUILD | 5 ++++- test/common/stats/BUILD | 1 + test/server/BUILD | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/common/grpc/BUILD b/test/common/grpc/BUILD index f005647672dc9..523c1d1fd2662 100644 --- a/test/common/grpc/BUILD +++ b/test/common/grpc/BUILD @@ -168,5 +168,8 @@ envoy_cc_test_library( name = "utility_lib", hdrs = ["utility.h"], data = ["//test/config/integration/certs"], - deps = ["//test/test_common:environment_lib"], + deps = [ + "//test/test_common:environment_lib", + "@envoy_api//envoy/api/v2/core:pkg_cc_proto", + ], ) diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index 5d6c4cc3ee9ce..b39c0b6243d5e 100644 --- a/test/common/stats/BUILD +++ b/test/common/stats/BUILD @@ -189,6 +189,7 @@ envoy_cc_test( "//test/mocks/thread_local:thread_local_mocks", "//test/test_common:logging_lib", "//test/test_common:utility_lib", + "@envoy_api//envoy/config/metrics/v2:pkg_cc_proto", ], ) diff --git a/test/server/BUILD b/test/server/BUILD index 9c3267b5a3fec..9aeabecac6c73 100644 --- a/test/server/BUILD +++ b/test/server/BUILD @@ -179,6 +179,7 @@ envoy_cc_test_library( "//test/test_common:simulated_time_system_lib", "//test/test_common:test_time_lib", "//test/test_common:threadsafe_singleton_injector_lib", + "@envoy_api//envoy/admin/v2alpha:pkg_cc_proto", ], ) From f4d9a07661d9387baeb64138bcb107365bc1708e Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Mon, 25 Nov 2019 15:41:44 +0100 Subject: [PATCH 13/15] Fix problem with empty headers Signed-off-by: Lukasz Dziedziak --- source/common/http/conn_manager_impl.cc | 39 +++++++++++++++++------- source/common/local_reply/local_reply.cc | 11 ++----- source/common/local_reply/local_reply.h | 1 - 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 2563aaeb4cef1..e2754c8740b07 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -1352,7 +1352,7 @@ void ConnectionManagerImpl::ActiveStream::sendLocalReply( createFilterChain(); } stream_info_.setResponseCodeDetails(details); - + HeaderMap* empty_headers{}; Utility::sendLocalReply( state_.destroyed_, Utility::EncodeFunctions{ @@ -1371,14 +1371,19 @@ void ConnectionManagerImpl::ActiveStream::sendLocalReply( encodeData(nullptr, data, end_stream, FilterIterationStartState::CanStartFromCurrent); }}, Utility::RewriteReplyFunctions{ - [this](Code& code) -> void { + [this, empty_headers](Code& code) -> void { connection_manager_.config_.localReply()->matchAndRewrite( - request_headers_.get(), response_headers_.get(), response_trailers_.get(), + request_headers_ == nullptr ? empty_headers : request_headers_.get(), + response_headers_ == nullptr ? empty_headers : response_headers_.get(), + response_trailers_ == nullptr ? empty_headers : response_trailers_.get(), stream_info_, code); }, - [this](HeaderMapPtr&& headers, absl::string_view& body_text) -> std::string { + [this, empty_headers](HeaderMapPtr&& headers, + absl::string_view& body_text) -> std::string { std::string formatted_body = connection_manager_.config_.localReply()->format( - request_headers_.get(), response_headers_.get(), response_trailers_.get(), + request_headers_ == nullptr ? empty_headers : request_headers_.get(), + response_headers_ == nullptr ? empty_headers : response_headers_.get(), + response_trailers_ == nullptr ? empty_headers : response_trailers_.get(), stream_info_, body_text); connection_manager_.config_.localReply()->insertContentHeaders(formatted_body, @@ -2308,6 +2313,7 @@ void ConnectionManagerImpl::ActiveStreamEncoderFilter::responseDataTooLarge() { parent_.stream_info_.setResponseCodeDetails( StreamInfo::ResponseCodeDetails::get().RequestHeadersTooLarge); + HeaderMap* empty_headers{}; Http::Utility::sendLocalReply( parent_.state_.destroyed_, Utility::EncodeFunctions{[&](HeaderMapPtr&& response_headers, bool end_stream) -> void { @@ -2322,16 +2328,27 @@ void ConnectionManagerImpl::ActiveStreamEncoderFilter::responseDataTooLarge() { parent_.state_.local_complete_ = end_stream; }}, Utility::RewriteReplyFunctions{ - [&](Code& code) -> void { + [&, empty_headers](Code& code) -> void { parent_.connection_manager_.config_.localReply()->matchAndRewrite( - parent_.request_headers_.get(), parent_.response_headers_.get(), - parent_.response_trailers_.get(), parent_.stream_info_, code); + parent_.request_headers_ == nullptr ? empty_headers + : parent_.request_headers_.get(), + parent_.response_headers_ == nullptr ? empty_headers + : parent_.response_headers_.get(), + parent_.response_trailers_ == nullptr ? empty_headers + : parent_.response_trailers_.get(), + parent_.stream_info_, code); }, - [&](HeaderMapPtr&& headers, absl::string_view body_text) -> std::string { + [&, empty_headers](HeaderMapPtr&& headers, + absl::string_view body_text) -> std::string { std::string formatted_body = parent_.connection_manager_.config_.localReply()->format( - parent_.request_headers_.get(), parent_.response_headers_.get(), - parent_.response_trailers_.get(), parent_.stream_info_, body_text); + parent_.request_headers_ == nullptr ? empty_headers + : parent_.request_headers_.get(), + parent_.response_headers_ == nullptr ? empty_headers + : parent_.response_headers_.get(), + parent_.response_trailers_ == nullptr ? empty_headers + : parent_.response_trailers_.get(), + parent_.stream_info_, body_text); parent_.connection_manager_.config_.localReply()->insertContentHeaders( formatted_body, headers.get()); diff --git a/source/common/local_reply/local_reply.cc b/source/common/local_reply/local_reply.cc index 1f922054e99ec..8f86bd8802a32 100644 --- a/source/common/local_reply/local_reply.cc +++ b/source/common/local_reply/local_reply.cc @@ -39,10 +39,7 @@ void LocalReply::matchAndRewrite(const Http::HeaderMap* request_headers, const StreamInfo::StreamInfo& stream_info, Http::Code& code) { for (auto& mapper : mappers_) { - if (mapper->match(request_headers == nullptr ? empty_headers_ : request_headers, - response_headers == nullptr ? empty_headers_ : response_headers, - response_trailers == nullptr ? empty_headers_ : response_trailers, - stream_info)) { + if (mapper->match(request_headers, response_headers, response_trailers, stream_info)) { mapper->rewrite(code); break; } @@ -54,10 +51,8 @@ std::string LocalReply::format(const Http::HeaderMap* request_headers, const Http::HeaderMap* response_trailers, const StreamInfo::StreamInfo& stream_info, const absl::string_view& body) { - return formatter_->format(request_headers == nullptr ? *empty_headers_ : *request_headers, - response_headers == nullptr ? *empty_headers_ : *response_headers, - response_trailers == nullptr ? *empty_headers_ : *response_trailers, - stream_info, body); + return formatter_->format(*request_headers, *response_headers, *response_trailers, stream_info, + body); } void LocalReply::insertContentHeaders(const absl::string_view& body, Http::HeaderMap* headers) { diff --git a/source/common/local_reply/local_reply.h b/source/common/local_reply/local_reply.h index 76d414a3adc08..ee409df078f93 100644 --- a/source/common/local_reply/local_reply.h +++ b/source/common/local_reply/local_reply.h @@ -108,7 +108,6 @@ class LocalReply { std::list mappers_; AccessLog::FormatterPtr formatter_; std::string content_type_; - const Http::HeaderMap* empty_headers_{}; }; using LocalReplyPtr = std::unique_ptr; From 613203d2f1b9e2d25b1347119d06ea3b31623392 Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Mon, 25 Nov 2019 23:05:44 +0100 Subject: [PATCH 14/15] Another try to fix nullptr Signed-off-by: Lukasz Dziedziak --- source/common/http/conn_manager_impl.cc | 41 +++++++----------------- source/common/local_reply/local_reply.cc | 16 ++++----- source/common/local_reply/local_reply.h | 12 +++---- 3 files changed, 25 insertions(+), 44 deletions(-) diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index e2754c8740b07..60f5fe4cffc1f 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -1352,7 +1352,6 @@ void ConnectionManagerImpl::ActiveStream::sendLocalReply( createFilterChain(); } stream_info_.setResponseCodeDetails(details); - HeaderMap* empty_headers{}; Utility::sendLocalReply( state_.destroyed_, Utility::EncodeFunctions{ @@ -1371,20 +1370,14 @@ void ConnectionManagerImpl::ActiveStream::sendLocalReply( encodeData(nullptr, data, end_stream, FilterIterationStartState::CanStartFromCurrent); }}, Utility::RewriteReplyFunctions{ - [this, empty_headers](Code& code) -> void { + [this](Code& code) -> void { connection_manager_.config_.localReply()->matchAndRewrite( - request_headers_ == nullptr ? empty_headers : request_headers_.get(), - response_headers_ == nullptr ? empty_headers : response_headers_.get(), - response_trailers_ == nullptr ? empty_headers : response_trailers_.get(), - stream_info_, code); + *request_headers_, *response_headers_, *response_trailers_, stream_info_, code); }, - [this, empty_headers](HeaderMapPtr&& headers, - absl::string_view& body_text) -> std::string { + [this](HeaderMapPtr&& headers, absl::string_view& body_text) -> std::string { std::string formatted_body = connection_manager_.config_.localReply()->format( - request_headers_ == nullptr ? empty_headers : request_headers_.get(), - response_headers_ == nullptr ? empty_headers : response_headers_.get(), - response_trailers_ == nullptr ? empty_headers : response_trailers_.get(), - stream_info_, body_text); + *request_headers_, *response_headers_, *response_trailers_, stream_info_, + body_text); connection_manager_.config_.localReply()->insertContentHeaders(formatted_body, headers.get()); @@ -2313,7 +2306,6 @@ void ConnectionManagerImpl::ActiveStreamEncoderFilter::responseDataTooLarge() { parent_.stream_info_.setResponseCodeDetails( StreamInfo::ResponseCodeDetails::get().RequestHeadersTooLarge); - HeaderMap* empty_headers{}; Http::Utility::sendLocalReply( parent_.state_.destroyed_, Utility::EncodeFunctions{[&](HeaderMapPtr&& response_headers, bool end_stream) -> void { @@ -2328,27 +2320,16 @@ void ConnectionManagerImpl::ActiveStreamEncoderFilter::responseDataTooLarge() { parent_.state_.local_complete_ = end_stream; }}, Utility::RewriteReplyFunctions{ - [&, empty_headers](Code& code) -> void { + [&](Code& code) -> void { parent_.connection_manager_.config_.localReply()->matchAndRewrite( - parent_.request_headers_ == nullptr ? empty_headers - : parent_.request_headers_.get(), - parent_.response_headers_ == nullptr ? empty_headers - : parent_.response_headers_.get(), - parent_.response_trailers_ == nullptr ? empty_headers - : parent_.response_trailers_.get(), - parent_.stream_info_, code); + *parent_.request_headers_, *parent_.response_headers_, + *parent_.response_trailers_, parent_.stream_info_, code); }, - [&, empty_headers](HeaderMapPtr&& headers, - absl::string_view body_text) -> std::string { + [&](HeaderMapPtr&& headers, absl::string_view body_text) -> std::string { std::string formatted_body = parent_.connection_manager_.config_.localReply()->format( - parent_.request_headers_ == nullptr ? empty_headers - : parent_.request_headers_.get(), - parent_.response_headers_ == nullptr ? empty_headers - : parent_.response_headers_.get(), - parent_.response_trailers_ == nullptr ? empty_headers - : parent_.response_trailers_.get(), - parent_.stream_info_, body_text); + *parent_.request_headers_, *parent_.response_headers_, + *parent_.response_trailers_, parent_.stream_info_, body_text); parent_.connection_manager_.config_.localReply()->insertContentHeaders( formatted_body, headers.get()); diff --git a/source/common/local_reply/local_reply.cc b/source/common/local_reply/local_reply.cc index 8f86bd8802a32..cf7e70a313754 100644 --- a/source/common/local_reply/local_reply.cc +++ b/source/common/local_reply/local_reply.cc @@ -33,25 +33,25 @@ LocalReply::LocalReply(std::list mappers, AccessLog::Formatte std::string content_type) : mappers_(std::move(mappers)), formatter_(std::move(formatter)), content_type_(content_type) {} -void LocalReply::matchAndRewrite(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers, - const Http::HeaderMap* response_trailers, +void LocalReply::matchAndRewrite(const Http::HeaderMap& request_headers, + const Http::HeaderMap& response_headers, + const Http::HeaderMap& response_trailers, const StreamInfo::StreamInfo& stream_info, Http::Code& code) { for (auto& mapper : mappers_) { - if (mapper->match(request_headers, response_headers, response_trailers, stream_info)) { + if (mapper->match(&request_headers, &response_headers, &response_trailers, stream_info)) { mapper->rewrite(code); break; } } } -std::string LocalReply::format(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers, - const Http::HeaderMap* response_trailers, +std::string LocalReply::format(const Http::HeaderMap& request_headers, + const Http::HeaderMap& response_headers, + const Http::HeaderMap& response_trailers, const StreamInfo::StreamInfo& stream_info, const absl::string_view& body) { - return formatter_->format(*request_headers, *response_headers, *response_trailers, stream_info, + return formatter_->format(request_headers, response_headers, response_trailers, stream_info, body); } diff --git a/source/common/local_reply/local_reply.h b/source/common/local_reply/local_reply.h index ee409df078f93..81a734ce404ec 100644 --- a/source/common/local_reply/local_reply.h +++ b/source/common/local_reply/local_reply.h @@ -77,9 +77,9 @@ class LocalReply { * @param stream_info supplies the information about streams required by filters. * @param code local reply status code. */ - void matchAndRewrite(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers, - const Http::HeaderMap* response_trailers, + void matchAndRewrite(const Http::HeaderMap& request_headers, + const Http::HeaderMap& response_headers, + const Http::HeaderMap& response_trailers, const StreamInfo::StreamInfo& stream_info, Http::Code& code); /** @@ -92,9 +92,9 @@ class LocalReply { * @param body original response body. * @return std::string formatted response body. */ - std::string format(const Http::HeaderMap* request_headers, - const Http::HeaderMap* response_headers, - const Http::HeaderMap* response_trailers, + std::string format(const Http::HeaderMap& request_headers, + const Http::HeaderMap& response_headers, + const Http::HeaderMap& response_trailers, const StreamInfo::StreamInfo& stream_info, const absl::string_view& body); /** From 7af677dfcd763f37b3bd5f17fb643e226747581e Mon Sep 17 00:00:00 2001 From: Lukasz Dziedziak Date: Tue, 26 Nov 2019 07:45:03 +0100 Subject: [PATCH 15/15] Fix test Signed-off-by: Lukasz Dziedziak --- test/common/local_reply/local_reply_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/common/local_reply/local_reply_test.cc b/test/common/local_reply/local_reply_test.cc index 8cf50368b37d3..a3a60e4c17eba 100644 --- a/test/common/local_reply/local_reply_test.cc +++ b/test/common/local_reply/local_reply_test.cc @@ -107,7 +107,7 @@ TEST_F(LocalReplyTest, ShouldMatchFirstFilterAndReturn) { std::move(mappers), std::move(formatter_), Http::Headers::get().ContentTypeValues.Text}); // when - local_reply->matchAndRewrite(&headers_, &headers_, &headers_, stream_info_, code_); + local_reply->matchAndRewrite(headers_, headers_, headers_, stream_info_, code_); // then EXPECT_EQ(code_, Http::Code::GatewayTimeout); @@ -139,7 +139,7 @@ TEST_F(LocalReplyTest, ShouldMatchSecondFilterAndReturn) { std::move(mappers), std::move(formatter_), Http::Headers::get().ContentTypeValues.Text}); // when - local_reply->matchAndRewrite(&headers_, &headers_, &headers_, stream_info_, code_); + local_reply->matchAndRewrite(headers_, headers_, headers_, stream_info_, code_); // then EXPECT_EQ(code_, Http::Code::HTTPVersionNotSupported); @@ -170,7 +170,7 @@ TEST_F(LocalReplyTest, ShuldNotMatchAnyFilter) { std::move(mappers), std::move(formatter_), Http::Headers::get().ContentTypeValues.Text}); // when - local_reply->matchAndRewrite(&headers_, &headers_, &headers_, stream_info_, code_); + local_reply->matchAndRewrite(headers_, headers_, headers_, stream_info_, code_); // then EXPECT_EQ(code_, Http::Code::OK);