Skip to content

Commit

Permalink
http: support passing match result action to filter (#14462)
Browse files Browse the repository at this point in the history 
Adds support for passing through a match action from a match tree to the associated HTTP filter.

Some care has to be taken here around dual filters, so we introduce an abstraction that moves handling HttpMatchingData
updates and applying the match result into a FilterMatchState object that is shared between all filter wrappers for a given filter.

This should also avoid having to match twice for dual filters: the match result is shared for both filters, instead of both of
them having to independently arrive at it with the same data.

Signed-off-by: Snow Pettersen <snowp@lyft.com>
  • Loading branch information
@snowp
snowp authored Jan 13, 2021
1 parent a4bf246 commit ad7d36f
Show file tree
Hide file tree
Showing 5 changed files with 221 additions and 67 deletions.
6 changes: 6 additions & 0 deletions include/envoy/http/filter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,12 @@ class StreamFilterBase {
* onDestroy().
*/
virtual void onDestroy() PURE;

/**
* Called when a match result occurs that isn't handled by the filter manager.
* @param action the resulting match action
*/
virtual void onMatchCallback(const Matcher::Action&) {}
};

/**
Expand Down
67 changes: 43 additions & 24 deletions source/common/http/filter_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ void ActiveStreamFilterBase::clearRouteCache() {
parent_.filter_manager_callbacks_.clearRouteCache();
}

void ActiveStreamFilterBase::evaluateMatchTreeWithNewData(
void FilterMatchState::evaluateMatchTreeWithNewData(
std::function<void(HttpMatchingDataImpl&)> update_func) {
if (match_tree_evaluated_ || !matching_data_) {
return;
Expand All @@ -258,7 +258,9 @@ void ActiveStreamFilterBase::evaluateMatchTreeWithNewData(

if (match_tree_evaluated_ && match_result.result_) {
if (SkipAction().typeUrl() == match_result.result_->typeUrl()) {
skip_ = true;
skip_filter_ = true;
} else {
filter_->onMatchCallback(*match_result.result_);
}
}
}
Expand Down Expand Up @@ -418,11 +420,18 @@ void ActiveStreamDecoderFilter::requestDataTooLarge() {
}
}

void FilterManager::addStreamDecoderFilterWorker(
StreamDecoderFilterSharedPtr filter, Matcher::MatchTreeSharedPtr<HttpMatchingData> matcher,
HttpMatchingDataImplSharedPtr matching_data, bool dual_filter) {
ActiveStreamDecoderFilterPtr wrapper(new ActiveStreamDecoderFilter(
*this, filter, std::move(matcher), std::move(matching_data), dual_filter));
void FilterManager::addStreamDecoderFilterWorker(StreamDecoderFilterSharedPtr filter,
FilterMatchStateSharedPtr match_state,
bool dual_filter) {
ActiveStreamDecoderFilterPtr wrapper(
new ActiveStreamDecoderFilter(*this, filter, match_state, dual_filter));

// If we're a dual handling filter, have the encoding wrapper be the only thing registering itself
// as the handling filter.
if (match_state) {
match_state->filter_ = filter.get();
}

filter->setDecoderFilterCallbacks(*wrapper);
// Note: configured decoder filters are appended to decoder_filters_.
// This means that if filters are configured in the following order (assume all three filters are
Expand All @@ -435,11 +444,16 @@ void FilterManager::addStreamDecoderFilterWorker(
LinkedList::moveIntoListBack(std::move(wrapper), decoder_filters_);
}

void FilterManager::addStreamEncoderFilterWorker(
StreamEncoderFilterSharedPtr filter, Matcher::MatchTreeSharedPtr<HttpMatchingData> match_tree,
HttpMatchingDataImplSharedPtr matching_data, bool dual_filter) {
ActiveStreamEncoderFilterPtr wrapper(new ActiveStreamEncoderFilter(
*this, filter, std::move(match_tree), std::move(matching_data), dual_filter));
void FilterManager::addStreamEncoderFilterWorker(StreamEncoderFilterSharedPtr filter,
FilterMatchStateSharedPtr match_state,
bool dual_filter) {
ActiveStreamEncoderFilterPtr wrapper(
new ActiveStreamEncoderFilter(*this, filter, match_state, dual_filter));

if (match_state) {
match_state->filter_ = filter.get();
}

filter->setEncoderFilterCallbacks(*wrapper);
// Note: configured encoder filters are prepended to encoder_filters_.
// This means that if filters are configured in the following order (assume all three filters are
Expand Down Expand Up @@ -477,9 +491,12 @@ void FilterManager::decodeHeaders(ActiveStreamDecoderFilter* filter, RequestHead
std::list<ActiveStreamDecoderFilterPtr>::iterator continue_data_entry = decoder_filters_.end();

for (; entry != decoder_filters_.end(); entry++) {
(*entry)->evaluateMatchTreeWithNewData(
[&](auto& matching_data) { matching_data.onRequestHeaders(headers); });
if ((*entry)->skip_) {
if ((*entry)->filter_match_state_) {
(*entry)->filter_match_state_->evaluateMatchTreeWithNewData(
[&](auto& matching_data) { matching_data.onRequestHeaders(headers); });
}

if ((*entry)->skipFilter()) {
continue;
}

Expand Down Expand Up @@ -563,7 +580,7 @@ void FilterManager::decodeData(ActiveStreamDecoderFilter* filter, Buffer::Instan
commonDecodePrefix(filter, filter_iteration_start_state);

for (; entry != decoder_filters_.end(); entry++) {
if ((*entry)->skip_) {
if ((*entry)->skipFilter()) {
continue;
}
// If the filter pointed by entry has stopped for all frame types, return now.
Expand Down Expand Up @@ -697,7 +714,7 @@ void FilterManager::decodeTrailers(ActiveStreamDecoderFilter* filter, RequestTra
commonDecodePrefix(filter, FilterIterationStartState::CanStartFromCurrent);

for (; entry != decoder_filters_.end(); entry++) {
if ((*entry)->skip_) {
if ((*entry)->skipFilter()) {
continue;
}

Expand Down Expand Up @@ -729,7 +746,7 @@ void FilterManager::decodeMetadata(ActiveStreamDecoderFilter* filter, MetadataMa
commonDecodePrefix(filter, FilterIterationStartState::CanStartFromCurrent);

for (; entry != decoder_filters_.end(); entry++) {
if ((*entry)->skip_) {
if ((*entry)->skipFilter()) {
continue;
}
// If the filter pointed by entry has stopped for all frame type, stores metadata and returns.
Expand Down Expand Up @@ -939,7 +956,7 @@ void FilterManager::encode100ContinueHeaders(ActiveStreamEncoderFilter* filter,
std::list<ActiveStreamEncoderFilterPtr>::iterator entry =
commonEncodePrefix(filter, false, FilterIterationStartState::AlwaysStartFromNext);
for (; entry != encoder_filters_.end(); entry++) {
if ((*entry)->skip_) {
if ((*entry)->skipFilter()) {
continue;
}

Expand Down Expand Up @@ -984,9 +1001,11 @@ void FilterManager::encodeHeaders(ActiveStreamEncoderFilter* filter, ResponseHea
std::list<ActiveStreamEncoderFilterPtr>::iterator continue_data_entry = encoder_filters_.end();

for (; entry != encoder_filters_.end(); entry++) {
(*entry)->evaluateMatchTreeWithNewData(
[&headers](auto& matching_data) { matching_data.onResponseHeaders(headers); });
if ((*entry)->skip_) {
if ((*entry)->filter_match_state_) {
(*entry)->filter_match_state_->evaluateMatchTreeWithNewData(
[&headers](auto& matching_data) { matching_data.onResponseHeaders(headers); });
}
if ((*entry)->skipFilter()) {
continue;
}
ASSERT(!(state_.filter_call_state_ & FilterCallState::EncodeHeaders));
Expand Down Expand Up @@ -1043,7 +1062,7 @@ void FilterManager::encodeMetadata(ActiveStreamEncoderFilter* filter,
commonEncodePrefix(filter, false, FilterIterationStartState::CanStartFromCurrent);

for (; entry != encoder_filters_.end(); entry++) {
if ((*entry)->skip_) {
if ((*entry)->skipFilter()) {
continue;
}
// If the filter pointed by entry has stopped for all frame type, stores metadata and returns.
Expand Down Expand Up @@ -1114,7 +1133,7 @@ void FilterManager::encodeData(ActiveStreamEncoderFilter* filter, Buffer::Instan

const bool trailers_exists_at_start = filter_manager_callbacks_.responseTrailers().has_value();
for (; entry != encoder_filters_.end(); entry++) {
if ((*entry)->skip_) {
if ((*entry)->skipFilter()) {
continue;
}
// If the filter pointed by entry has stopped for all frame type, return now.
Expand Down
Loading

0 comments on commit ad7d36f

Please sign in to comment.