http: respecting configured added content encoding on local replies

docs/root/version_history/current.rst

@@ -28,6 +28,8 @@ Minor Behavior Changes
 * http: changed the http status code to 504 from 408 if the request timeouts after the request is completed. This behavior can be temporarily reverted by setting the runtime guard ``envoy.reloadable_features.override_request_timeout_by_gateway_timeout`` to false.
 * http: lazy disable downstream connection reading in the HTTP/1 codec to reduce unnecessary system calls. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.http1_lazy_read_disable`` to false.
 * http: now the max concurrent streams of http2 connection can not only be adjusted down according to the SETTINGS frame but also can be adjusted up, of course, it can not exceed the configured upper bounds. This fix is guarded by ``envoy.reloadable_features.http2_allow_capacity_increase_by_settings``.
+* http: respecting ``content-type`` in :ref:`headers_to_add <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.ResponseMapper.headers_to_add>` even when the response body is modified. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.allow_adding_content_type_in_local_replies`` to false.
+* http: when writing custom filters, `injectEncodedDataToFilterChain` and `injectDecodedDataToFilterChain` now trigger sending of headers if they were not yet sent due to `StopIteration`. Previously, calling one of the inject functions in that state would trigger an assertion. See issue #19891 for more details.
 * http: when writing custom filters, ``injectEncodedDataToFilterChain`` and ``injectDecodedDataToFilterChain`` now trigger sending of headers if they were not yet sent due to ``StopIteration``. Previously, calling one of the inject functions in that state would trigger an assertion. See issue #19891 for more details.
 * listener: the :ref:`ipv4_compat <envoy_api_field_core.SocketAddress.ipv4_compat>` flag can only be set on Ipv6 address and Ipv4-mapped Ipv6 address. A runtime guard is added ``envoy.reloadable_features.strict_check_on_ipv4_compat`` and the default is true.
 * network: add a new ConnectionEvent ``ConnectedZeroRtt`` which may be raised by QUIC connections to allow early data to be sent before the handshake finishes. This event is ignored at callsites which is only reachable for TCP connections in the Envoy core code. Any extensions which depend on ConnectionEvent enum value should audit their usage of it to make sure this new event is handled appropriately.

source/common/http/utility.cc

@@ -584,8 +584,11 @@ void Utility::sendLocalReply(const bool& is_reset, const EncodeFunctions& encode
   if (encode_functions.modify_headers_) {
     encode_functions.modify_headers_(*response_headers);
   }
+  bool has_custom_content_type = false;
   if (encode_functions.rewrite_) {
+    std::string content_type_value = std::string(response_headers->getContentTypeValue());
     encode_functions.rewrite_(*response_headers, response_code, body_text, content_type);
+    has_custom_content_type = (content_type_value != response_headers->getContentTypeValue());
   }
 
   // Respond with a gRPC trailers-only response if the request is gRPC
@@ -619,12 +622,19 @@ void Utility::sendLocalReply(const bool& is_reset, const EncodeFunctions& encode
 
   if (!body_text.empty()) {
     response_headers->setContentLength(body_text.size());
-    // If the `rewrite` function has changed body_text or content-type is not set, set it.
-    // This allows `modify_headers` function to set content-type for the body. For example,
-    // router.direct_response is calling sendLocalReply and may need to set content-type for
-    // the body.
-    if (body_text != local_reply_data.body_text_ || response_headers->ContentType() == nullptr) {
-      response_headers->setReferenceContentType(content_type);
+    // If the content-type is not set, set it.
+    // Alternately if the `rewrite` function has changed body_text and the config didn't explicitly
+    // set a content type header, set the content type to be based on the changed body.
+    if (Runtime::runtimeFeatureEnabled(
+            "envoy.reloadable_features.allow_adding_content_type_in_local_replies")) {
+      if (response_headers->ContentType() == nullptr ||
+          (body_text != local_reply_data.body_text_ && !has_custom_content_type)) {
+        response_headers->setReferenceContentType(content_type);
+      }
+    } else {
+      if (body_text != local_reply_data.body_text_ || response_headers->ContentType() == nullptr) {
+        response_headers->setReferenceContentType(content_type);
+      }
     }
   } else {
     response_headers->removeContentLength();

source/common/runtime/runtime_features.cc

@@ -29,6 +29,7 @@
 // If issues are found that require a runtime feature to be disabled, it should be reported
 // ASAP by filing a bug on github. Overriding non-buggy code is strongly discouraged to avoid the
 // problem of the bugs being found after the old code path has been removed.
+RUNTIME_GUARD(envoy_reloadable_features_allow_adding_content_type_in_local_replies);
 RUNTIME_GUARD(envoy_reloadable_features_allow_upstream_inline_write);
 RUNTIME_GUARD(envoy_reloadable_features_append_or_truncate);
 RUNTIME_GUARD(envoy_reloadable_features_append_to_accept_content_encoding_only_once);

test/integration/local_reply_integration_test.cc

@@ -34,6 +34,10 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson) {
           key: foo
           value: bar
         append: false
+      - header:
+          key: content-type
+          value: "application/json-custom"
+        append: false
 body_format:
   json_format:
     level: TRACE
@@ -78,7 +82,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson) {
   EXPECT_EQ(0U, upstream_request_->bodyLength());
 
   EXPECT_TRUE(response->complete());
-  EXPECT_EQ("application/json", response->headers().ContentType()->value().getStringView());
+  EXPECT_EQ("application/json-custom", response->headers().ContentType()->value().getStringView());
   EXPECT_EQ("150", response->headers().ContentLength()->value().getStringView());
   EXPECT_EQ("550", response->headers().Status()->value().getStringView());
   EXPECT_EQ(response->headers().getProxyStatusValue(),