Skip to content

Commit

Permalink
http: Introduce preserve_upstream_date option (#11077)
Browse files Browse the repository at this point in the history
The preserve_upstream_date option allows the HTTP Connection Manager to be configured to pass through the original date header from the upstream response rather than overwriting it. The default behaviour for the date response header remains the same as before -- the header value will be overwritten by Envoy.

Signed-off-by: Craig Radcliffe <craig.radcliffe@broadcom.com>
  • Loading branch information
chradcliffe authored May 8, 2020
1 parent e6e37da commit 10c755e
Show file tree
Hide file tree
Showing 13 changed files with 125 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// HTTP connection manager :ref:`configuration overview <config_http_conn_man>`.
// [#extension: envoy.filters.network.http_connection_manager]

// [#next-free-field: 38]
// [#next-free-field: 39]
message HttpConnectionManager {
option (udpa.annotations.versioning).previous_message_type =
"envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager";
Expand Down Expand Up @@ -506,6 +506,11 @@ message HttpConnectionManager {
//
// 3. Tracing decision (sampled, forced, etc) is set in 14th byte of the UUID.
RequestIDExtension request_id_extension = 36;

// If `preserve_upstream_date` is true, the value of the `date` header sent by the upstream
// host will not be overwritten by the HTTP Connection Manager. The default behaviour is
// to overwrite the `date` header unconditionally.
bool preserve_upstream_date = 38;
}

message Rds {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO
// HTTP connection manager :ref:`configuration overview <config_http_conn_man>`.
// [#extension: envoy.filters.network.http_connection_manager]

// [#next-free-field: 38]
// [#next-free-field: 39]
message HttpConnectionManager {
option (udpa.annotations.versioning).previous_message_type =
"envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager";
Expand Down Expand Up @@ -506,6 +506,11 @@ message HttpConnectionManager {
//
// 3. Tracing decision (sampled, forced, etc) is set in 14th byte of the UUID.
RequestIDExtension request_id_extension = 36;

// If `preserve_upstream_date` is true, the value of the `date` header sent by the upstream
// host will not be overwritten by the HTTP Connection Manager. The default behaviour is
// to overwrite the `date` header unconditionally.
bool preserve_upstream_date = 38;
}

message Rds {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions source/common/http/conn_manager_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,12 @@ class ConnectionManagerConfig {
*/
virtual envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
headersWithUnderscoresAction() const PURE;

/**
* @return if the HttpConnectionManager should preserve the `date` response header sent by the
* upstream host.
*/
virtual bool shouldPreserveUpstreamDate() const PURE;
};
} // namespace Http
} // namespace Envoy
4 changes: 3 additions & 1 deletion source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1620,7 +1620,9 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilte
void ConnectionManagerImpl::ActiveStream::encodeHeadersInternal(ResponseHeaderMap& headers,
bool end_stream) {
// Base headers.
connection_manager_.config_.dateProvider().setDateHeader(headers);
if (!connection_manager_.config_.shouldPreserveUpstreamDate() || !headers.Date()) {
connection_manager_.config_.dateProvider().setDateHeader(headers);
}
// Following setReference() is safe because serverName() is constant for the life of the listener.
const auto transformation = connection_manager_.config_.serverHeaderTransformation();
if (transformation == ConnectionManagerConfig::HttpConnectionManagerProto::OVERWRITE ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
#endif
merge_slashes_(config.merge_slashes()),
headers_with_underscores_action_(
config.common_http_protocol_options().headers_with_underscores_action()) {
config.common_http_protocol_options().headers_with_underscores_action()),
preserve_upstream_date_(config.preserve_upstream_date()) {
// If idle_timeout_ was not configured in common_http_protocol_options, use value in deprecated
// idle_timeout field.
// TODO(asraa): Remove when idle_timeout is removed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
return headers_with_underscores_action_;
}
std::chrono::milliseconds delayedCloseTimeout() const override { return delayed_close_timeout_; }
bool shouldPreserveUpstreamDate() const override { return preserve_upstream_date_; }

private:
enum class CodecType { HTTP1, HTTP2, HTTP3, AUTO };
Expand Down Expand Up @@ -226,6 +227,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
const bool merge_slashes_;
const envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
headers_with_underscores_action_;
const bool preserve_upstream_date_;

// Default idle timeout is 5 minutes if nothing is specified in the HCM config.
static const uint64_t StreamIdleTimeoutMs = 5 * 60 * 1000;
Expand Down
1 change: 1 addition & 0 deletions source/server/http/admin.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ class AdminImpl : public Admin,
headersWithUnderscoresAction() const override {
return envoy::config::core::v3::HttpProtocolOptions::ALLOW;
}
bool shouldPreserveUpstreamDate() const override { return false; }
Http::Code request(absl::string_view path_and_query, absl::string_view method,
Http::ResponseHeaderMap& response_headers, std::string& body) override;
void closeSocket();
Expand Down
1 change: 1 addition & 0 deletions test/common/http/conn_manager_impl_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ class FuzzConfig : public ConnectionManagerConfig {
headersWithUnderscoresAction() const override {
return envoy::config::core::v3::HttpProtocolOptions::ALLOW;
}
bool shouldPreserveUpstreamDate() const override { return false; }

const envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager
config_;
Expand Down
52 changes: 52 additions & 0 deletions test/common/http/conn_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan
headersWithUnderscoresAction() const override {
return headers_with_underscores_action_;
}
bool shouldPreserveUpstreamDate() const override { return preserve_upstream_date_; }

Envoy::Event::SimulatedTimeSystem test_time_;
NiceMock<Router::MockRouteConfigProvider> route_config_provider_;
Expand Down Expand Up @@ -414,6 +415,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan
NiceMock<Network::MockClientConnection> upstream_conn_; // for websocket tests
NiceMock<Tcp::ConnectionPool::MockInstance> conn_pool_; // for websocket tests
RequestIDExtensionSharedPtr request_id_extension_;
bool preserve_upstream_date_ = false;

// TODO(mattklein123): Not all tests have been converted over to better setup. Convert the rest.
MockResponseEncoder response_encoder_;
Expand Down Expand Up @@ -897,6 +899,56 @@ TEST_F(HttpConnectionManagerImplTest, RouteShouldUseSantizedPath) {
conn_manager_->onData(fake_input, false);
}

TEST_F(HttpConnectionManagerImplTest, PreserveUpstreamDateDisabledDateNotSet) {
setup(false, "");
setUpEncoderAndDecoder(false, false);
sendRequestHeadersAndData();
preserve_upstream_date_ = false;
const auto* modified_headers = sendResponseHeaders(
ResponseHeaderMapPtr{new TestResponseHeaderMapImpl{{":status", "200"}, {"server", "foo"}}});
ASSERT_TRUE(modified_headers);
EXPECT_TRUE(modified_headers->Date());
}

TEST_F(HttpConnectionManagerImplTest, PreserveUpstreamDateDisabledDateSet) {
setup(false, "");
setUpEncoderAndDecoder(false, false);
sendRequestHeadersAndData();
preserve_upstream_date_ = false;
const std::string expected_date{"Tue, 15 Nov 1994 08:12:31 GMT"};
const auto* modified_headers =
sendResponseHeaders(ResponseHeaderMapPtr{new TestResponseHeaderMapImpl{
{":status", "200"}, {"server", "foo"}, {"date", expected_date.c_str()}}});
ASSERT_TRUE(modified_headers);
ASSERT_TRUE(modified_headers->Date());
EXPECT_NE(expected_date, modified_headers->Date()->value().getStringView());
}

TEST_F(HttpConnectionManagerImplTest, PreserveUpstreamDateEnabledDateNotSet) {
setup(false, "");
setUpEncoderAndDecoder(false, false);
sendRequestHeadersAndData();
preserve_upstream_date_ = true;
const auto* modified_headers = sendResponseHeaders(
ResponseHeaderMapPtr{new TestResponseHeaderMapImpl{{":status", "200"}, {"server", "foo"}}});
ASSERT_TRUE(modified_headers);
EXPECT_TRUE(modified_headers->Date());
}

TEST_F(HttpConnectionManagerImplTest, PreserveUpstreamDateEnabledDateSet) {
setup(false, "");
setUpEncoderAndDecoder(false, false);
sendRequestHeadersAndData();
preserve_upstream_date_ = true;
const std::string expected_date{"Tue, 15 Nov 1994 08:12:31 GMT"};
const auto* modified_headers =
sendResponseHeaders(ResponseHeaderMapPtr{new TestResponseHeaderMapImpl{
{":status", "200"}, {"server", "foo"}, {"date", expected_date.c_str()}}});
ASSERT_TRUE(modified_headers);
ASSERT_TRUE(modified_headers->Date());
EXPECT_EQ(expected_date, modified_headers->Date()->value().getStringView());
}

TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlow) {
setup(false, "");

Expand Down
1 change: 1 addition & 0 deletions test/common/http/conn_manager_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig {
MOCK_METHOD(bool, shouldMergeSlashes, (), (const));
MOCK_METHOD(envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction,
headersWithUnderscoresAction, (), (const));
MOCK_METHOD(bool, shouldPreserveUpstreamDate, (), (const));

std::unique_ptr<Http::InternalAddressConfig> internal_address_config_ =
std::make_unique<DefaultInternalAddressConfig>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1080,6 +1080,39 @@ TEST_F(HttpConnectionManagerConfigTest, UnconfiguredRequestTimeout) {
EXPECT_EQ(0, config.requestTimeout().count());
}

TEST_F(HttpConnectionManagerConfigTest, DisabledPreserveResponseDate) {
const std::string yaml_string = R"EOF(
stat_prefix: ingress_http
request_timeout: 0s
route_config:
name: local_route
http_filters:
- name: envoy.filters.http.router
)EOF";

HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_,
date_provider_, route_config_provider_manager_,
scoped_routes_config_provider_manager_, http_tracer_manager_);
EXPECT_FALSE(config.shouldPreserveUpstreamDate());
}

TEST_F(HttpConnectionManagerConfigTest, EnabledPreserveResponseDate) {
const std::string yaml_string = R"EOF(
stat_prefix: ingress_http
request_timeout: 0s
route_config:
name: local_route
http_filters:
- name: envoy.filters.http.router
preserve_upstream_date: true
)EOF";

HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_,
date_provider_, route_config_provider_manager_,
scoped_routes_config_provider_manager_, http_tracer_manager_);
EXPECT_TRUE(config.shouldPreserveUpstreamDate());
}

TEST_F(HttpConnectionManagerConfigTest, SingleDateProvider) {
const std::string yaml_string = R"EOF(
codec_type: http1
Expand Down

0 comments on commit 10c755e

Please sign in to comment.