Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a release note for this please

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

// This is off by default, and a standard reason phrase is used for a corresponding HTTP response code.
bool forward_reason_phrase = 1;
}
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,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).
* oauth filter: added :ref:`cookie_names <envoy_v3_api_field_extensions.filters.http.oauth2.v3.OAuth2Credentials.cookie_names>` to allow overriding (default) cookie names (``BearerToken``, ``OauthHMAC``, and ``OauthExpires``) set by the filter.
Expand Down
10 changes: 10 additions & 0 deletions envoy/http/header_formatter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
22 changes: 19 additions & 3 deletions source/common/http/http1/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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());

Expand Down
2 changes: 2 additions & 0 deletions source/common/http/http1/codec_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {}
Expand Down
6 changes: 5 additions & 1 deletion source/common/http/http1/legacy_parser_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions source/common/http/http1/parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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>();
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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
Expand Down
14 changes: 14 additions & 0 deletions test/extensions/http/header_formatters/preserve_case/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace HeaderFormatters {
namespace PreserveCase {

TEST(PreserveCaseFormatterTest, All) {
PreserveCaseHeaderFormatter formatter;
PreserveCaseHeaderFormatter formatter(false);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you verify that when this is false, and you set a reason phrase, you don't get a reason phrase back?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added another test case for this.

formatter.processKey("Foo");
formatter.processKey("Bar");
formatter.processKey("BAR");
Expand All @@ -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
Expand Down