http: fix filter manager dropping a body frame on continue (#46842) - #619
Merged
jwendell merged 1 commit intoSep 16, 2026
Conversation
Member
Author
|
Depends on #618 to be merged first; Then I'll rebase and fix some conflicts/overlapping. |
Backport to release/v1.35. This is a cherry-pick of the release/v1.36 commit 0d9b3a8, which in turn cherry-picked the main-branch commit 0871c75a6f329465c28cc6280cc904dc746d151b. When an HTTP filter that is already iterating moves the current body frame into the filter-manager buffer via `addDecodedData()`/`addEncodedData()` and then returns `Continue`, the buffered frame was silently dropped: the now-empty frame was forwarded down the chain instead of the just-buffered bytes. This corrupts streamed request/response bodies. It was observed by the Kuadrant wasm-shim project, where a wasm filter using `allow_on_headers_stop_iteration` stops on headers, resumes asynchronously, and then buffers a body chunk before continuing; chained ahead of an `ext_proc` filter in `FULL_DUPLEX_STREAMED` mode, one body chunk was lost end-to-end. The defect is in core `commonHandleAfterDataCallback()` and is not wasm- or ext_proc-specific. The `Continue` path now forwards the filter-manager buffer instead of the empty frame when the filter drained the frame while already iterating. The behavior is guarded by the runtime feature `envoy.reloadable_features.filter_manager_forward_added_data_on_continue` (default-on) so it can be reverted if needed. Adds a `FilterManager` unit test covering both guard states and an `ext_proc` `FULL_DUPLEX_STREAMED` integration test (with a new `add-data-and-continue-filter` test filter) asserting byte-exact body delivery. Backport notes (changes needed to resolve conflicts against 1.35): - The source fix (filter_manager.cc/.h), the FilterManager unit test, the new `add-data-and-continue-filter` test filter and its BUILD wiring all applied cleanly. - `source/common/runtime/runtime_features.cc`: only added the `filter_manager_forward_added_data_on_continue` guard; the unrelated `generic_proxy_codec_buffer_limit` and `ext_proc_inject_data_with_state_update` guards in the surrounding 1.36 context do not exist on 1.35 and were dropped. - 1.35 predates the 1.36 ext_proc test refactor, so it has no `ext_proc_full_duplex_integration_test.cc`. That file was dropped and its single new test `AddDataAndContinueBeforeExtProcDuplexStreamed` was ported into 1.35's monolithic `ext_proc_integration_test.cc`, adapting it to the duplex-streamed helper names/signatures present on 1.35 (`serverReceiveHeaderReq`, `serverSendHeaderResp`, and the `serverReceiveBodyDuplexStreamed`/`serverSendBodyRespDuplexStreamed` helpers that take the processor stream as an argument). Closes: #46841 Signed-off-by: Jonh Wendell <jwendell@redhat.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jwendell
force-pushed
the
backport-filter-manager-body-frame-1.35
branch
from
September 16, 2026 12:30
b40c7a3 to
2cf6b96
Compare
jwendell
marked this pull request as ready for review
September 16, 2026 12:30
dcillera
self-requested a review
September 16, 2026 12:39
|
This is a clean, faithful cherry-pick of an upstream-reviewed fix (envoy #46842/#46841) that addresses data loss in the HTTP filter manager. The core production change is small and sound:
The related test: have been successfully executed locally. |
dcillera
approved these changes
Sep 16, 2026
jwendell
added a commit
that referenced
this pull request
Sep 24, 2026
…619) (#621) Backport to release/v1.34. This is a cherry-pick of the release/v1.35 commit 6101f0c, which in turn backported the release/v1.36 commit 0d9b3a8, itself a cherry-pick of the main-branch commit 0871c75a6f329465c28cc6280cc904dc746d151b. When an HTTP filter that is already iterating moves the current body frame into the filter-manager buffer via `addDecodedData()`/`addEncodedData()` and then returns `Continue`, the buffered frame was silently dropped: the now-empty frame was forwarded down the chain instead of the just-buffered bytes. This corrupts streamed request/response bodies. It was observed by the Kuadrant wasm-shim project, where a wasm filter using `allow_on_headers_stop_iteration` stops on headers, resumes asynchronously, and then buffers a body chunk before continuing; chained ahead of an `ext_proc` filter in `FULL_DUPLEX_STREAMED` mode, one body chunk was lost end-to-end. The defect is in core `commonHandleAfterDataCallback()` and is not wasm- or ext_proc-specific. The `Continue` path now forwards the filter-manager buffer instead of the empty frame when the filter drained the frame while already iterating. The behavior is guarded by the runtime feature `envoy.reloadable_features.filter_manager_forward_added_data_on_continue` (default-on) so it can be reverted if needed. Adds a `FilterManager` unit test covering both guard states and an `ext_proc` `FULL_DUPLEX_STREAMED` integration test (with a new `add-data-and-continue-filter` test filter) asserting byte-exact body delivery. Backport notes (changes needed to resolve conflicts against 1.34): - The source fix (filter_manager.cc/.h), the FilterManager unit test, and the runtime guard applied cleanly. runtime_features.cc only added the `filter_manager_forward_added_data_on_continue` guard. - 1.34 has no `test/integration/filters/test_filters.proto` (nor the `test_filters_proto_cc_proto` target); it was introduced after 1.34. The cherry-pick's modification of that proto (adding an `AddDataAndContinueFilterConfig` message) was dropped and the proto file kept absent. - The new `add-data-and-continue-filter` test filter was adapted to use the proto-less `EmptyHttpFilterConfig` base (backed by `ProtobufWkt::Struct`, as `UniqueEmptyHttpFilterConfig` does not exist on 1.34), its `test_filters.pb.h` include and the `test_filters_proto_cc_proto` BUILD dep were removed, and the ext_proc integration test prepends the filter by name only (matching how 1.34 wires other `EmptyHttpFilterConfig` filters such as `backpressure-filter`). - The ported `AddDataAndContinueBeforeExtProcDuplexStreamed` test reuses the duplex-streamed helpers already present on 1.34. Closes: #46841 Signed-off-by: Jonh Wendell <jwendell@redhat.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport to release/v1.35. This is a cherry-pick of the release/v1.36
commit 0d9b3a8, which in turn
cherry-picked the main-branch commit 0871c75a6f329465c28cc6280cc904dc746d151b.
When an HTTP filter that is already iterating moves the current body
frame into the filter-manager buffer via
addDecodedData()/addEncodedData()and then returnsContinue, thebuffered frame was silently dropped: the now-empty frame was forwarded
down the chain instead of the just-buffered bytes.
This corrupts streamed request/response bodies. It was observed by the
Kuadrant wasm-shim project, where a wasm filter using
allow_on_headers_stop_iterationstops on headers, resumesasynchronously, and then buffers a body chunk before continuing; chained
ahead of an
ext_procfilter inFULL_DUPLEX_STREAMEDmode, one bodychunk was lost end-to-end. The defect is in core
commonHandleAfterDataCallback()and is not wasm- or ext_proc-specific.The
Continuepath now forwards the filter-manager buffer instead ofthe empty frame when the filter drained the frame while already
iterating. The behavior is guarded by the runtime feature
envoy.reloadable_features.filter_manager_forward_added_data_on_continue(default-on) so it can be reverted if needed.
Adds a
FilterManagerunit test covering both guard states and anext_procFULL_DUPLEX_STREAMEDintegration test (with a newadd-data-and-continue-filtertest filter) asserting byte-exact bodydelivery.
Backport notes (changes needed to resolve conflicts against 1.35):
the new
add-data-and-continue-filtertest filter and its BUILDwiring all applied cleanly.
source/common/runtime/runtime_features.cc: only added thefilter_manager_forward_added_data_on_continueguard; the unrelatedgeneric_proxy_codec_buffer_limitandext_proc_inject_data_with_state_updateguards in the surrounding1.36 context do not exist on 1.35 and were dropped.
ext_proc_full_duplex_integration_test.cc. That file was dropped andits single new test
AddDataAndContinueBeforeExtProcDuplexStreamedwas ported into 1.35's monolithic
ext_proc_integration_test.cc,adapting it to the duplex-streamed helper names/signatures present on
1.35 (
serverReceiveHeaderReq,serverSendHeaderResp, and theserverReceiveBodyDuplexStreamed/serverSendBodyRespDuplexStreamedhelpers that take the processor stream as an argument).
Closes: #46841