Skip to content
6 changes: 6 additions & 0 deletions source/extensions/filters/http/ext_authz/ext_authz.cc
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,12 @@ bool Filter::isBufferFull() const {
}

void Filter::continueDecoding() {
if (buffer_data_ && !skip_check_) {
// When the filter is asked to buffer the data but the buffer is full, it skips buffering more
// data for the next iteration.
buffer_data_ = !isBufferFull();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you just do buffer_data_ = false; here? When we continue after a pause aren't we done and never want to buffer again?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. Thank you for this. Updated.


filter_return_ = FilterReturn::ContinueDecoding;
if (!initiating_call_) {
callbacks_->continueDecoding();
Expand Down
56 changes: 56 additions & 0 deletions test/extensions/filters/http/ext_authz/ext_authz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,62 @@ TEST_F(HttpFilterTest, RequestDataWithPartialMessage) {
EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_trailers_));
}

// Checks that the filter initiates an authorization request when the buffer reaches maximum
// request bytes and allow_partial_message is set to true. In addition to that, after the filter
// sending the check request, data decoding continues.
Comment thread
dio marked this conversation as resolved.
Outdated
TEST_F(HttpFilterTest, RequestDataWithPartialMessageThenContinueDecoding) {
InSequence s;

initialize(R"EOF(
grpc_service:
envoy_grpc:
cluster_name: "ext_authz_server"
failure_mode_allow: false
with_request_body:
max_request_bytes: 10
allow_partial_message: true
)EOF");

ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_));
ON_CALL(filter_callbacks_, decodingBuffer()).WillByDefault(Return(&data_));
EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(0);
EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_));
EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_));

// The check call should only be called once.
EXPECT_CALL(*client_, check(_, _, _, testing::A<Tracing::Span&>(), _))
.WillOnce(
WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void {
request_callbacks_ = &callbacks;
})));

EXPECT_CALL(filter_callbacks_, continueDecoding());

EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_->decodeHeaders(request_headers_, false));

data_.add("foo");
EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false));

data_.add("bar");
EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false));

data_.add("barfoo");
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_, false));

Filters::Common::ExtAuthz::Response response{};
response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
request_callbacks_->onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));

data_.add("more data after calling check request");
EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, true));

EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers_));
}

// Checks that the filter initiates the authorization process only when the filter decode trailers
// is called.
TEST_F(HttpFilterTest, RequestDataWithSmallBuffer) {
Expand Down