custom_response: expose existing local reply body to formatter

Project: envoyproxy/envoy | Language: C++ | Concepts: api-design, error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/envoyproxy/envoy/pull/46555

The problem

When using Envoy's custom response filter with a local response policy, the %LOCAL_REPLY_BODY% formatter variable was always empty unless the policy explicitly configured its own body. This meant that if a local reply already had a body (e.g., set by sendLocalReply()), it could not be accessed or reused by the formatter in the response policy.

Why it's non-trivial

A naive fix might try to always use the existing local reply body, but this could break cases where the policy intentionally sets its own body, which should take precedence. The solution must preserve the correct precedence: an explicitly configured policy body should override any existing reply body, and only fall back to the existing body if the policy doesn't set one.

The approach

1. Documenting the precedence in the API:

-  // Optional new local reply body text. It will be used
-  // in the ``%LOCAL_REPLY_BODY%`` command operator in the ``body_format``.
+  // Optional new local reply body text. It will be used
+  // in the ``%LOCAL_REPLY_BODY%`` command operator in the ``body_format``. If unset when
+  // ``body_format`` is configured for an existing local reply, the existing body will be used as
+  // the formatter input instead.

This clarifies for users that the formatter will use the existing reply body if the policy doesn't set one.

2. Passing the local reply body through the filter chain:

-  StreamFilterBase::LocalReplyData data{code, grpc_status, details, false};
+  StreamFilterBase::LocalReplyData data{code, grpc_status, details, false, body};
   onLocalReply(data);

The LocalReplyData struct now carries the body string, so downstream filters and policies can access it.

3. Making the body available in the filter:

+    // LocalReplyData::body_ is valid only during this callback, so retain an owned copy for the
+    // local response policy to use later in encodeHeaders().
+    local_reply_body_ = std::string(data.body_);

The filter copies the body during onLocalReply() so it can be used later when formatting the response.

4. Using the existing body in the formatter when appropriate:

-  // Handle local body
   std::string body;
+  // When body_format is configured without an explicit body, seed %LOCAL_REPLY_BODY% with the
+  // body captured from the original local reply. Limiting this to the formatter path keeps an
+  // explicit body authoritative, while policies with neither body nor body_format retain their
+  // existing empty body behavior.
+  if (formatter_ && !local_body_.has_value()) {
+    body = std::string(custom_response_filter.localReplyBody());
+  }

This ensures that the existing body is only used if no explicit body is set in the policy.

5. Updating tests to cover the new behavior and precedence:

+TEST_F(CustomResponseFilterTest, ExistingLocalReplyBodyWithFormatter) {
+  // ... setup omitted ...
+  const ::Envoy::Http::StreamFilterBase::LocalReplyData local_reply_data{
+      ::Envoy::Http::Code::Unauthorized, std::nullopt, "details", false, "original body"};
+  EXPECT_EQ(filter_->onLocalReply(local_reply_data), ::Envoy::Http::LocalErrorStatus::Continue);
+  // ... asserts that "original body" is used ...
+}
+
+TEST_F(CustomResponseFilterTest, ConfiguredBodyTakesPrecedenceOverExistingLocalReplyBody) {
+  // ... setup omitted ...
+  const ::Envoy::Http::StreamFilterBase::LocalReplyData local_reply_data{
+      ::Envoy::Http::Code::Unauthorized, std::nullopt, "details", false, "original body"};
+  EXPECT_EQ(filter_->onLocalReply(local_reply_data), ::Envoy::Http::LocalErrorStatus::Continue);
+  // ... asserts that "configured body" is used, not "original body" ...
+}

These tests verify that the fallback and precedence logic works as intended.

From the review thread

No notable review discussion on this PR.

Takeaways

• When extending APIs or behaviors, document precedence and fallback logic clearly to avoid ambiguity for users and maintainers.
• When introducing new data flow (like passing the body through callbacks), ensure lifetime and ownership are handled to prevent invalid memory access.
• Always add or update tests to cover new code paths and edge cases, especially when changing precedence or fallback logic.