Skip to content

dynamic modules: keep C++ SDK response callbacks after a local reply - #46902

Draft
derekargueta wants to merge 1 commit into
envoyproxy:mainfrom
derekargueta:dargueta-dynmod-cpp-local-reply-callbacks
Draft

dynamic modules: keep C++ SDK response callbacks after a local reply#46902
derekargueta wants to merge 1 commit into
envoyproxy:mainfrom
derekargueta:dargueta-dynmod-cpp-local-reply-callbacks

Conversation

@derekargueta

@derekargueta derekargueta commented Aug 24, 2026

Copy link
Copy Markdown
Member

Commit Message: dynamic modules: keep C++ SDK response callbacks after a local reply
Additional Description:

Fixes #46901. The C++ SDK used one local_reply_sent_ flag for two things. sendLocalResponse() sets it, which is right, since the module generated the response and skipping its own response callbacks avoids re-entrancy. But the on_http_filter_local_reply notification set it too, and that fires for every local reply whatever the source, after which the response headers, body and trailers entry points all returned early without calling into the module.

So a C++ dynamic module couldn't see or modify the response on a direct_response route, a local reply from another filter, or an error Envoy generated itself. A module that injects response headers worked on proxied traffic and silently did nothing on error paths. The Rust and Go SDKs have no equivalent flag and were unaffected.

This drops the assignment in the notification handler, so the flag means what its name says and onLocalReply stays a pure notification, matching the other two SDKs.

No HTTP dynamic-modules integration test used a direct_response route before this, which is why the path was uncovered. Adding that coverage also turned up #46905, a separate pre-existing bug where a Go module hangs the request on such a route. That reproduces with the in-tree passthrough filter and is unrelated to this change, so the new test skips the go param and cites the issue.

Risk Level: Low. Removes one assignment in the C++ SDK; no ABI or config surface changes, and the other SDKs are untouched.
Testing: New ResponseCallbacksOnLocalReply integration test over a direct_response route, with a minimal local_reply_response_headers filter in the rust, go and cpp test data so it covers every param rather than only the SDK that was broken. Locally: rust, rust_static and cpp pass, go skipped per #46905. Verified as a real regression test by restoring the removed line, where cpp then fails. Also checked out of tree against v1.39.0, where a C++ module on a direct_response route sets its response header with this change and sets nothing without it.
Docs Changes: None.
Release Notes: changelogs/current/bug_fixes/dynamic_modules__cpp-sdk-response-callbacks-after-local-reply.rst

@repokitteh-read-only

Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #46902 was opened by derekargueta.

see: more, trace.

The C++ SDK used one local_reply_sent_ flag for two things. sendLocalResponse()
sets it, which is right: the module generated the response, so skipping its own
response callbacks avoids re-entrancy. But the on_http_filter_local_reply
notification set it too, and that fires for every local reply whatever the
source. After that the response headers, body and trailers entry points all
returned early without calling into the module.

So a C++ dynamic module could not see or modify the response on a
direct_response route, a local reply from another filter, or an error Envoy
generated itself. A module that injects response headers worked on proxied
traffic and silently did nothing on error paths. The Rust and Go SDKs have no
equivalent flag and were unaffected.

Drops the assignment in the notification handler, so the flag now means what its
name says and onLocalReply is a pure notification, matching the other two SDKs.

Adds a local_reply_response_headers test filter to all three language test data
modules and one integration test over a direct_response route. It runs for
rust, rust_static, go and cpp, so it also pins the parity rather than only
covering the SDK that was broken.

Fixes envoyproxy#46901

Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
@derekargueta
derekargueta force-pushed the dargueta-dynmod-cpp-local-reply-callbacks branch from 9795acf to 8cba6e6 Compare August 24, 2026 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C++ dynamic-modules SDK: response callbacks are permanently suppressed after any local reply, including ones the module did not send

1 participant