Extend StatefulHeaderFormatter to allow forwarding HTTP1 reason phrase (

#18997)

Signed-off-by: Max Kuznetsov <[email protected]>

api/envoy/extensions/http/header_formatters/preserve_case/v3/preserve_case.proto

@@ -16,4 +16,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // See the :ref:`header casing <config_http_conn_man_header_casing>` configuration guide for more
 // information.
 message PreserveCaseFormatterConfig {
+  // Allows forwarding reason phrase text.
+  // This is off by default, and a standard reason phrase is used for a corresponding HTTP response code.
+  bool forward_reason_phrase = 1;
 }

docs/root/version_history/current.rst

@@ -70,6 +70,7 @@ New Features
 * http: added support for %REQUESTED_SERVER_NAME% to extract SNI as a custom header.
 * http: added support for :ref:`retriable health check status codes <envoy_v3_api_field_config.core.v3.HealthCheck.HttpHealthCheck.retriable_statuses>`.
 * http: added timing information about upstream connection and encryption establishment to stream info. These can currently be accessed via custom access loggers.
+* http: added support for :ref:`forwarding HTTP1 reason phrase <envoy_v3_api_field_extensions.http.header_formatters.preserve_case.v3.PreserveCaseFormatterConfig.forward_reason_phrase>`.
 * listener: added API for extensions to access :ref:`typed_filter_metadata <envoy_v3_api_field_config.core.v3.Metadata.typed_filter_metadata>` configured in the listener's :ref:`metadata <envoy_v3_api_field_config.listener.v3.Listener.metadata>` field.
 * listener: added support for :ref:`MPTCP <envoy_v3_api_field_config.listener.v3.Listener.enable_mptcp>` (multipath TCP).
 * listener: added support for opting out listeners from the globally set downstream connection limit via :ref:`ignore_global_conn_limit <envoy_v3_api_field_config.listener.v3.Listener.ignore_global_conn_limit>`.

envoy/http/header_formatter.h

@@ -33,6 +33,16 @@ class StatefulHeaderKeyFormatter : public HeaderKeyFormatter {
    * Called for each header key received by the codec.
    */
   virtual void processKey(absl::string_view key) PURE;
+
+  /**
+   * Called to save received reason phrase
+   */
+  virtual void setReasonPhrase(absl::string_view reason_phrase) PURE;
+
+  /**
+   * Called to get saved reason phrase
+   */
+  virtual absl::string_view getReasonPhrase() const PURE;
 };
 
 using StatefulHeaderKeyFormatterPtr = std::unique_ptr<StatefulHeaderKeyFormatter>;

source/common/http/http1/codec_impl.cc

@@ -414,9 +414,15 @@ void ResponseEncoderImpl::encodeHeaders(const ResponseHeaderMap& headers, bool e
   connection_.addIntToBuffer(numeric_status);
   connection_.addCharToBuffer(' ');
 
-  const char* status_string = CodeUtility::toString(static_cast<Code>(numeric_status));
-  uint32_t status_string_len = strlen(status_string);
-  connection_.copyToBuffer(status_string, status_string_len);
+  StatefulHeaderKeyFormatterOptConstRef formatter(headers.formatter());
+
+  if (formatter.has_value() && !formatter->getReasonPhrase().empty()) {
+    connection_.addToBuffer(formatter->getReasonPhrase());
+  } else {
+    const char* status_string = CodeUtility::toString(static_cast<Code>(numeric_status));
+    uint32_t status_string_len = strlen(status_string);
+    connection_.copyToBuffer(status_string, status_string_len);
+  }
 
   connection_.addCharToBuffer('\r');
   connection_.addCharToBuffer('\n');
@@ -1301,6 +1307,16 @@ RequestEncoder& ClientConnectionImpl::newStream(ResponseDecoder& response_decode
   return pending_response_.value().encoder_;
 }
 
+Status ClientConnectionImpl::onStatus(const char* data, size_t length) {
+  auto& headers = absl::get<ResponseHeaderMapPtr>(headers_or_trailers_);
+  StatefulHeaderKeyFormatterOptRef formatter(headers->formatter());
+  if (formatter.has_value()) {
+    formatter->setReasonPhrase(absl::string_view(data, length));
+  }
+
+  return okStatus();
+}
+
 Envoy::StatusOr<ParserStatus> ClientConnectionImpl::onHeadersCompleteBase() {
   ENVOY_CONN_LOG(trace, "status_code {}", connection_, parser_->statusCode());
 

source/common/http/http1/codec_impl.h

@@ -499,6 +499,7 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
 
   // ParserCallbacks.
   Status onUrl(const char* data, size_t length) override;
+  Status onStatus(const char*, size_t) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
   // ConnectionImpl
   void onEncodeComplete() override;
   StreamInfo::BytesMeter& getBytesMeter() override {
@@ -593,6 +594,7 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl {
 
   // ParserCallbacks.
   Status onUrl(const char*, size_t) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
+  Status onStatus(const char* data, size_t length) override;
   // ConnectionImpl
   Http::Status dispatch(Buffer::Instance& data) override;
   void onEncodeComplete() override {}

source/common/http/http1/legacy_parser_impl.cc

@@ -51,7 +51,11 @@ class LegacyHttpParserImpl::Impl {
           auto status = conn_impl->onUrl(at, length);
           return conn_impl->setAndCheckCallbackStatus(std::move(status));
         },
-        nullptr, // on_status
+        [](http_parser* parser, const char* at, size_t length) -> int {
+          auto* conn_impl = static_cast<ParserCallbacks*>(parser->data);
+          auto status = conn_impl->onStatus(at, length);
+          return conn_impl->setAndCheckCallbackStatus(std::move(status));
+        },
         [](http_parser* parser, const char* at, size_t length) -> int {
           auto* conn_impl = static_cast<ParserCallbacks*>(parser->data);
           auto status = conn_impl->onHeaderField(at, length);

source/common/http/http1/parser.h

@@ -72,6 +72,14 @@ class ParserCallbacks {
    */
   virtual Status onHeaderValue(const char* data, size_t length) PURE;
 
+  /**
+   * Called when response status data is received.
+   * @param data supplies the start address.
+   * @param length supplies the length.
+   * @return Status representing success or failure.
+   */
+  virtual Status onStatus(const char* data, size_t length) PURE;
+
   /**
    * Called when headers are complete. A base routine happens first then a virtual dispatch is
    * invoked. Note that this only applies to headers and NOT trailers. End of

source/extensions/http/header_formatters/preserve_case/preserve_case_formatter.cc

@@ -4,12 +4,17 @@
 #include "envoy/extensions/http/header_formatters/preserve_case/v3/preserve_case.pb.validate.h"
 #include "envoy/registry/registry.h"
 
+#include "source/common/protobuf/message_validator_impl.h"
+
 namespace Envoy {
 namespace Extensions {
 namespace Http {
 namespace HeaderFormatters {
 namespace PreserveCase {
 
+PreserveCaseHeaderFormatter::PreserveCaseHeaderFormatter(const bool forward_reason_phrase)
+    : forward_reason_phrase_(forward_reason_phrase) {}
+
 std::string PreserveCaseHeaderFormatter::format(absl::string_view key) const {
   const auto remembered_key_itr = original_header_keys_.find(key);
   // TODO(mattklein123): We can avoid string copies here if the formatter interface allowed us
@@ -34,23 +39,46 @@ void PreserveCaseHeaderFormatter::processKey(absl::string_view key) {
   original_header_keys_.emplace(key);
 }
 
+void PreserveCaseHeaderFormatter::setReasonPhrase(absl::string_view reason_phrase) {
+  if (forward_reason_phrase_) {
+    reason_phrase_ = std::string(reason_phrase);
+  }
+};
+
+absl::string_view PreserveCaseHeaderFormatter::getReasonPhrase() const {
+  return absl::string_view(reason_phrase_);
+};
+
 class PreserveCaseFormatterFactory : public Envoy::Http::StatefulHeaderKeyFormatterFactory {
 public:
+  PreserveCaseFormatterFactory(const bool forward_reason_phrase)
+      : forward_reason_phrase_(forward_reason_phrase) {}
+
   // Envoy::Http::StatefulHeaderKeyFormatterFactory
   Envoy::Http::StatefulHeaderKeyFormatterPtr create() override {
-    return std::make_unique<PreserveCaseHeaderFormatter>();
+    return std::make_unique<PreserveCaseHeaderFormatter>(forward_reason_phrase_);
   }
+
+private:
+  const bool forward_reason_phrase_;
 };
 
 class PreserveCaseFormatterFactoryConfig
     : public Envoy::Http::StatefulHeaderKeyFormatterFactoryConfig {
 public:
   // Envoy::Http::StatefulHeaderKeyFormatterFactoryConfig
   std::string name() const override { return "preserve_case"; }
+
   Envoy::Http::StatefulHeaderKeyFormatterFactorySharedPtr
-  createFromProto(const Protobuf::Message&) override {
-    return std::make_shared<PreserveCaseFormatterFactory>();
+  createFromProto(const Protobuf::Message& message) override {
+    auto config =
+        MessageUtil::downcastAndValidate<const envoy::extensions::http::header_formatters::
+                                             preserve_case::v3::PreserveCaseFormatterConfig&>(
+            message, ProtobufMessage::getStrictValidationVisitor());
+
+    return std::make_shared<PreserveCaseFormatterFactory>(config.forward_reason_phrase());
   }
+
   ProtobufTypes::MessagePtr createEmptyConfigProto() override {
     return std::make_unique<envoy::extensions::http::header_formatters::preserve_case::v3::
                                 PreserveCaseFormatterConfig>();

source/extensions/http/header_formatters/preserve_case/preserve_case_formatter.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "envoy/extensions/http/header_formatters/preserve_case/v3/preserve_case.pb.h"
 #include "envoy/http/header_formatter.h"
 
 #include "source/common/common/utility.h"
@@ -13,11 +14,17 @@ namespace PreserveCase {
 class PreserveCaseHeaderFormatter : public Envoy::Http::StatefulHeaderKeyFormatter {
 public:
   // Envoy::Http::StatefulHeaderKeyFormatter
+  PreserveCaseHeaderFormatter(const bool forward_reason_phrase);
+
   std::string format(absl::string_view key) const override;
   void processKey(absl::string_view key) override;
+  void setReasonPhrase(absl::string_view reason_phrase) override;
+  absl::string_view getReasonPhrase() const override;
 
 private:
   StringUtil::CaseUnorderedSet original_header_keys_;
+  bool forward_reason_phrase_{false};
+  std::string reason_phrase_;
 };
 
 } // namespace PreserveCase

test/extensions/http/header_formatters/preserve_case/BUILD

@@ -35,3 +35,17 @@ envoy_extension_cc_test(
         "//test/integration:http_integration_lib",
     ],
 )
+
+envoy_extension_cc_test(
+    name = "preserve_case_formatter_reason_phrase_integration_test",
+    srcs = [
+        "preserve_case_formatter_reason_phrase_integration_test.cc",
+    ],
+    extension_names = ["envoy.http.stateful_header_formatters.preserve_case"],
+    deps = [
+        "//source/extensions/http/header_formatters/preserve_case:preserve_case_formatter",
+        "//test/integration:http_integration_lib",
+        "//test/integration:http_protocol_integration_lib",
+        "@envoy_api//envoy/extensions/http/header_formatters/preserve_case/v3:pkg_cc_proto",
+    ],
+)

test/extensions/http/header_formatters/preserve_case/preserve_case_formatter_reason_phrase_integration_test.cc

@@ -0,0 +1,93 @@
+#include "envoy/extensions/http/header_formatters/preserve_case/v3/preserve_case.pb.h"
+
+#include "test/integration/filters/common.h"
+#include "test/integration/http_integration.h"
+#include "test/test_common/registry.h"
+
+namespace Envoy {
+namespace {
+
+struct TestParams {
+  Network::Address::IpVersion ip_version;
+  bool forward_reason_phrase;
+};
+
+std::string testParamsToString(const ::testing::TestParamInfo<TestParams>& p) {
+  return fmt::format("{}_{}",
+                     p.param.ip_version == Network::Address::IpVersion::v4 ? "IPv4" : "IPv6",
+                     p.param.forward_reason_phrase ? "enabled" : "disabled");
+}
+
+std::vector<TestParams> getTestsParams() {
+  std::vector<TestParams> ret;
+
+  for (auto ip_version : TestEnvironment::getIpVersionsForTest()) {
+    ret.push_back(TestParams{ip_version, true});
+    ret.push_back(TestParams{ip_version, false});
+  }
+
+  return ret;
+}
+
+class PreserveCaseFormatterReasonPhraseIntegrationTest : public testing::TestWithParam<TestParams>,
+                                                         public HttpIntegrationTest {
+public:
+  PreserveCaseFormatterReasonPhraseIntegrationTest()
+      : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam().ip_version) {}
+
+  void SetUp() override {
+    setDownstreamProtocol(Http::CodecType::HTTP1);
+    setUpstreamProtocol(Http::CodecType::HTTP1);
+  }
+
+  void initialize() override {
+    if (upstreamProtocol() == Http::CodecType::HTTP1) {
+      config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) {
+        ConfigHelper::HttpProtocolOptions protocol_options;
+        auto typed_extension_config = protocol_options.mutable_explicit_http_config()
+                                          ->mutable_http_protocol_options()
+                                          ->mutable_header_key_format()
+                                          ->mutable_stateful_formatter();
+        typed_extension_config->set_name("preserve_case");
+
+        auto config =
+            TestUtility::parseYaml<envoy::extensions::http::header_formatters::preserve_case::v3::
+                                       PreserveCaseFormatterConfig>(fmt::format(
+                "forward_reason_phrase: {}", GetParam().forward_reason_phrase ? "true" : "false"));
+        typed_extension_config->mutable_typed_config()->PackFrom(config);
+
+        ConfigHelper::setProtocolOptions(*bootstrap.mutable_static_resources()->mutable_clusters(0),
+                                         protocol_options);
+      });
+    }
+
+    HttpIntegrationTest::initialize();
+  }
+};
+
+INSTANTIATE_TEST_SUITE_P(CaseFormatter, PreserveCaseFormatterReasonPhraseIntegrationTest,
+                         testing::ValuesIn(getTestsParams()), testParamsToString);
+
+TEST_P(PreserveCaseFormatterReasonPhraseIntegrationTest, VerifyReasonPhraseEnabled) {
+  initialize();
+
+  IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("http"));
+  auto request = "GET / HTTP/1.1\r\nhost: host\r\n\r\n";
+  ASSERT_TRUE(tcp_client->write(request, false));
+
+  Envoy::FakeRawConnectionPtr upstream_connection;
+  ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(upstream_connection));
+
+  auto response = "HTTP/1.1 503 Slow Down\r\ncontent-length: 0\r\n\r\n";
+  ASSERT_TRUE(upstream_connection->write(response));
+
+  auto expected_reason_phrase =
+      GetParam().forward_reason_phrase ? "Slow Down" : "Service Unavailable";
+  // Verify that the downstream response has proper reason phrase
+  tcp_client->waitForData(fmt::format("HTTP/1.1 503 {}", expected_reason_phrase), false);
+
+  tcp_client->close();
+}
+
+} // namespace
+} // namespace Envoy

test/extensions/http/header_formatters/preserve_case/preserve_case_formatter_test.cc

@@ -9,7 +9,7 @@ namespace HeaderFormatters {
 namespace PreserveCase {
 
 TEST(PreserveCaseFormatterTest, All) {
-  PreserveCaseHeaderFormatter formatter;
+  PreserveCaseHeaderFormatter formatter(false);
   formatter.processKey("Foo");
   formatter.processKey("Bar");
   formatter.processKey("BAR");
@@ -22,6 +22,22 @@ TEST(PreserveCaseFormatterTest, All) {
   EXPECT_EQ("baz", formatter.format("baz"));
 }
 
+TEST(PreserveCaseFormatterTest, ReasonPhraseEnabled) {
+  PreserveCaseHeaderFormatter formatter(true);
+
+  formatter.setReasonPhrase(absl::string_view("Slow Down"));
+
+  EXPECT_EQ("Slow Down", formatter.getReasonPhrase());
+}
+
+TEST(PreserveCaseFormatterTest, ReasonPhraseDisabled) {
+  PreserveCaseHeaderFormatter formatter(false);
+
+  formatter.setReasonPhrase(absl::string_view("Slow Down"));
+
+  EXPECT_TRUE(formatter.getReasonPhrase().empty());
+}
+
 } // namespace PreserveCase
 } // namespace HeaderFormatters
 } // namespace Http