Skip to content

Commit

Permalink
Revert "http: Introduce preserve_upstream_date option (#11077)"
Browse files Browse the repository at this point in the history
This reverts commit 10c755e.

Signed-off-by: Matt Klein <mklein@lyft.com>
  • Loading branch information
mattklein123 committed May 8, 2020
1 parent 49efb98 commit c293b94
Show file tree
Hide file tree
Showing 13 changed files with 2 additions and 121 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -507,11 +507,6 @@ 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;

// Determines if the port part should be removed from host/authority header before any processing
// of request by HTTP filters or routing. The port would be removed only if it is equal to the :ref:`listener's<envoy_api_field_config.listener.v3.Listener.address>`
// local port and request method is not CONNECT. This affects the upstream host header as well.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,11 +507,6 @@ 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;

// Determines if the port part should be removed from host/authority header before any processing
// of request by HTTP filters or routing. The port would be removed only if it is equal to the :ref:`listener's<envoy_api_field_config.listener.v4alpha.Listener.address>`
// local port and request method is not CONNECT. This affects the upstream host header as well.
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: 0 additions & 6 deletions source/common/http/conn_manager_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -435,12 +435,6 @@ 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: 1 addition & 3 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1631,9 +1631,7 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilte
void ConnectionManagerImpl::ActiveStream::encodeHeadersInternal(ResponseHeaderMap& headers,
bool end_stream) {
// Base headers.
if (!connection_manager_.config_.shouldPreserveUpstreamDate() || !headers.Date()) {
connection_manager_.config_.dateProvider().setDateHeader(headers);
}
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 @@ -218,8 +218,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
merge_slashes_(config.merge_slashes()),
strip_matching_port_(config.strip_matching_host_port()),
headers_with_underscores_action_(
config.common_http_protocol_options().headers_with_underscores_action()),
preserve_upstream_date_(config.preserve_upstream_date()) {
config.common_http_protocol_options().headers_with_underscores_action()) {
// 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 @@ -163,7 +163,6 @@ 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 @@ -229,7 +228,6 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
const bool strip_matching_port_;
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: 0 additions & 1 deletion source/server/http/admin.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ 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: 0 additions & 1 deletion test/common/http/conn_manager_impl_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ 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: 0 additions & 52 deletions test/common/http/conn_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,6 @@ 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 @@ -417,7 +416,6 @@ 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 @@ -975,56 +973,6 @@ TEST_F(HttpConnectionManagerImplTest, RouteShouldUseNormalizedHost) {
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: 0 additions & 1 deletion test/common/http/conn_manager_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig {
MOCK_METHOD(bool, shouldStripMatchingPort, (), (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 @@ -1130,39 +1130,6 @@ 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 c293b94

Please sign in to comment.