Skip to content

[Feature Request] Allow HTTP filters to add a body that's not readily available to headers-only responses  #12814

@yosrym93

Description

@yosrym93

TL;DR: For headers-only responses, we need a way to tell the FilterManager in encodeHeaders that we want to add body to this response without having it readily available (i.e don't end stream after sending headers downstream).

I am working on the HTTP CacheFilter. When a cached response is being revalidated, a 304 (not modified) response (headers only) means that the cached response is valid. In this scenario, the CacheFilter (currently) updates the headers in encodeHeaders, stops iteration, and requests the cached response from the cache. The cache fetches the cached response along several chunks of data. Whenever a chunk is ready, the CacheFilter calls StreamEncoderFilterCallbacks::addEncodedData to add it to the body. After the whole cached response is added, the CacheFilter calls StreamEncoderFilterCallbacks::continueEncoding to resume encoding with the full response.

This approach has several problems:

  • The headers and body are held until the response fully available, which incurs a delay.
  • The memory usage for buffering the whole response may be high, and it may cause a buffer overflow.

Ideally we would stream headers and body chunks downstream whenever they are available. We could use injectEncodedDataToFilterChain instead of addEncodedData to directly pass the data chunks as they arrive. However, the problem is (AFAIK) there's no current way in the filters api to "tell" the HCM (or the FilterManager) in encodeHeaders that there's body that will be added to this response without having it ready. In other words, change end_stream to false.

One way of changing a headers-only response to a response w/ body without making much changes to the HCM or the API is to call addEncodedData in CacheFilter::encodeHeaders with an empty body. This will cause data iteration for the response to kick-start after the headers iteration. However, the way addEncodedData works now is that if a filter calls it during the encodeHeaders callback, and it continues iteration, then the data iteration starts at the next filter downstream. This means the CacheFilter has no way of stopping the data iteration until the cached response is fetched.

If acceptable, the default behavior of addEncodedData in this case (being called in encodeHeaders) can be changed by a small edit to FilterManager::encodeHeaders. Specifically changing this if condition:

// Here we handle the case where we have a header only response, but a filter adds a body
// to it. We need to not raise end_stream = true to further filters during inline iteration.
if (end_stream && buffered_response_data_ && continue_data_entry == encoder_filters_.end()) {
    continue_data_entry = entry;
}

to:

// Here we handle the case where we have a header only response, but a filter adds a body
// to it. We need to not raise end_stream = true to further filters during inline iteration.
if (end_stream && buffered_response_data_ && continue_data_entry == encoder_filters_.end()) {
    continue_data_entry = entry;
    (*entry)->end_stream_ = false;
    (*entry)->iterate_from_current_filter_ = true;
}

Currently, only a single HTTP filter (json_transcoder) calls addEncodedData during encodeHeaders. If this default behavior is acceptable we can implement it and fix this filter (maybe by skipping encodeData if it was invoked by the filter itself).

Another way to do this is to change the addEncodedData api and add an extra parameter as whether the filter wants to participate in encoding the data it added. However, (AFAIK) this parameter would only be relevant when the function is called in encodeHeaders callbacks, which is not the common case.

Any other suggestions on ways to accomplish this are highly appreciated!

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions