Skip to content

http: fix filter manager dropping a body frame on continue (#46842) - #619

Merged
jwendell merged 1 commit into
envoyproxy:release/v1.35from
jwendell:backport-filter-manager-body-frame-1.35
Sep 16, 2026
Merged

jwendell merged 1 commit into
envoyproxy:release/v1.35from
jwendell:backport-filter-manager-body-frame-1.35

Conversation

@jwendell

@jwendell jwendell commented Sep 11, 2026 •

Copy link
Copy Markdown
Member

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

@jwendell

Copy link
Copy Markdown
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
jwendell force-pushed the backport-filter-manager-body-frame-1.35 branch from b40c7a3 to 2cf6b96 Compare September 16, 2026 12:30
@jwendell
jwendell marked this pull request as ready for review September 16, 2026 12:30
@dcillera
dcillera self-requested a review September 16, 2026 12:39
@dcillera

Copy link
Copy Markdown

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:

  • Both call sites of the changed commonHandleAfterDataCallback signature are correctly updated.
  • The runtime guard (filter_manager_forward_added_data_on_continue) is properly registered and named.
  • The ext_proc integration-test helper signatures match the 1.35 definitions.
  • The changelog directory format is correct for this branch.

The related test:

//test/extensions/filters/http/ext_proc:ext_proc_integration_test'
//test/common/http:filter_manager_test

have been successfully executed locally.

@jwendell
jwendell merged commit 6101f0c into envoyproxy:release/v1.35 Sep 16, 2026
1 check passed
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>
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.

2 participants