diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index ff377b04e18c7..fe23529924af7 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -20,6 +20,7 @@ Minor Behavior Changes * http: added :ref:`headers_to_add ` to :ref:`local reply mapper ` to allow its users to add/append/override response HTTP headers to local replies. * http: added HCM level configuration of :ref:`error handling on invalid messaging ` which substantially changes Envoy's behavior when encountering invalid HTTP/1.1 defaulting to closing the connection instead of allowing reuse. This can temporarily be reverted by setting `envoy.reloadable_features.hcm_stream_error_on_invalid_message` to false, or permanently reverted by setting the HTTP/1 configuration :ref:`override_stream_error_on_invalid_http_message ` to true to restore prior HTTP/1.1 behavior (i.e. connection isn't terminated) and to retain prior HTTP/2 behavior (i.e. connection is terminated). * http: added HCM level configuration of :ref:`error handling on invalid messaging ` which substantially changes Envoy's behavior when encountering invalid HTTP/1.1 defaulting to closing the connection instead of allowing reuse. This can temporarily be reverted by setting `envoy.reloadable_features.hcm_stream_error_on_invalid_message` to false, or permanently reverted by setting the :ref:`HCM option ` to true to restore prior HTTP/1.1 beavior and setting the *new* HTTP/2 configuration :ref:`override_stream_error_on_invalid_http_message ` to false to retain prior HTTP/2 behavior. +* http: applying route level header modifications to local replies sent on that route. This behavior may be temporarily reverted by setting `envoy.reloadable_features.always_apply_route_header_rules` to false. * http: changed Envoy to send GOAWAY to HTTP2 downstreams when the :ref:`disable_keepalive ` overload action is active. This behavior may be temporarily reverted by setting `envoy.reloadable_features.overload_manager_disable_keepalive_drain_http2` to false. * http: changed Envoy to send error headers and body when possible. This behavior may be temporarily reverted by setting `envoy.reloadable_features.allow_response_for_timeout` to false. * http: changed empty trailers encoding behavior by sending empty data with ``end_stream`` true (instead of sending empty trailers) for HTTP/2. This behavior can be reverted temporarily by setting runtime feature ``envoy.reloadable_features.http2_skip_encoding_empty_trailers`` to false. diff --git a/source/common/http/filter_manager.cc b/source/common/http/filter_manager.cc index 77b28f415e4f3..ebc51191db5b6 100644 --- a/source/common/http/filter_manager.cc +++ b/source/common/http/filter_manager.cc @@ -812,7 +812,16 @@ void FilterManager::sendLocalReplyViaFilterChain( Utility::sendLocalReply( state_.destroyed_, Utility::EncodeFunctions{ - modify_headers, + [this, modify_headers](ResponseHeaderMap& headers) -> void { + if (streamInfo().route_entry_ && + Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.always_apply_route_header_rules")) { + streamInfo().route_entry_->finalizeResponseHeaders(headers, streamInfo()); + } + if (modify_headers) { + modify_headers(headers); + } + }, [this](ResponseHeaderMap& response_headers, Code& code, std::string& body, absl::string_view& content_type) -> void { // TODO(snowp): This &get() business isn't nice, rework LocalReply and others to accept @@ -846,7 +855,16 @@ void FilterManager::sendDirectLocalReply( Http::Utility::sendLocalReply( state_.destroyed_, Utility::EncodeFunctions{ - modify_headers, + [this, modify_headers](ResponseHeaderMap& headers) -> void { + if (streamInfo().route_entry_ && + Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.always_apply_route_header_rules")) { + streamInfo().route_entry_->finalizeResponseHeaders(headers, streamInfo()); + } + if (modify_headers) { + modify_headers(headers); + } + }, [&](ResponseHeaderMap& response_headers, Code& code, std::string& body, absl::string_view& content_type) -> void { local_reply_.rewrite(filter_manager_callbacks_.requestHeaders().has_value() diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 9070056802837..952a622894be6 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -57,6 +57,7 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.test_feature_true", // Begin alphabetically sorted section. "envoy.deprecated_features.allow_deprecated_extension_names", + "envoy.reloadable_features.always_apply_route_header_rules", "envoy.reloadable_features.activate_fds_next_event_loop", "envoy.reloadable_features.activate_timers_next_event_loop", "envoy.reloadable_features.allow_500_after_100", diff --git a/test/integration/idle_timeout_integration_test.cc b/test/integration/idle_timeout_integration_test.cc index 963b04660f5a3..1f549adafddf1 100644 --- a/test/integration/idle_timeout_integration_test.cc +++ b/test/integration/idle_timeout_integration_test.cc @@ -26,6 +26,10 @@ class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest { auto* route = virtual_host->mutable_routes(0)->mutable_route(); route->mutable_idle_timeout()->set_seconds(0); route->mutable_idle_timeout()->set_nanos(IdleTimeoutMs * 1000 * 1000); + + auto* header = virtual_host->mutable_response_headers_to_add()->Add()->mutable_header(); + header->set_key("foo"); + header->set_value("bar"); } if (enable_request_timeout_) { hcm.mutable_request_timeout()->set_seconds(0); @@ -178,6 +182,9 @@ TEST_P(IdleTimeoutIntegrationTest, PerStreamIdleTimeoutAfterDownstreamHeaders) { EXPECT_EQ(0U, upstream_request_->bodyLength()); EXPECT_TRUE(response->complete()); EXPECT_EQ("408", response->headers().getStatusValue()); + auto foo = Http::LowerCaseString("foo"); + ASSERT_TRUE(response->headers().get(foo) != nullptr); + EXPECT_EQ("bar", response->headers().get(foo)->value().getStringView()); EXPECT_EQ("stream timeout", response->body()); EXPECT_THAT(waitForAccessLog(access_log_name_), HasSubstr("stream_idle_timeout")); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index e6a584c803c2e..da326a437e8c3 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -854,6 +854,16 @@ TEST_P(DownstreamProtocolIntegrationTest, HittingDecoderFilterLimit) { // Test hitting the encoder buffer filter with too many response bytes to buffer. Given the request // headers are sent on early, the stream/connection will be reset. TEST_P(ProtocolIntegrationTest, HittingEncoderFilterLimit) { + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) -> void { + auto* route_config = hcm.mutable_route_config(); + auto* virtual_host = route_config->mutable_virtual_hosts(0); + auto* header = virtual_host->mutable_response_headers_to_add()->Add()->mutable_header(); + header->set_key("foo"); + header->set_value("bar"); + }); + useAccessLog(); config_helper_.addFilter("{ name: encoder-decoder-buffer-filter, typed_config: { \"@type\": " "type.googleapis.com/google.protobuf.Empty } }"); @@ -886,6 +896,11 @@ TEST_P(ProtocolIntegrationTest, HittingEncoderFilterLimit) { response->waitForEndStream(); EXPECT_TRUE(response->complete()); EXPECT_EQ("500", response->headers().getStatusValue()); + // Regression test all sendLocalReply paths add route-requested headers. + auto foo = Http::LowerCaseString("foo"); + ASSERT_TRUE(response->headers().get(foo) != nullptr); + EXPECT_EQ("bar", response->headers().get(foo)->value().getStringView()); + // Regression test https://github.com/envoyproxy/envoy/issues/9881 by making // sure this path does standard HCM header transformations. EXPECT_TRUE(response->headers().Date() != nullptr);