Skip to content

Commit 1d81dd1

Browse files
fix(ext_proc): fix downstream hang with chunked empty body on mode override to FULL_DUPLEX_STREAMED
In sendBufferedDataInStreamedMode, hasBufferedData() checks length > 0, skipping 0-length buffers. When mode_override changes response_body_mode from BUFFERED to FULL_DUPLEX_STREAMED and the upstream sends a chunked response with an empty body, the 0-byte buffer is never sent to ext_proc, causing the filter chain to hang. Change hasBufferedData() to bufferedData() (non-null check) so the empty body chunk with end_stream=true is properly dispatched to the ext_proc server, matching the Idle path in handleCompleteBodyAvailable. Signed-off-by: Renuka Fernando <renukapiyumal@gmail.com>
1 parent 1679e84 commit 1d81dd1

2 files changed

Lines changed: 66 additions & 1 deletion

File tree

source/extensions/filters/http/ext_proc/processor_state.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ bool ProcessorState::restartMessageTimer(const uint32_t message_timeout_ms) {
112112

113113
// Process the data being buffered in STREAMED or FULL_DUPLEX_STREAMED mode.
114114
void ProcessorState::sendBufferedDataInStreamedMode(bool end_stream) {
115-
if (hasBufferedData()) {
115+
if (bufferedData()) {
116116
Buffer::OwnedImpl buffered_chunk;
117117
modifyBufferedData([&buffered_chunk](Buffer::Instance& data) { buffered_chunk.move(data); });
118118
ENVOY_STREAM_LOG(debug, "Sending a chunk of buffered data ({})", *filter_callbacks_,

test/extensions/filters/http/ext_proc/filter_test.cc

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6058,6 +6058,71 @@ TEST_F(HttpFilterTest, HttpEventTrafficStatsTest) {
60586058
filter_->onDestroy();
60596059
}
60606060

6061+
// Test that mode_override from BUFFERED to FULL_DUPLEX_STREAMED during response header
6062+
// processing does not hang when the upstream sends a Transfer-Encoding: chunked response
6063+
// with an empty body. With chunked encoding, encodeHeaders is called with end_stream=false,
6064+
// and the terminal chunk (0\r\n\r\n) triggers encodeData with an empty buffer and
6065+
// end_stream=true before the ext_proc server responds to the header request.
6066+
TEST_F(HttpFilterTest, ModeOverrideBufferedToFullDuplexChunkedEmptyBody) {
6067+
initialize(R"EOF(
6068+
grpc_service:
6069+
envoy_grpc:
6070+
cluster_name: "ext_proc_server"
6071+
allow_mode_override: true
6072+
processing_mode:
6073+
request_header_mode: "SEND"
6074+
response_header_mode: "SEND"
6075+
response_body_mode: "BUFFERED"
6076+
)EOF");
6077+
6078+
EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, false));
6079+
processRequestHeaders(false, absl::nullopt);
6080+
6081+
Buffer::OwnedImpl req_data("foo");
6082+
EXPECT_EQ(FilterDataStatus::Continue, filter_->decodeData(req_data, true));
6083+
6084+
// Response headers arrive with end_stream=false (Transfer-Encoding: chunked).
6085+
// Set up encoding buffer: the filter manager will hold a 0-length buffer after
6086+
// encodeData returns StopIterationAndBuffer for the empty body.
6087+
Buffer::OwnedImpl encoding_buf;
6088+
EXPECT_CALL(encoder_callbacks_, encodingBuffer()).WillRepeatedly(Return(&encoding_buf));
6089+
EXPECT_CALL(encoder_callbacks_, modifyEncodingBuffer(_))
6090+
.WillRepeatedly(
6091+
Invoke([&encoding_buf](std::function<void(Buffer::Instance&)> cb) { cb(encoding_buf); }));
6092+
6093+
response_headers_.addCopy(LowerCaseString(":status"), "200");
6094+
response_headers_.addCopy(LowerCaseString("content-type"), "text/plain");
6095+
EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->encodeHeaders(response_headers_, false));
6096+
6097+
// Empty response body arrives BEFORE ext proc responds to headers. This simulates
6098+
// chunked encoding with empty body: the terminal chunk arrives quickly after headers.
6099+
// Body mode is still BUFFERED at this point, so the filter returns StopIterationAndBuffer.
6100+
Buffer::OwnedImpl empty_resp_data;
6101+
EXPECT_EQ(FilterDataStatus::StopIterationAndBuffer, filter_->encodeData(empty_resp_data, true));
6102+
6103+
// Process response headers with mode_override changing response_body_mode to
6104+
// FULL_DUPLEX_STREAMED.
6105+
EXPECT_CALL(encoder_callbacks_, injectEncodedDataToFilterChain(_, _)).Times(AnyNumber());
6106+
processResponseHeaders(false,
6107+
[](const HttpHeaders&, ProcessingResponse& response, HeadersResponse&) {
6108+
auto mode = response.mutable_mode_override();
6109+
mode->set_response_body_mode(ProcessingMode::FULL_DUPLEX_STREAMED);
6110+
});
6111+
6112+
// Process response body: ext_proc receives the 0-byte body with end_stream=true and
6113+
// responds with a FULL_DUPLEX_STREAMED streamed_response.
6114+
Buffer::OwnedImpl want_response_body;
6115+
processResponseBodyHelper("", want_response_body, true, true);
6116+
6117+
filter_->onDestroy();
6118+
6119+
EXPECT_EQ(1, config_->stats().streams_started_.value());
6120+
// 3 messages sent: request_headers, response_headers, response_body.
6121+
EXPECT_EQ(3, config_->stats().stream_msgs_sent_.value());
6122+
EXPECT_EQ(3, config_->stats().stream_msgs_received_.value());
6123+
EXPECT_EQ(1, config_->stats().streams_closed_.value());
6124+
}
6125+
60616126
} // namespace
60626127
} // namespace ExternalProcessing
60636128
} // namespace HttpFilters

0 commit comments

Comments
 (0)