envoyproxy / envoyproxy/envoy

Consistent request header semantics for core code

Open
#6,449 0 comments 0 reactions 0 assignees View on GitHub
help wanted tech debt
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
428

Description

Currently there are a few headers that are assumed to be present in incoming requests.
Things like `:host`, `:method`, `:scheme` (assumed to be present) and `:path` (assumed to have non zero length), but also more subtle things like `x-forwarded-proto` (assumed to be present).
If not present, the `x-forwarded-proto` is added during the incoming request processing, by the `ConnectionManagerImpl::ActiveStream::decodeHeaders` method.

(I found these headers by running this regex search `ASSERT\(.*headers.*(\.|->)` using ripgrep)

One example of this invariant breaking is in #6417, where a request is rejected (that happens before the headers are added). This invoke encoder filters without the `x-forwarded-proto` headers, which will cause a crash if the encoder filters calls `encoder_callbacks_->route()`.

We should decide on a consistent approach on how to solve this. The following options were discussed in #6417
1. Decide and document which headers are guaranteed to be present in the request headers, and make sure they are added very early in the request processing stage, so that they are always available (even when the request is rejected at an early stage).
2. Decide that there are no guarantees and make sure the code always does null checks before accessing request headers.

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.