From 2e501e189b550f22ed1f60dc47fdb32fb6090fca Mon Sep 17 00:00:00 2001 From: Dhi Aurrahman Date: Wed, 15 May 2019 15:15:29 +0700 Subject: [PATCH 1/2] Make sure initiateCall only called once After watermarked, it is possible that the decodeData will be called again. Hence, check if we have finished decoding and do initiateCall only once. Signed-off-by: Dhi Aurrahman --- source/extensions/filters/http/ext_authz/ext_authz.cc | 10 +++++++--- .../filters/http/ext_authz/ext_authz_test.cc | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index c7b4bb98779a7..7c7c500673162 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -96,9 +96,13 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) { if (buffer_data_) { - if (end_stream || isBufferFull()) { - ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request", *callbacks_); - initiateCall(*request_headers_); + const bool buffer_is_full = isBufferFull(); + if (end_stream || buffer_is_full) { + if (filter_return_ != FilterReturn::StopDecoding) { + ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request since {}", + *callbacks_, buffer_is_full ? "buffer is full" : "stream is ended"); + initiateCall(*request_headers_); + } return filter_return_ == FilterReturn::StopDecoding ? Http::FilterDataStatus::StopIterationAndWatermark : Http::FilterDataStatus::Continue; diff --git a/test/extensions/filters/http/ext_authz/ext_authz_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_test.cc index e011340c5a16b..ee929316cf652 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc @@ -329,6 +329,10 @@ TEST_F(HttpFilterTest, RequestDataWithPartialMessage) { data_.add("barfoo"); EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true)); + + data_.add("more data after watermark is set is possible"); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true)); + EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_)); } From c6b9334bbb7d84ed0b9b60ac9cff04a92c39d704 Mon Sep 17 00:00:00 2001 From: Dhi Aurrahman Date: Thu, 16 May 2019 17:11:23 +0700 Subject: [PATCH 2/2] Review comments Signed-off-by: Dhi Aurrahman --- .../extensions/filters/http/ext_authz/ext_authz.cc | 12 +++++++----- .../filters/http/ext_authz/ext_authz_test.cc | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc index 7c7c500673162..cd366c8f0de32 100644 --- a/source/extensions/filters/http/ext_authz/ext_authz.cc +++ b/source/extensions/filters/http/ext_authz/ext_authz.cc @@ -31,6 +31,10 @@ void FilterConfigPerRoute::merge(const FilterConfigPerRoute& other) { } void Filter::initiateCall(const Http::HeaderMap& headers) { + if (filter_return_ == FilterReturn::StopDecoding) { + return; + } + Router::RouteConstSharedPtr route = callbacks_->route(); if (route == nullptr || route->routeEntry() == nullptr) { return; @@ -98,11 +102,9 @@ Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) { if (buffer_data_) { const bool buffer_is_full = isBufferFull(); if (end_stream || buffer_is_full) { - if (filter_return_ != FilterReturn::StopDecoding) { - ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request since {}", - *callbacks_, buffer_is_full ? "buffer is full" : "stream is ended"); - initiateCall(*request_headers_); - } + ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request since {}", + *callbacks_, buffer_is_full ? "buffer is full" : "stream is ended"); + initiateCall(*request_headers_); return filter_return_ == FilterReturn::StopDecoding ? Http::FilterDataStatus::StopIterationAndWatermark : Http::FilterDataStatus::Continue; diff --git a/test/extensions/filters/http/ext_authz/ext_authz_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_test.cc index ee929316cf652..df2af91180eef 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc @@ -328,7 +328,7 @@ TEST_F(HttpFilterTest, RequestDataWithPartialMessage) { EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false)); data_.add("barfoo"); - EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, false)); data_.add("more data after watermark is set is possible"); EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true));