Ext-Proc: Data truncation with two ext_proc filters configured and body modes are STREAMED
- Dominant language
- C++
- Stars
- 28.9k
- Forks
- 5.6k
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 437
Description
*Title*: Data truncation with two ext_proc filters configured and body modes are STREAMED
*Description*:
If there are two ext_proc filters configured, and the body processing modes of both of the filters are STREAMED, there is a data truncation issue happens if:
1) Client request contains trailers.
2) Client body external processing is going on in the 1st ext_proc filter.
3) Client trailers arrive Envoy.
4) Header response is received by the Envoy 2nd ext_proc filter.
The root cause is that the 2nd ext_proc filter header response processing call continueIfNecessary(), which evetually call filter_manager commonContinue(). As trailers is already received, it will call doTrailers(): https://github.com/envoyproxy/envoy/blob/26a8314c4e787bb606c30a51976679c816a8ca30/source/common/http/filter_manager.cc#L142. This will terminate the decoding process early while the data is still being processed by the 1st filter. Then when the 1st ext_proc filter finished body processing, re-inject the data to filter chain will cause ASSERT() failure in debug build. In prod build, the data will be lost.
Such issue happens for FULL_DUPLEX_STREAMED mode as well.
The root cause is calling commonContinue() during Envoy ext_proc header response processing incorrectly intertwines the filter manager states with ext_proc filter states. This should be avoided during the header response processing.
The original fix is proposed by: https://github.com/envoyproxy/envoy/pull/45355. However, that's causing an issue for websocket: https://github.com/envoyproxy/envoy/pull/47060. So, https://github.com/envoyproxy/envoy/pull/45355 is reverted for STREAMED mode. However, the data truncation issue resurface after that revert.
The new fix is proposed: https://github.com/envoyproxy/envoy/pull/47353. This PR basically keeps the original behavior, i.e, send the headers down the filter chain once the header response arrives Envoy. It adds new injectDecodeHeadersToFilterChain()/injectEncodeHeadersToFilterChain APIs to achieve this. As this approach is sending the headers to the filter chain without call the filter manager: commonContinue(), thus the filter_manager state machine won't cause problem with it.
Contributor guide
Assessment
This issue has not been assessed yet.