Skip to content

Commit

Permalink
wasm: clear route cache when modifying HTTP request headers. (#14318)
Browse files Browse the repository at this point in the history
Proxy-Wasm ABI v0.1.0 has a dedicated call for clearing route cache
(proxy_clear_route_cache).

Proxy-Wasm ABI v0.2.0 removed it in favor of automatically clearing
route cache, but that part was never implemented in Envoy.

Partially fixes proxy-wasm/spec#16.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
  • Loading branch information
PiotrSikora authored Dec 15, 2020
1 parent 5dc58a6 commit ca7250d
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 0 deletions.
12 changes: 12 additions & 0 deletions source/extensions/common/wasm/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,9 @@ WasmResult Context::addHeaderMapValue(WasmHeaderMapType type, absl::string_view
}
const Http::LowerCaseString lower_key{std::string(key)};
map->addCopy(lower_key, std::string(value));
if (type == WasmHeaderMapType::RequestHeaders) {
decoder_callbacks_->clearRouteCache();
}
return WasmResult::Ok;
}

Expand Down Expand Up @@ -769,6 +772,9 @@ WasmResult Context::setHeaderMapPairs(WasmHeaderMapType type, const Pairs& pairs
const Http::LowerCaseString lower_key{std::string(p.first)};
map->addCopy(lower_key, std::string(p.second));
}
if (type == WasmHeaderMapType::RequestHeaders) {
decoder_callbacks_->clearRouteCache();
}
return WasmResult::Ok;
}

Expand All @@ -779,6 +785,9 @@ WasmResult Context::removeHeaderMapValue(WasmHeaderMapType type, absl::string_vi
}
const Http::LowerCaseString lower_key{std::string(key)};
map->remove(lower_key);
if (type == WasmHeaderMapType::RequestHeaders) {
decoder_callbacks_->clearRouteCache();
}
return WasmResult::Ok;
}

Expand All @@ -790,6 +799,9 @@ WasmResult Context::replaceHeaderMapValue(WasmHeaderMapType type, absl::string_v
}
const Http::LowerCaseString lower_key{std::string(key)};
map->setCopy(lower_key, value);
if (type == WasmHeaderMapType::RequestHeaders) {
decoder_callbacks_->clearRouteCache();
}
return WasmResult::Ok;
}

Expand Down
5 changes: 5 additions & 0 deletions test/extensions/filters/http/wasm/test_data/headers_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ impl HttpContext for TestStream {
Action::Continue
}

fn on_http_response_headers(&mut self, _: usize) -> Action {
self.set_http_response_header("test-status", Some("OK"));
Action::Continue
}

fn on_http_response_trailers(&mut self, _: usize) -> Action {
Action::Pause
}
Expand Down
10 changes: 10 additions & 0 deletions test/extensions/filters/http/wasm/test_data/test_cpp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class TestContext : public Context {

FilterHeadersStatus onRequestHeaders(uint32_t, bool) override;
FilterTrailersStatus onRequestTrailers(uint32_t) override;
FilterHeadersStatus onResponseHeaders(uint32_t, bool) override;
FilterTrailersStatus onResponseTrailers(uint32_t) override;
FilterDataStatus onRequestBody(size_t body_buffer_length, bool end_of_stream) override;
void onLog() override;
Expand Down Expand Up @@ -277,6 +278,15 @@ FilterTrailersStatus TestContext::onRequestTrailers(uint32_t) {
return FilterTrailersStatus::Continue;
}

FilterHeadersStatus TestContext::onResponseHeaders(uint32_t, bool) {
root()->stream_context_id_ = id();
auto test = root()->test_;
if (test == "headers") {
CHECK_RESULT(addResponseHeader("test-status", "OK"));
}
return FilterHeadersStatus::Continue;
}

FilterTrailersStatus TestContext::onResponseTrailers(uint32_t) {
auto value = getResponseTrailer("bogus-trailer");
if (value && value->view() != "") {
Expand Down
17 changes: 17 additions & 0 deletions test/extensions/filters/http/wasm/wasm_filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ TEST_P(WasmHttpFilterTest, HeadersOnlyRequestHeadersOnly) {
log_(spdlog::level::debug, Eq(absl::string_view("onRequestHeaders 2 headers"))));
EXPECT_CALL(filter(), log_(spdlog::level::info, Eq(absl::string_view("header path /"))));
EXPECT_CALL(filter(), log_(spdlog::level::warn, Eq(absl::string_view("onDone 2"))));

// Verify that route cache is cleared when modifying HTTP request headers.
Http::MockStreamDecoderFilterCallbacks decoder_callbacks;
filter().setDecoderFilterCallbacks(decoder_callbacks);
EXPECT_CALL(decoder_callbacks, clearRouteCache()).Times(2);

Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}, {"server", "envoy"}};
EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter().decodeHeaders(request_headers, true));
EXPECT_THAT(request_headers.get_("newheader"), Eq("newheadervalue"));
Expand All @@ -137,6 +143,12 @@ TEST_P(WasmHttpFilterTest, AllHeadersAndTrailers) {
log_(spdlog::level::debug, Eq(absl::string_view("onRequestHeaders 2 headers"))));
EXPECT_CALL(filter(), log_(spdlog::level::info, Eq(absl::string_view("header path /"))));
EXPECT_CALL(filter(), log_(spdlog::level::warn, Eq(absl::string_view("onDone 2"))));

// Verify that route cache is cleared when modifying HTTP request headers.
Http::MockStreamDecoderFilterCallbacks decoder_callbacks;
filter().setDecoderFilterCallbacks(decoder_callbacks);
EXPECT_CALL(decoder_callbacks, clearRouteCache()).Times(2);

Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}, {"server", "envoy"}};
EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter().decodeHeaders(request_headers, false));
EXPECT_THAT(request_headers.get_("newheader"), Eq("newheadervalue"));
Expand All @@ -145,8 +157,13 @@ TEST_P(WasmHttpFilterTest, AllHeadersAndTrailers) {
EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter().decodeTrailers(request_trailers));
Http::MetadataMap request_metadata{};
EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter().decodeMetadata(request_metadata));

// Verify that route cache is NOT cleared when modifying HTTP response headers.
EXPECT_CALL(decoder_callbacks, clearRouteCache()).Times(0);

Http::TestResponseHeaderMapImpl response_headers{};
EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter().encodeHeaders(response_headers, false));
EXPECT_THAT(response_headers.get_("test-status"), Eq("OK"));
Http::TestResponseTrailerMapImpl response_trailers{};
EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter().encodeTrailers(response_trailers));
Http::MetadataMap response_metadata{};
Expand Down

0 comments on commit ca7250d

Please sign in to comment.