dynamic module body ABI problems #40918
Description
Title: dynamic module body ABI
Description:
Describe the desired behavior, what scenario it enables and how it
would be used.
In the dynamic modules ABI, the get body method is implemented as follows:
bool envoy_dynamic_module_callback_http_get_request_body_vector(
envoy_dynamic_module_type_http_filter_envoy_ptr filter_envoy_ptr,
envoy_dynamic_module_type_envoy_buffer* result_buffer_vector) {
auto filter = static_cast<DynamicModuleHttpFilter*>(filter_envoy_ptr);
auto buffer = filter->decoder_callbacks_->decodingBuffer();
if (!buffer) {
buffer = filter->current_request_body_;
if (!buffer) {
return false;
}
// See the comment on current_request_body_ for when we reach this.
}
auto raw_slices = buffer->getRawSlices(std::nullopt);
auto counter = 0;
for (const auto& slice : raw_slices) {
result_buffer_vector[counter].length = slice.len_;
result_buffer_vector[counter].ptr = static_cast<char*>(slice.mem_);
counter++;
}
return true;
}
bool envoy_dynamic_module_callback_http_append_request_body(
envoy_dynamic_module_type_http_filter_envoy_ptr filter_envoy_ptr,
envoy_dynamic_module_type_buffer_module_ptr data, size_t length) {
auto filter = static_cast<DynamicModuleHttpFilter*>(filter_envoy_ptr);
if (!filter->decoder_callbacks_->decodingBuffer()) {
if (filter->current_request_body_) { // See the comment on current_request_body_ for when we
// enter this block.
filter->current_request_body_->add(absl::string_view(static_cast<const char*>(data), length));
return true;
}
return false;
}
filter->decoder_callbacks_->modifyDecodingBuffer([data, length](Buffer::Instance& buffer) {
buffer.add(absl::string_view(static_cast<const char*>(data), length));
});
return true;
}
bool envoy_dynamic_module_callback_http_drain_request_body(
envoy_dynamic_module_type_http_filter_envoy_ptr filter_envoy_ptr, size_t number_of_bytes) {
auto filter = static_cast<DynamicModuleHttpFilter*>(filter_envoy_ptr);
if (!filter->decoder_callbacks_->decodingBuffer()) {
if (filter->current_request_body_) { // See the comment on current_request_body_ for when we
// enter this block.
auto size = std::min<uint64_t>(filter->current_request_body_->length(), number_of_bytes);
filter->current_request_body_->drain(size);
return true;
}
return false;
}
filter->decoder_callbacks_->modifyDecodingBuffer([number_of_bytes](Buffer::Instance& buffer) {
auto size = std::min<uint64_t>(buffer.length(), number_of_bytes);
buffer.drain(size);
});
return true;
}
Basically, the accumulated buffer (decodingBuffer()) will be used as priority. It's fine if the plugin is read only. But if the plugin need the modify the buffer, then it will result in some problems:
- Unfriendly to stream operations, for example, SSE. If we want to modify the body based on every event, it almost impossible because we don't know which data piece is processed and which is not. You cannot assume all processed will be sent out, because other filters may buffer it. You cannot record the offset of processed data because part of data may be sent.
- Incorrect body content: when the first body event (chunk 1) is coming, because there is no buffered data, then the
get_request_bodywill return the new received body (chunk 1). And if the filter return theStopIterationAndBuffer, then when the second body event (chunk 2) is coming, because previous body content is buffered, then theget_request_bodywill return the chunk 1 again. - Break some basic assumptions. Modification to body for now would be a problem because the
modifyDecodingBufferormodifyEncodingBufferis necessary to complete the modification in current design. ButmodifyDecodingBuffershould only be used by thelatest data decoding filter, or it may break the whole assumption of the filter chain (For example, a filter after the dynamic modules have see body chunk X, then, the X may be drained by the dynamic modules)
[optional Relevant Links:]
Any extra documentation required to understand the issue.