envoyproxy / envoyproxy/envoy

wasm: Context::sendLocalResponse() can return Ok without doing anything

Open
#39,797 7 comments 0 reactions 0 assignees View on GitHub
area/wasm bug no stalebot
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 22h
Merged PRs (30d)
430

Description

The current implementation of Envoy::Extensions::Common::Wasm::Context::sendLocalResponse does nothing in the case where `decoder_callbacks_` is null [here](https://github.com/envoyproxy/envoy/blob/4189535918f7e96106d8c196b885dc8d54a57dbc/source/extensions/common/wasm/context.cc#L1622).

In practice, we've found that error case is easy to hit if the call to `sendLocalResponse()` is performed in a callback where the effective context is not a stream context. A concrete example is a gRPC callout, where response callbacks do not automatically set the effective context to the stream context present when the call was initiated. Although one can address this failure by having plugin code call `setEffectiveContext()` prior to `sendLocalResponse()`, it's not at all clear to the plugin that this is needed since without it, `sendLocalResponse()` silently fails and returns Ok.

Proposed fix is to add an `else` clause [here](https://github.com/envoyproxy/envoy/blob/4189535918f7e96106d8c196b885dc8d54a57dbc/source/extensions/common/wasm/context.cc#L1642) that returns `NotFound`, `InternalFailure`, or some new enum value of `WasmResult` in this case.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.