ext_authz: fix body buffering feature on ExtAuthZ per-route filter (e…

…nvoyproxy#31437)

Signed-off-by: Rohit Agrawal <[email protected]>

changelogs/current.yaml

@@ -146,6 +146,9 @@ bug_fixes:
 - area: docker
   change: |
     Updated base image to ``ubuntu:22.04`` to fix Redis memory issue (https://github.com/envoyproxy/envoy/issues/31248).
+- area: ext_authz
+  change: |
+    Fixed a bug to ensure the proper functioning of the ``with_request_body`` feature within the per-route ExtAuthZ filter.
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

source/extensions/filters/http/ext_authz/ext_authz.cc

@@ -104,9 +104,9 @@ void Filter::initiateCall(const Http::RequestHeaderMap& headers) {
 
   Filters::Common::ExtAuthz::CheckRequestUtils::createHttpCheck(
       decoder_callbacks_, headers, std::move(context_extensions), std::move(metadata_context),
-      std::move(route_metadata_context), check_request_, config_->maxRequestBytes(),
-      config_->packAsBytes(), config_->includePeerCertificate(), config_->includeTLSSession(),
-      config_->destinationLabels(), config_->requestHeaderMatchers());
+      std::move(route_metadata_context), check_request_, max_request_bytes_, config_->packAsBytes(),
+      config_->includePeerCertificate(), config_->includeTLSSession(), config_->destinationLabels(),
+      config_->requestHeaderMatchers());
 
   ENVOY_STREAM_LOG(trace, "ext_authz filter calling authorization server", *decoder_callbacks_);
   // Store start time of ext_authz filter call
@@ -155,16 +155,15 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers,
   if (buffer_data_) {
     ENVOY_STREAM_LOG(debug, "ext_authz filter is buffering the request", *decoder_callbacks_);
 
-    const auto allow_partial_message =
-        check_settings.has_with_request_body()
-            ? check_settings.with_request_body().allow_partial_message()
-            : config_->allowPartialMessage();
-    const auto max_request_bytes = check_settings.has_with_request_body()
-                                       ? check_settings.with_request_body().max_request_bytes()
-                                       : config_->maxRequestBytes();
+    allow_partial_message_ = check_settings.has_with_request_body()
+                                 ? check_settings.with_request_body().allow_partial_message()
+                                 : config_->allowPartialMessage();
+    max_request_bytes_ = check_settings.has_with_request_body()
+                             ? check_settings.with_request_body().max_request_bytes()
+                             : config_->maxRequestBytes();
 
-    if (!allow_partial_message) {
-      decoder_callbacks_->setDecoderBufferLimit(max_request_bytes);
+    if (!allow_partial_message_) {
+      decoder_callbacks_->setDecoderBufferLimit(max_request_bytes_);
     }
     return Http::FilterHeadersStatus::StopIteration;
   }
@@ -479,7 +478,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) {
 }
 
 bool Filter::isBufferFull(uint64_t num_bytes_processing) const {
-  if (!config_->allowPartialMessage()) {
+  if (!allow_partial_message_) {
     return false;
   }
 
@@ -489,7 +488,7 @@ bool Filter::isBufferFull(uint64_t num_bytes_processing) const {
     num_bytes_buffered += buffer->length();
   }
 
-  return num_bytes_buffered >= config_->maxRequestBytes();
+  return num_bytes_buffered >= max_request_bytes_;
 }
 
 void Filter::continueDecoding() {

source/extensions/filters/http/ext_authz/ext_authz.h

@@ -386,6 +386,10 @@ class Filter : public Logger::Loggable<Logger::Id::ext_authz>,
   // The stats for the filter.
   ExtAuthzFilterStats stats_;
 
+  // This is used to hold the final configs after we merge them with per-route configs.
+  bool allow_partial_message_{};
+  uint32_t max_request_bytes_;
+
   // Used to identify if the callback to onComplete() is synchronous (on the stack) or asynchronous.
   bool initiating_call_{};
   bool buffer_data_{};

test/extensions/filters/http/ext_authz/ext_authz_test.cc

@@ -2974,7 +2974,7 @@ TEST_P(HttpFilterTestParam, NoCluster) {
 
 // Check that config validation for per-route filter works as expected.
 TEST_F(HttpFilterTest, PerRouteCheckSettingsConfigCheck) {
-  // Set allow_partial_message to true and max_request_bytes to 10 on the per-route filter.
+  // Set allow_partial_message to true and max_request_bytes to 5 on the per-route filter.
   envoy::extensions::filters::http::ext_authz::v3::BufferSettings buffer_settings;
   buffer_settings.set_max_request_bytes(5);        // Set the max_request_bytes value
   buffer_settings.set_allow_partial_message(true); // Set the allow_partial_message value
@@ -3004,7 +3004,7 @@ TEST_F(HttpFilterTest, PerRouteCheckSettingsWorks) {
   failure_mode_allow: false
   )EOF");
 
-  // Set allow_partial_message to true and max_request_bytes to 10 on the per-route filter.
+  // Set allow_partial_message to true and max_request_bytes to 5 on the per-route filter.
   envoy::extensions::filters::http::ext_authz::v3::BufferSettings buffer_settings;
   buffer_settings.set_max_request_bytes(5);        // Set the max_request_bytes value
   buffer_settings.set_allow_partial_message(true); // Set the allow_partial_message value
@@ -3036,7 +3036,7 @@ TEST_F(HttpFilterTest, PerRouteCheckSettingsWorks) {
   data_.add(buffer1.toString());
 
   Buffer::OwnedImpl buffer2("bar");
-  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(buffer2, false));
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(buffer2, true));
   data_.add(buffer2.toString());
 
   Buffer::OwnedImpl buffer3("barfoo");
@@ -3066,7 +3066,7 @@ TEST_F(HttpFilterTest, PerRouteCheckSettingsOverrideWorks) {
 
   // Set allow_partial_message to true and max_request_bytes to 10 on the per-route filter.
   envoy::extensions::filters::http::ext_authz::v3::BufferSettings buffer_settings;
-  buffer_settings.set_max_request_bytes(5);        // Set the max_request_bytes value
+  buffer_settings.set_max_request_bytes(10);       // Set the max_request_bytes value
   buffer_settings.set_allow_partial_message(true); // Set the allow_partial_message value
   // Set the per-route filter config.
   envoy::extensions::filters::http::ext_authz::v3::CheckSettings check_settings;