proxy-wasm
HTTP: support adding a body to a request or response that did not previously have one
(This issue is semi-related to #64 "HTTP: add control of the end_of_stream flag", arguably they could be coalesced)
In its section on Buffers, the v0.2.1 ABI spec says:
Access to buffers (listed in proxy_buffer_type_t) using functions in this section is restricted to specific callbacks:
- HTTP_REQUEST_BODY can be read and modified in proxy_on_request_body (or for as long as request processing is paused from it).
- HTTP_RESPONSE_BODY can be read and modified in proxy_on_response_body (or for as long as response processing is paused from it).
However, in the case of an HTTP request that does not contain a body, or (more rarely) an HTTP response without a body, a plugin will receive a proxy_on_request_headers or proxy_on_response_headers callback with end_of_stream=true, and won't see a proxy_on_*_body callback. This means that if the plugin wants to add a body to the request/response being proxied, it has no way of doing so, since it would need to access the HTTP_REQUEST_BODY or HTTP_RESPONSE_BODY buffer, which is not available.
Some possible solutions:
-
A. Allow proxy_on_request_headers to access the HTTP_REQUEST_BODY buffer in the case where end_of_stream=true.
-
B. Specify that the host should call proxy_on_request_headers(..., end_of_stream=false) followed by proxy_on_request_body(body_size=0, end_of_stream=true).
-
C. Add hostcalls corresponding to Envoy's StreamDecoderFilterCallbacks::addDecodedData and StreamEncoderFilterCallbacks::addEncodedData methods.
Among these, (A) seems appealing in that it is a minor change that might even be possible to introduce in the current ABI without breaking plugins. (B) would be a more visible behavior change, and (C) seems less uniform (and more Envoy-specific) WRT the existing setBuffer API.
- A. Allow proxy_on_request_headers to access the HTTP_REQUEST_BODY buffer in the case where end_of_stream=true.
This is the way to do it, since this is how we handle the same use case for trailers (see: https://github.com/envoyproxy/envoy/pull/15831).
And yes, this wouldn't be a breaking change, so it's safe to do it and "fix" the text in the ABI docs.
- B. Specify that the host should call proxy_on_request_headers(..., end_of_stream=false) followed by proxy_on_request_body(body_size=0, end_of_stream=true).
This would definitely be a breaking change, and it wouldn't be great from the micro-optimization point-of-view (i.e. increases the number of callbacks, which is relatively costly in case of inline Wasm plugins).
Note that we'd also need to add corresponding proxy_on_{request,response}_trailers(end_of_stream=true) in that case to make the ABI consistent.
- C. Add hostcalls corresponding to Envoy's StreamDecoderFilterCallbacks::addDecodedData and StreamEncoderFilterCallbacks::addEncodedData methods.
Isn't this effectively the same as A (i.e. you need to use those Envoy functions to implement A?) or do you mean adding new hostcalls to the ABI? If you're suggesting the latter, then it doesn't really make sense, since we already have the hostcalls to set that request/response body buffers.
