From 7c8f57fe7531715243b36b0988422fad30f063c7 Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Fri, 25 Jan 2019 21:44:52 -0800
Subject: [PATCH 01/17] wip - initial commit, missing tests

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../filter/http/ext_authz/v2/ext_authz.proto  | 14 ++++++
 source/common/http/headers.h                  |  3 +-
 .../filters/http/ext_authz/ext_authz.cc       | 50 ++++++++++++++++++-
 .../filters/http/ext_authz/ext_authz.h        | 20 ++++++--
 4 files changed, 81 insertions(+), 6 deletions(-)

diff --git a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
index 0e385a6eb7ce9..20987595ae38b 100644
--- a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
+++ b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
@@ -38,6 +38,20 @@ message ExtAuthz {
   // Note that errors can be *always* tracked in the :ref:`stats
   // <config_http_filters_ext_authz_stats>`.
   bool failure_mode_allow = 2;
+
+  // Enables filter to buffer the message request body and to dispatch it over the external
+  // authorization server for further validation.
+  BufferSettings with_request_data = 4;
+}
+
+// Configuration for enabling the filter's buffering mechenism.
+message BufferSettings {
+  // Sets the maximum size of a request message that the filter will hold in memory.
+  uint32 max_request_bytes = 1 [(validate.rules).uint32.gt = 0];
+
+  // When set to true, filter buffers the request body until *max_request_bytes* is reached.
+  // (ELABORATE IT)
+  bool allow_partial_message = 2;
 }
 
 // HttpService is used for raw HTTP comunication between the filter and the authorization service.
diff --git a/source/common/http/headers.h b/source/common/http/headers.h
index 0f90238da3d70..79c0b1cd153c7 100644
--- a/source/common/http/headers.h
+++ b/source/common/http/headers.h
@@ -172,8 +172,9 @@ class HeaderValues {
     const std::string Connect{"CONNECT"};
     const std::string Get{"GET"};
     const std::string Head{"HEAD"};
-    const std::string Post{"POST"};
     const std::string Options{"OPTIONS"};
+    const std::string Post{"POST"};
+    const std::string Trace{"TRACE"};
   } MethodValues;
 
   struct {
diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc
index a4dac53b8f2ad..7e597094734cd 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.cc
+++ b/source/extensions/filters/http/ext_authz/ext_authz.cc
@@ -12,6 +12,22 @@ namespace Extensions {
 namespace HttpFilters {
 namespace ExtAuthz {
 
+namespace {
+// This in conjunction with filter decoder header and end_stream parameter to make sure that request
+// has no massage body. In theory, end of stream should be received in most of cases which
+// header-only request methods are used. However, this is not true when WebSocket handshake is
+// performed which the request method is a GET, but the end of stream is false.
+bool isHeaderOnlyMethod(absl::string_view method) {
+  // List of request headers-only method (https://tools.ietf.org/html/rfc7231#section-4.3).
+  const static std::vector<std::string>* keys = new std::vector<std::string>(
+      {Http::Headers::get().MethodValues.Get, Http::Headers::get().MethodValues.Head,
+       Http::Headers::get().MethodValues.Connect, Http::Headers::get().MethodValues.Options,
+       Http::Headers::get().MethodValues.Trace});
+
+  return std::any_of(keys->begin(), keys->end(), [&method](const auto& key) { return key == method; });
+}
+} // namespace
+
 void FilterConfigPerRoute::merge(const FilterConfigPerRoute& other) {
   disabled_ = other.disabled_;
   auto begin_it = other.context_extensions_.begin();
@@ -65,14 +81,44 @@ void Filter::initiateCall(const Http::HeaderMap& headers) {
   initiating_call_ = false;
 }
 
-Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool) {
+Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool end_stream) {
   request_headers_ = &headers;
+
+  buffer_data_ = config_->withRequestData() &&
+                 !(end_stream || isHeaderOnlyMethod(headers.Method()->value().getStringView()));
+  if (buffer_data_) {
+    ENVOY_STREAM_LOG(debug, "ext_authz is buffering", *callbacks_);
+
+    if (!config_->allowPartialMessage()) {
+      callbacks_->setDecoderBufferLimit(config_->maxRequestBytes());
+    }
+
+    return Http::FilterHeadersStatus::StopIteration;
+  }
+
   initiateCall(headers);
   return filter_return_ == FilterReturn::StopDecoding ? Http::FilterHeadersStatus::StopIteration
                                                       : Http::FilterHeadersStatus::Continue;
 }
 
-Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool) {
+Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) {
+
+   if (buffer_data_) {
+    if (!end_stream) {
+      if (config_->allowPartialMessage()) {
+        const auto* buffer = callbacks_->decodingBuffer();
+        if (buffer != nullptr && buffer->length() < config_->maxRequestBytes()) {
+          return Http::FilterDataStatus::StopIterationAndBuffer;  
+        }
+      } else {
+        return Http::FilterDataStatus::StopIterationAndBuffer;  
+      }
+    }
+  
+    ENVOY_STREAM_LOG(debug, "ext_authz initiating call after buffering", *callbacks_);
+    initiateCall(*request_headers_);  
+  }
+
   return filter_return_ == FilterReturn::StopDecoding
              ? Http::FilterDataStatus::StopIterationAndWatermark
              : Http::FilterDataStatus::Continue;
diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h
index 1993ec641c18e..785cfbe145588 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.h
+++ b/source/extensions/filters/http/ext_authz/ext_authz.h
@@ -39,18 +39,31 @@ class FilterConfig {
   FilterConfig(const envoy::config::filter::http::ext_authz::v2::ExtAuthz& config,
                const LocalInfo::LocalInfo& local_info, Stats::Scope& scope,
                Runtime::Loader& runtime, Http::Context& http_context)
-      : failure_mode_allow_(config.failure_mode_allow()), local_info_(local_info), scope_(scope),
-        runtime_(runtime), http_context_(http_context) {}
+      : allow_partial_message_(config.with_request_data().allow_partial_message()),
+        failure_mode_allow_(config.failure_mode_allow()),
+        max_request_bytes_(config.with_request_data().max_request_bytes()), local_info_(local_info),
+        scope_(scope), runtime_(runtime), http_context_(http_context) {}
+
+  bool allowPartialMessage() const { return allow_partial_message_; }
+
+  bool withRequestData() const { return max_request_bytes_ > 0; }
+
+  uint32_t maxRequestBytes() const { return max_request_bytes_; }
 
   bool failureModeAllow() const { return failure_mode_allow_; }
+
   const LocalInfo::LocalInfo& localInfo() const { return local_info_; }
+
   Runtime::Loader& runtime() { return runtime_; }
+
   Stats::Scope& scope() { return scope_; }
 
   Http::Context& httpContext() { return http_context_; }
 
 private:
-  bool failure_mode_allow_{};
+  const bool allow_partial_message_;
+  const bool failure_mode_allow_;
+  const uint32_t max_request_bytes_;
   const LocalInfo::LocalInfo& local_info_;
   Stats::Scope& scope_;
   Runtime::Loader& runtime_;
@@ -139,6 +152,7 @@ class Filter : public Logger::Loggable<Logger::Id::filter>,
 
   // Used to identify if the callback to onComplete() is synchronous (on the stack) or asynchronous.
   bool initiating_call_{};
+  bool buffer_data_{};
   envoy::service::auth::v2::CheckRequest check_request_{};
 };
 

From 34796d859d6df3de6bd0836a66ca3bf3312d489f Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Sat, 26 Jan 2019 10:43:19 -0800
Subject: [PATCH 02/17] wip - saving the work

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 source/extensions/filters/http/ext_authz/ext_authz.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc
index 7e597094734cd..e0028bf46e29a 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.cc
+++ b/source/extensions/filters/http/ext_authz/ext_authz.cc
@@ -108,10 +108,10 @@ Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) {
       if (config_->allowPartialMessage()) {
         const auto* buffer = callbacks_->decodingBuffer();
         if (buffer != nullptr && buffer->length() < config_->maxRequestBytes()) {
-          return Http::FilterDataStatus::StopIterationAndBuffer;  
+          return Http::FilterDataStatus::StopIterationAndWatermark;  
         }
       } else {
-        return Http::FilterDataStatus::StopIterationAndBuffer;  
+        return Http::FilterDataStatus::StopIterationAndBuffer;
       }
     }
   

From c16ad7a96d783e88b1c6896b785b6b995f28c05e Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Thu, 31 Jan 2019 00:17:20 -0800
Subject: [PATCH 03/17] added more tests and improved documentation

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../filter/http/ext_authz/v2/ext_authz.proto  |  15 +-
 api/envoy/service/auth/v2/BUILD               |   1 +
 .../service/auth/v2/attribute_context.proto   |   4 +
 .../common/ext_authz/check_request_utils.cc   |  16 +-
 .../common/ext_authz/check_request_utils.h    |   9 +-
 .../common/ext_authz/ext_authz_http_impl.cc   |  20 ++-
 .../filters/http/ext_authz/ext_authz.cc       |  37 ++---
 .../filters/http/ext_authz/ext_authz.h        |  10 +-
 .../ext_authz/check_request_utils_test.cc     |  12 +-
 .../filters/http/ext_authz/config_test.cc     |   2 +
 .../filters/http/ext_authz/ext_authz_test.cc  | 137 +++++++++++++++++-
 11 files changed, 209 insertions(+), 54 deletions(-)

diff --git a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
index 20987595ae38b..8856c626f01df 100644
--- a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
+++ b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
@@ -39,19 +39,16 @@ message ExtAuthz {
   // <config_http_filters_ext_authz_stats>`.
   bool failure_mode_allow = 2;
 
-  // Enables filter to buffer the message request body and to dispatch it over the external
-  // authorization server for further validation.
-  BufferSettings with_request_data = 4;
+  // Enables filter to buffer and add the request body to the authorization request.
+  BufferSettings with_request_body = 4;
 }
 
-// Configuration for enabling the filter's buffering mechenism.
+// Configuration for buffering the request data.
 message BufferSettings {
-  // Sets the maximum size of a request message that the filter will hold in memory.
+  // Sets the maximum size of a message body that the filter will hold in memory. Envoy will return
+  // *HTTP 413* and will *not* initiate the authorization process when buffer reaches the number
+  // set in this field.
   uint32 max_request_bytes = 1 [(validate.rules).uint32.gt = 0];
-
-  // When set to true, filter buffers the request body until *max_request_bytes* is reached.
-  // (ELABORATE IT)
-  bool allow_partial_message = 2;
 }
 
 // HttpService is used for raw HTTP comunication between the filter and the authorization service.
diff --git a/api/envoy/service/auth/v2/BUILD b/api/envoy/service/auth/v2/BUILD
index 5faba48ac3dbc..2ee5e5044db28 100644
--- a/api/envoy/service/auth/v2/BUILD
+++ b/api/envoy/service/auth/v2/BUILD
@@ -9,6 +9,7 @@ api_proto_library_internal(
     ],
     deps = [
         "//envoy/api/v2/core:address",
+        "//envoy/api/v2/core:base",
     ],
 )
 
diff --git a/api/envoy/service/auth/v2/attribute_context.proto b/api/envoy/service/auth/v2/attribute_context.proto
index ca3fde9cfec96..0c04fd290eb5b 100644
--- a/api/envoy/service/auth/v2/attribute_context.proto
+++ b/api/envoy/service/auth/v2/attribute_context.proto
@@ -3,6 +3,7 @@ syntax = "proto3";
 package envoy.service.auth.v2;
 option java_package = "io.envoyproxy.envoy.service.auth.v2";
 
+import "envoy/api/v2/core/base.proto";
 import "envoy/api/v2/core/address.proto";
 
 import "google/protobuf/timestamp.proto";
@@ -102,6 +103,9 @@ message AttributeContext {
     // The network protocol used with the request, such as
     // "http/1.1", "spdy/3", "h2", "h2c"
     string protocol = 10;
+
+    // The HTTP request body.
+    envoy.api.v2.core.DataSource body = 11;
   }
 
   // The source of a network activity, such as starting a TCP connection.
diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.cc b/source/extensions/filters/common/ext_authz/check_request_utils.cc
index 30474ad1b2962..c8661634b65fd 100644
--- a/source/extensions/filters/common/ext_authz/check_request_utils.cc
+++ b/source/extensions/filters/common/ext_authz/check_request_utils.cc
@@ -74,7 +74,7 @@ std::string CheckRequestUtils::getHeaderStr(const Envoy::Http::HeaderEntry* entr
 void CheckRequestUtils::setHttpRequest(
     ::envoy::service::auth::v2::AttributeContext_HttpRequest& httpreq,
     const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
-    const Envoy::Http::HeaderMap& headers) {
+    const Envoy::Http::HeaderMap& headers, bool with_request_body) {
 
   // Set id
   // The streamId is not qualified as a const. Although it is as it does not modify the object.
@@ -114,20 +114,26 @@ void CheckRequestUtils::setHttpRequest(
         return Envoy::Http::HeaderMap::Iterate::Continue;
       },
       mutable_headers);
+
+  // Set request body.
+  const Buffer::Instance* buffer = sdfc->decodingBuffer();
+  if (with_request_body && buffer != nullptr) {
+    httpreq.mutable_body()->set_inline_bytes(buffer->toString());
+  }
 }
 
 void CheckRequestUtils::setAttrContextRequest(
     ::envoy::service::auth::v2::AttributeContext_Request& req,
     const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
-    const Envoy::Http::HeaderMap& headers) {
-  setHttpRequest(*req.mutable_http(), callbacks, headers);
+    const Envoy::Http::HeaderMap& headers, bool with_request_body) {
+  setHttpRequest(*req.mutable_http(), callbacks, headers, with_request_body);
 }
 
 void CheckRequestUtils::createHttpCheck(
     const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
     const Envoy::Http::HeaderMap& headers,
     Protobuf::Map<ProtobufTypes::String, ProtobufTypes::String>&& context_extensions,
-    envoy::service::auth::v2::CheckRequest& request) {
+    envoy::service::auth::v2::CheckRequest& request, bool with_request_body) {
 
   auto attrs = request.mutable_attributes();
 
@@ -138,7 +144,7 @@ void CheckRequestUtils::createHttpCheck(
 
   setAttrContextPeer(*attrs->mutable_source(), *cb->connection(), service, false);
   setAttrContextPeer(*attrs->mutable_destination(), *cb->connection(), "", true);
-  setAttrContextRequest(*attrs->mutable_request(), callbacks, headers);
+  setAttrContextRequest(*attrs->mutable_request(), callbacks, headers, with_request_body);
 
   // Fill in the context extensions:
   (*attrs->mutable_context_extensions()) = std::move(context_extensions);
diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.h b/source/extensions/filters/common/ext_authz/check_request_utils.h
index a3214d17b50d9..ffc4d8daba11d 100644
--- a/source/extensions/filters/common/ext_authz/check_request_utils.h
+++ b/source/extensions/filters/common/ext_authz/check_request_utils.h
@@ -43,20 +43,19 @@ class CheckRequestUtils {
    * @param headers supplies the header map with http headers that will be used to create the
    *        check request.
    * @param request is the reference to the check request that will be filled up.
-   *
+   * @param with_request_body when true, will add the request body to the check request.
    */
   static void
   createHttpCheck(const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
                   const Envoy::Http::HeaderMap& headers,
                   Protobuf::Map<ProtobufTypes::String, ProtobufTypes::String>&& context_extensions,
-                  envoy::service::auth::v2::CheckRequest& request);
+                  envoy::service::auth::v2::CheckRequest& request, bool with_request_body);
 
   /**
    * createTcpCheck is used to extract the attributes from the network layer and fill them up
    * in the CheckRequest proto message.
    * @param callbacks supplies the network layer context from which data can be extracted.
    * @param request is the reference to the check request that will be filled up.
-   *
    */
   static void createTcpCheck(const Network::ReadFilterCallbacks* callbacks,
                              envoy::service::auth::v2::CheckRequest& request);
@@ -67,10 +66,10 @@ class CheckRequestUtils {
                                  const bool local);
   static void setHttpRequest(::envoy::service::auth::v2::AttributeContext_HttpRequest& httpreq,
                              const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
-                             const Envoy::Http::HeaderMap& headers);
+                             const Envoy::Http::HeaderMap& headers, bool with_request_body);
   static void setAttrContextRequest(::envoy::service::auth::v2::AttributeContext_Request& req,
                                     const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
-                                    const Envoy::Http::HeaderMap& headers);
+                                    const Envoy::Http::HeaderMap& headers, bool with_request_body);
   static std::string getHeaderStr(const Envoy::Http::HeaderEntry* entry);
   static Envoy::Http::HeaderMap::Iterate fillHttpHeaders(const Envoy::Http::HeaderEntry&, void*);
 };
diff --git a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc
index de58a3fae12a2..bce9fa0c1bdd4 100644
--- a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc
+++ b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc
@@ -161,7 +161,17 @@ void RawHttpClientImpl::check(RequestCallbacks& callbacks,
   ASSERT(callbacks_ == nullptr);
   callbacks_ = &callbacks;
 
-  Http::HeaderMapPtr headers = std::make_unique<Http::HeaderMapImpl>(lengthZeroHeader());
+  Http::HeaderMapPtr headers{};
+  const uint64_t request_length =
+      request.attributes().request().http().body().inline_bytes().length();
+  if (request_length > 0) {
+    const Http::HeaderMap& header_map =
+        Http::HeaderMapImpl{{Http::Headers::get().ContentLength, std::to_string(request_length)}};
+    headers = std::make_unique<Http::HeaderMapImpl>(header_map);
+  } else {
+    headers = std::make_unique<Http::HeaderMapImpl>(lengthZeroHeader());
+  }
+
   for (const auto& header : request.attributes().request().http().headers()) {
     const Http::LowerCaseString key{header.first};
     if (config_->requestHeaderMatchers()->matches(key.get())) {
@@ -179,8 +189,14 @@ void RawHttpClientImpl::check(RequestCallbacks& callbacks,
     headers->setReference(header_to_add.first, header_to_add.second);
   }
 
+  Http::MessagePtr message = std::make_unique<Envoy::Http::RequestMessageImpl>(std::move(headers));
+  if (request_length > 0) {
+    message->body() = std::make_unique<Buffer::OwnedImpl>(
+        request.attributes().request().http().body().inline_bytes());
+  }
+
   request_ = cm_.httpAsyncClientForCluster(config_->cluster())
-                 .send(std::make_unique<Envoy::Http::RequestMessageImpl>(std::move(headers)), *this,
+                 .send(std::move(message), *this,
                        Http::AsyncClient::RequestOptions().setTimeout(config_->timeout()));
 }
 
diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc
index e0028bf46e29a..029f9dc7c3ecb 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.cc
+++ b/source/extensions/filters/http/ext_authz/ext_authz.cc
@@ -24,7 +24,8 @@ bool isHeaderOnlyMethod(absl::string_view method) {
        Http::Headers::get().MethodValues.Connect, Http::Headers::get().MethodValues.Options,
        Http::Headers::get().MethodValues.Trace});
 
-  return std::any_of(keys->begin(), keys->end(), [&method](const auto& key) { return key == method; });
+  return std::any_of(keys->begin(), keys->end(),
+                     [&method](const auto& key) { return key == method; });
 }
 } // namespace
 
@@ -70,7 +71,7 @@ void Filter::initiateCall(const Http::HeaderMap& headers) {
     context_extensions = maybe_merged_per_route_config.value().takeContextExtensions();
   }
   Filters::Common::ExtAuthz::CheckRequestUtils::createHttpCheck(
-      callbacks_, headers, std::move(context_extensions), check_request_);
+      callbacks_, headers, std::move(context_extensions), check_request_, buffer_data_);
 
   state_ = State::Calling;
   // Don't let the filter chain continue as we are going to invoke check call.
@@ -83,16 +84,13 @@ void Filter::initiateCall(const Http::HeaderMap& headers) {
 
 Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool end_stream) {
   request_headers_ = &headers;
+  const auto* method = headers.Method();
+  buffer_data_ = config_->withRequestBody() &&
+                 !(end_stream || (method && isHeaderOnlyMethod(method->value().getStringView())));
 
-  buffer_data_ = config_->withRequestData() &&
-                 !(end_stream || isHeaderOnlyMethod(headers.Method()->value().getStringView()));
   if (buffer_data_) {
     ENVOY_STREAM_LOG(debug, "ext_authz is buffering", *callbacks_);
-
-    if (!config_->allowPartialMessage()) {
-      callbacks_->setDecoderBufferLimit(config_->maxRequestBytes());
-    }
-
+    callbacks_->setDecoderBufferLimit(config_->maxRequestBytes());
     return Http::FilterHeadersStatus::StopIteration;
   }
 
@@ -102,21 +100,16 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e
 }
 
 Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) {
-
-   if (buffer_data_) {
+  if (buffer_data_) {
     if (!end_stream) {
-      if (config_->allowPartialMessage()) {
-        const auto* buffer = callbacks_->decodingBuffer();
-        if (buffer != nullptr && buffer->length() < config_->maxRequestBytes()) {
-          return Http::FilterDataStatus::StopIterationAndWatermark;  
-        }
-      } else {
-        return Http::FilterDataStatus::StopIterationAndBuffer;
-      }
+      // Buffer until the complete request has been processed or the ConnectionManagerImpl sends a
+      // 413.
+      return Http::FilterDataStatus::StopIterationAndBuffer;
     }
-  
-    ENVOY_STREAM_LOG(debug, "ext_authz initiating call after buffering", *callbacks_);
-    initiateCall(*request_headers_);  
+
+    // Call the authorization service.
+    ENVOY_STREAM_LOG(debug, "ext_authz finished buffering", *callbacks_);
+    initiateCall(*request_headers_);
   }
 
   return filter_return_ == FilterReturn::StopDecoding
diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h
index 785cfbe145588..4d92f91225cde 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.h
+++ b/source/extensions/filters/http/ext_authz/ext_authz.h
@@ -39,14 +39,11 @@ class FilterConfig {
   FilterConfig(const envoy::config::filter::http::ext_authz::v2::ExtAuthz& config,
                const LocalInfo::LocalInfo& local_info, Stats::Scope& scope,
                Runtime::Loader& runtime, Http::Context& http_context)
-      : allow_partial_message_(config.with_request_data().allow_partial_message()),
-        failure_mode_allow_(config.failure_mode_allow()),
-        max_request_bytes_(config.with_request_data().max_request_bytes()), local_info_(local_info),
+      : failure_mode_allow_(config.failure_mode_allow()),
+        max_request_bytes_(config.with_request_body().max_request_bytes()), local_info_(local_info),
         scope_(scope), runtime_(runtime), http_context_(http_context) {}
 
-  bool allowPartialMessage() const { return allow_partial_message_; }
-
-  bool withRequestData() const { return max_request_bytes_ > 0; }
+  bool withRequestBody() const { return max_request_bytes_ > 0; }
 
   uint32_t maxRequestBytes() const { return max_request_bytes_; }
 
@@ -61,7 +58,6 @@ class FilterConfig {
   Http::Context& httpContext() { return http_context_; }
 
 private:
-  const bool allow_partial_message_;
   const bool failure_mode_allow_;
   const uint32_t max_request_bytes_;
   const LocalInfo::LocalInfo& local_info_;
diff --git a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
index f03291fb2f499..c93bfd1552a62 100644
--- a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
+++ b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
@@ -26,6 +26,7 @@ class CheckRequestUtilsTest : public testing::Test {
   CheckRequestUtilsTest() {
     addr_ = std::make_shared<Network::Address::Ipv4Instance>("1.2.3.4", 1111);
     protocol_ = Envoy::Http::Protocol::Http10;
+    buffer_ = std::make_unique<Buffer::OwnedImpl>("foo");
   };
 
   Network::Address::InstanceConstSharedPtr addr_;
@@ -36,6 +37,7 @@ class CheckRequestUtilsTest : public testing::Test {
   NiceMock<Envoy::Network::MockConnection> connection_;
   NiceMock<Envoy::Ssl::MockConnection> ssl_;
   NiceMock<Envoy::StreamInfo::MockStreamInfo> req_info_;
+  Buffer::InstancePtr buffer_;
 };
 
 // Verify that createTcpCheck's dependencies are invoked when it's called.
@@ -57,12 +59,15 @@ TEST_F(CheckRequestUtilsTest, BasicHttp) {
   EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_));
   EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_));
   EXPECT_CALL(Const(connection_), ssl()).Times(2).WillRepeatedly(Return(&ssl_));
-  EXPECT_CALL(callbacks_, streamId()).WillOnce(Return(0));
+  EXPECT_CALL(callbacks_, streamId()).Times(1).WillOnce(Return(0));
+  EXPECT_CALL(callbacks_, decodingBuffer()).WillOnce(Return(buffer_.get()));
   EXPECT_CALL(callbacks_, streamInfo()).Times(3).WillRepeatedly(ReturnRef(req_info_));
   EXPECT_CALL(req_info_, protocol()).Times(2).WillRepeatedly(ReturnPointee(&protocol_));
   Protobuf::Map<ProtobufTypes::String, ProtobufTypes::String> empty;
 
-  CheckRequestUtils::createHttpCheck(&callbacks_, headers, std::move(empty), request);
+  CheckRequestUtils::createHttpCheck(&callbacks_, headers, std::move(empty), request, true);
+
+  EXPECT_EQ("foo", request.attributes().request().http().body().inline_bytes());
 }
 
 // Verify that createHttpCheck extract the proper attributes from the http request into CheckRequest
@@ -77,6 +82,7 @@ TEST_F(CheckRequestUtilsTest, CheckAttrContextPeer) {
   EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_));
   EXPECT_CALL(callbacks_, streamId()).WillRepeatedly(Return(0));
   EXPECT_CALL(callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_));
+  EXPECT_CALL(callbacks_, decodingBuffer()).Times(1);
   EXPECT_CALL(req_info_, protocol()).WillRepeatedly(ReturnPointee(&protocol_));
   EXPECT_CALL(ssl_, uriSanPeerCertificate()).WillOnce(Return("source"));
   EXPECT_CALL(ssl_, uriSanLocalCertificate()).WillOnce(Return("destination"));
@@ -85,7 +91,7 @@ TEST_F(CheckRequestUtilsTest, CheckAttrContextPeer) {
   context_extensions["key"] = "value";
 
   CheckRequestUtils::createHttpCheck(&callbacks_, request_headers, std::move(context_extensions),
-                                     request);
+                                     request, false);
 
   EXPECT_EQ("source", request.attributes().source().principal());
   EXPECT_EQ("destination", request.attributes().destination().principal());
diff --git a/test/extensions/filters/http/ext_authz/config_test.cc b/test/extensions/filters/http/ext_authz/config_test.cc
index d6043f785588e..80e5ece36b312 100644
--- a/test/extensions/filters/http/ext_authz/config_test.cc
+++ b/test/extensions/filters/http/ext_authz/config_test.cc
@@ -76,6 +76,8 @@ TEST(HttpExtAuthzConfigTest, CorrectProtoHttp) {
     path_prefix: /extauth
     
   failure_mode_allow: true
+  with_request_body:
+    max_request_bytes: 100
   )EOF";
 
   ExtAuthzFilterConfig factory;
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 f84275d633641..35ec2d9b6ad37 100644
--- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc
+++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc
@@ -65,7 +65,7 @@ template <class T> class HttpFilterTestBase : public T {
   std::unique_ptr<Filter> filter_;
   NiceMock<Http::MockStreamDecoderFilterCallbacks> filter_callbacks_;
   Filters::Common::ExtAuthz::RequestCallbacks* request_callbacks_{};
-  Http::TestHeaderMapImpl request_headers_;
+  Http::TestHeaderMapImpl request_headers_{};
   Buffer::OwnedImpl data_;
   Stats::IsolatedStoreImpl stats_store_;
   NiceMock<Runtime::MockLoader> runtime_;
@@ -263,6 +263,141 @@ TEST_F(HttpFilterTest, BadConfig) {
       ProtoValidationException);
 }
 
+// Checks that filter does not initiate the authorization request when the buffer reaches the max
+// request bytes.
+TEST_F(HttpFilterTest, RequestDataIsTooLarge) {
+  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
+  )EOF");
+
+  ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_));
+  EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(1);
+  EXPECT_CALL(connection_, remoteAddress()).Times(0);
+  EXPECT_CALL(connection_, localAddress()).Times(0);
+  EXPECT_CALL(*client_, check(_, _, _)).Times(0);
+
+  EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(request_headers_, false));
+
+  Buffer::OwnedImpl buffer1("foo");
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(buffer1, false));
+
+  Buffer::OwnedImpl buffer2("foobarbaz");
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(buffer2, false));
+}
+
+// Checks that filter buffers data and initiates the authorization request.
+TEST_F(HttpFilterTest, AuthWithRequestData) {
+  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
+  )EOF");
+
+  ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_));
+  EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(1);
+  EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_));
+  EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_));
+
+  Filters::Common::ExtAuthz::Response response{};
+  response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
+
+  EXPECT_CALL(*client_, check(_, _, _))
+      .WillOnce(
+          WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void {
+            callbacks.onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
+          })));
+  EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0);
+  EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(request_headers_, false));
+  Buffer::OwnedImpl buffer("foo");
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(buffer, false));
+  EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, true));
+  EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_));
+  EXPECT_EQ(1U, filter_callbacks_.clusterInfo()->statsScope().counter("ext_authz.ok").value());
+}
+
+// Checks that filter does not buffer data on header-only request.
+TEST_F(HttpFilterTest, AuthHeaderOnlyRequest) {
+  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
+  )EOF");
+
+  ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_));
+  EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(0);
+  EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_));
+  EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_));
+
+  Filters::Common::ExtAuthz::Response response{};
+  response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
+
+  EXPECT_CALL(*client_, check(_, _, _))
+      .WillOnce(
+          WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void {
+            callbacks.onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
+          })));
+  EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0);
+  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, true));
+  EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, true));
+  EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_));
+  EXPECT_EQ(1U, filter_callbacks_.clusterInfo()->statsScope().counter("ext_authz.ok").value());
+}
+
+// Checks that filter does not buffer data when is not the end of the stream, but header-only
+// request has been received.
+TEST_F(HttpFilterTest, HeaderOnlyRequestWithStream) {
+  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
+  )EOF");
+
+  request_headers_.addCopy(":method", "GET");
+
+  ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_));
+  EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(0);
+  EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_));
+  EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_));
+
+  Filters::Common::ExtAuthz::Response response{};
+  response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
+
+  EXPECT_CALL(*client_, check(_, _, _))
+      .WillOnce(
+          WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void {
+            callbacks.onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
+          })));
+  EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0);
+  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false));
+  EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, true));
+  EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_));
+  EXPECT_EQ(1U, filter_callbacks_.clusterInfo()->statsScope().counter("ext_authz.ok").value());
+}
+
 // -------------------
 // Parameterized Tests
 // -------------------

From a57af7ae734d2908857241c3771fe4d0908bfa87 Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Mon, 4 Feb 2019 00:47:11 -0800
Subject: [PATCH 04/17] edited comment

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 source/extensions/filters/http/ext_authz/ext_authz.cc | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc
index 029f9dc7c3ecb..5c5f820346297 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.cc
+++ b/source/extensions/filters/http/ext_authz/ext_authz.cc
@@ -13,12 +13,9 @@ namespace HttpFilters {
 namespace ExtAuthz {
 
 namespace {
-// This in conjunction with filter decoder header and end_stream parameter to make sure that request
-// has no massage body. In theory, end of stream should be received in most of cases which
-// header-only request methods are used. However, this is not true when WebSocket handshake is
-// performed which the request method is a GET, but the end of stream is false.
+// Asserts that the HTTP method is for a header-only request. For details
+// https://tools.ietf.org/html/rfc7231#section-4.3.
 bool isHeaderOnlyMethod(absl::string_view method) {
-  // List of request headers-only method (https://tools.ietf.org/html/rfc7231#section-4.3).
   const static std::vector<std::string>* keys = new std::vector<std::string>(
       {Http::Headers::get().MethodValues.Get, Http::Headers::get().MethodValues.Head,
        Http::Headers::get().MethodValues.Connect, Http::Headers::get().MethodValues.Options,

From 3f1779577c1f96c026076096a6ff2783780f642c Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Mon, 4 Feb 2019 00:58:59 -0800
Subject: [PATCH 05/17] updated the release notes

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 docs/root/intro/version_history.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
index cc3cb90f95107..0e415acabd242 100644
--- a/docs/root/intro/version_history.rst
+++ b/docs/root/intro/version_history.rst
@@ -11,6 +11,7 @@ Version history
 * config: removed deprecated_v1 sds_config from :ref:`Bootstrap config <config_overview_v2_bootstrap>`.
 * config: removed REST_LEGACY as a valid :ref:`ApiType <envoy_api_field_core.ApiConfigSource.api_type>`.
 * cors: added :ref:`filter_enabled & shadow_enabled RuntimeFractionalPercent flags <cors-runtime>` to filter.
+* ext_authz: added support for buffering request body.
 * ext_authz: migrated from V2alpha to V2 and improved docs.
 * ext_authz: authorization request and response configuration has been separated into two distinct objects: :ref:`authorization request
   <envoy_api_field_config.filter.http.ext_authz.v2.HttpService.authorization_request>` and :ref:`authorization response

From e157eabbe177b795c685e79a078b9530a5e15679 Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Thu, 28 Feb 2019 21:19:53 -0800
Subject: [PATCH 06/17] fixed spelling

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
index 98d3297bfd348..692a6de602582 100644
--- a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
+++ b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
@@ -59,8 +59,9 @@ message BufferSettings {
   // <envoy_api_field_config.filter.http.ext_authz.v2.ExtAuthz.failure_mode_allow>`.
   uint32 max_request_bytes = 1 [(validate.rules).uint32.gt = 0];
 
-  // When this field is true, Envoy buffers until *max_request_bytes* is reached. The filter will
-  // dispatche the authorization request with instead of reject it with an *HTTP 413*.
+  // When this field is true, Envoy buffers until *max_request_bytes* is reached. However, instead
+  // of rejecting the client request with an *HTTP 413*, the filter will send the authorization
+  // request with whatever data has been buffered.
   bool allow_partial_request = 2;
 }
 

From 8d747b5132dec19b90d476a01a43ba12e73b8718 Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Thu, 28 Feb 2019 21:34:32 -0800
Subject: [PATCH 07/17] fixed check request test

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../filters/common/ext_authz/check_request_utils_test.cc        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
index c93bfd1552a62..7a15284bf31af 100644
--- a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
+++ b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
@@ -67,7 +67,7 @@ TEST_F(CheckRequestUtilsTest, BasicHttp) {
 
   CheckRequestUtils::createHttpCheck(&callbacks_, headers, std::move(empty), request, true);
 
-  EXPECT_EQ("foo", request.attributes().request().http().body().inline_bytes());
+  EXPECT_EQ("foo", request.attributes().request().http().body());
 }
 
 // Verify that createHttpCheck extract the proper attributes from the http request into CheckRequest

From 255f624d7a67818a27aa2679b6bd7dac5f9d2d7a Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Sun, 3 Mar 2019 22:27:50 -0800
Subject: [PATCH 08/17] fixed check request tests

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../filter/http/ext_authz/v2/ext_authz.proto  | 10 ++---
 .../common/ext_authz/check_request_utils.cc   | 17 +++++----
 .../common/ext_authz/check_request_utils.h    |  7 ++--
 .../filters/http/ext_authz/ext_authz.cc       | 18 ++++++---
 .../filters/http/ext_authz/ext_authz.h        | 13 +++++--
 .../ext_authz/check_request_utils_test.cc     |  6 +--
 .../filters/http/ext_authz/ext_authz_test.cc  | 38 ++++++++++++++++++-
 7 files changed, 81 insertions(+), 28 deletions(-)

diff --git a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
index 692a6de602582..71b69c1792a19 100644
--- a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
+++ b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto
@@ -47,7 +47,7 @@ message ExtAuthz {
   // version upgrade. See release notes for more details.
   bool use_alpha = 4 [deprecated = true];
 
-  // Enables filter to buffer and add the request body to the authorization request.
+  // Enables filter to buffer the client request body and send it within the authorization request.
   BufferSettings with_request_body = 5;
 }
 
@@ -59,10 +59,10 @@ message BufferSettings {
   // <envoy_api_field_config.filter.http.ext_authz.v2.ExtAuthz.failure_mode_allow>`.
   uint32 max_request_bytes = 1 [(validate.rules).uint32.gt = 0];
 
-  // When this field is true, Envoy buffers until *max_request_bytes* is reached. However, instead
-  // of rejecting the client request with an *HTTP 413*, the filter will send the authorization
-  // request with whatever data has been buffered.
-  bool allow_partial_request = 2;
+  // When this field is true, Envoy will buffer the message until *max_request_bytes* is reached.
+  // The authorization request will be dispatched and no 413 HTTP error will be returned by the
+  // filter.
+  bool allow_partial_message = 2;
 }
 
 // HttpService is used for raw HTTP communication between the filter and the authorization service.
diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.cc b/source/extensions/filters/common/ext_authz/check_request_utils.cc
index 5d305385f005d..661ca25e9991b 100644
--- a/source/extensions/filters/common/ext_authz/check_request_utils.cc
+++ b/source/extensions/filters/common/ext_authz/check_request_utils.cc
@@ -74,7 +74,7 @@ std::string CheckRequestUtils::getHeaderStr(const Envoy::Http::HeaderEntry* entr
 void CheckRequestUtils::setHttpRequest(
     ::envoy::service::auth::v2::AttributeContext_HttpRequest& httpreq,
     const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
-    const Envoy::Http::HeaderMap& headers, bool with_request_body) {
+    const Envoy::Http::HeaderMap& headers, uint64_t max_request_bytes) {
 
   // Set id
   // The streamId is not qualified as a const. Although it is as it does not modify the object.
@@ -117,23 +117,26 @@ void CheckRequestUtils::setHttpRequest(
 
   // Set request body.
   const Buffer::Instance* buffer = sdfc->decodingBuffer();
-  if (with_request_body && buffer != nullptr) {
-    httpreq.set_body(buffer->toString());
+  if (max_request_bytes > 0 && buffer != nullptr) {
+    const uint64_t len = std::min(buffer->length(), max_request_bytes);
+    std::unique_ptr<char[]> data(new char[len]);
+    buffer->copyOut(0, len, data.get());
+    httpreq.set_body(data.get(), len);
   }
 }
 
 void CheckRequestUtils::setAttrContextRequest(
     ::envoy::service::auth::v2::AttributeContext_Request& req,
     const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
-    const Envoy::Http::HeaderMap& headers, bool with_request_body) {
-  setHttpRequest(*req.mutable_http(), callbacks, headers, with_request_body);
+    const Envoy::Http::HeaderMap& headers, uint64_t max_request_bytes) {
+  setHttpRequest(*req.mutable_http(), callbacks, headers, max_request_bytes);
 }
 
 void CheckRequestUtils::createHttpCheck(
     const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
     const Envoy::Http::HeaderMap& headers,
     Protobuf::Map<ProtobufTypes::String, ProtobufTypes::String>&& context_extensions,
-    envoy::service::auth::v2::CheckRequest& request, bool with_request_body) {
+    envoy::service::auth::v2::CheckRequest& request, uint64_t max_request_bytes) {
 
   auto attrs = request.mutable_attributes();
 
@@ -144,7 +147,7 @@ void CheckRequestUtils::createHttpCheck(
 
   setAttrContextPeer(*attrs->mutable_source(), *cb->connection(), service, false);
   setAttrContextPeer(*attrs->mutable_destination(), *cb->connection(), "", true);
-  setAttrContextRequest(*attrs->mutable_request(), callbacks, headers, with_request_body);
+  setAttrContextRequest(*attrs->mutable_request(), callbacks, headers, max_request_bytes);
 
   // Fill in the context extensions:
   (*attrs->mutable_context_extensions()) = std::move(context_extensions);
diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.h b/source/extensions/filters/common/ext_authz/check_request_utils.h
index ffc4d8daba11d..da89fe72fe4e7 100644
--- a/source/extensions/filters/common/ext_authz/check_request_utils.h
+++ b/source/extensions/filters/common/ext_authz/check_request_utils.h
@@ -49,7 +49,7 @@ class CheckRequestUtils {
   createHttpCheck(const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
                   const Envoy::Http::HeaderMap& headers,
                   Protobuf::Map<ProtobufTypes::String, ProtobufTypes::String>&& context_extensions,
-                  envoy::service::auth::v2::CheckRequest& request, bool with_request_body);
+                  envoy::service::auth::v2::CheckRequest& request, uint64_t max_request_bytes);
 
   /**
    * createTcpCheck is used to extract the attributes from the network layer and fill them up
@@ -66,10 +66,11 @@ class CheckRequestUtils {
                                  const bool local);
   static void setHttpRequest(::envoy::service::auth::v2::AttributeContext_HttpRequest& httpreq,
                              const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
-                             const Envoy::Http::HeaderMap& headers, bool with_request_body);
+                             const Envoy::Http::HeaderMap& headers, uint64_t max_request_bytes);
   static void setAttrContextRequest(::envoy::service::auth::v2::AttributeContext_Request& req,
                                     const Envoy::Http::StreamDecoderFilterCallbacks* callbacks,
-                                    const Envoy::Http::HeaderMap& headers, bool with_request_body);
+                                    const Envoy::Http::HeaderMap& headers,
+                                    uint64_t max_request_bytes);
   static std::string getHeaderStr(const Envoy::Http::HeaderEntry* entry);
   static Envoy::Http::HeaderMap::Iterate fillHttpHeaders(const Envoy::Http::HeaderEntry&, void*);
 };
diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc
index ab5500c696b8a..0463ca5eaacc3 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.cc
+++ b/source/extensions/filters/http/ext_authz/ext_authz.cc
@@ -70,7 +70,8 @@ void Filter::initiateCall(const Http::HeaderMap& headers) {
     context_extensions = maybe_merged_per_route_config.value().takeContextExtensions();
   }
   Filters::Common::ExtAuthz::CheckRequestUtils::createHttpCheck(
-      callbacks_, headers, std::move(context_extensions), check_request_, buffer_data_);
+      callbacks_, headers, std::move(context_extensions), check_request_,
+      config_->maxRequestBytes());
 
   state_ = State::Calling;
   // Don't let the filter chain continue as we are going to invoke check call.
@@ -89,7 +90,9 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e
 
   if (buffer_data_) {
     ENVOY_STREAM_LOG(debug, "ext_authz is buffering", *callbacks_);
-    callbacks_->setDecoderBufferLimit(config_->maxRequestBytes());
+    if (!config_->allowPartialMessage()) {
+      callbacks_->setDecoderBufferLimit(config_->maxRequestBytes());
+    }
     return Http::FilterHeadersStatus::StopIteration;
   }
 
@@ -100,9 +103,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e
 
 Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) {
   if (buffer_data_) {
-    if (!end_stream) {
-      // Buffer until the complete request has been processed or the ConnectionManagerImpl sends a
-      // 413.
+    if (!end_stream && !isBufferFull()) {
       return Http::FilterDataStatus::StopIterationAndBuffer;
     }
 
@@ -217,6 +218,13 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) {
   }
 }
 
+bool Filter::isBufferFull() {
+  if (config_->allowPartialMessage()) {
+    return callbacks_->decodingBuffer()->length() >= config_->maxRequestBytes();
+  }
+  return false;
+}
+
 } // namespace ExtAuthz
 } // namespace HttpFilters
 } // namespace Extensions
diff --git a/source/extensions/filters/http/ext_authz/ext_authz.h b/source/extensions/filters/http/ext_authz/ext_authz.h
index 4d92f91225cde..3cc8821509767 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.h
+++ b/source/extensions/filters/http/ext_authz/ext_authz.h
@@ -39,16 +39,19 @@ class FilterConfig {
   FilterConfig(const envoy::config::filter::http::ext_authz::v2::ExtAuthz& config,
                const LocalInfo::LocalInfo& local_info, Stats::Scope& scope,
                Runtime::Loader& runtime, Http::Context& http_context)
-      : failure_mode_allow_(config.failure_mode_allow()),
+      : allow_partial_message_(config.with_request_body().allow_partial_message()),
+        failure_mode_allow_(config.failure_mode_allow()),
         max_request_bytes_(config.with_request_body().max_request_bytes()), local_info_(local_info),
         scope_(scope), runtime_(runtime), http_context_(http_context) {}
 
-  bool withRequestBody() const { return max_request_bytes_ > 0; }
+  bool allowPartialMessage() const { return allow_partial_message_; }
 
-  uint32_t maxRequestBytes() const { return max_request_bytes_; }
+  bool withRequestBody() const { return max_request_bytes_ > 0; }
 
   bool failureModeAllow() const { return failure_mode_allow_; }
 
+  uint32_t maxRequestBytes() const { return max_request_bytes_; }
+
   const LocalInfo::LocalInfo& localInfo() const { return local_info_; }
 
   Runtime::Loader& runtime() { return runtime_; }
@@ -58,6 +61,7 @@ class FilterConfig {
   Http::Context& httpContext() { return http_context_; }
 
 private:
+  const bool allow_partial_message_;
   const bool failure_mode_allow_;
   const uint32_t max_request_bytes_;
   const LocalInfo::LocalInfo& local_info_;
@@ -125,6 +129,8 @@ class Filter : public Logger::Loggable<Logger::Id::filter>,
 
 private:
   void addResponseHeaders(Http::HeaderMap& header_map, const Http::HeaderVector& headers);
+  void initiateCall(const Http::HeaderMap& headers);
+  bool isBufferFull();
 
   // State of this filter's communication with the external authorization service.
   // The filter has either not started calling the external service, in the middle of calling
@@ -136,7 +142,6 @@ class Filter : public Logger::Loggable<Logger::Id::filter>,
   // the filter chain should stop. Otherwise the filter chain can continue to the next filter.
   enum class FilterReturn { ContinueDecoding, StopDecoding };
 
-  void initiateCall(const Http::HeaderMap& headers);
   Http::HeaderMapPtr getHeaderMap(const Filters::Common::ExtAuthz::ResponsePtr& response);
   FilterConfigSharedPtr config_;
   Filters::Common::ExtAuthz::ClientPtr client_;
diff --git a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
index 7a15284bf31af..dcda5d242d47b 100644
--- a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
+++ b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
@@ -26,7 +26,7 @@ class CheckRequestUtilsTest : public testing::Test {
   CheckRequestUtilsTest() {
     addr_ = std::make_shared<Network::Address::Ipv4Instance>("1.2.3.4", 1111);
     protocol_ = Envoy::Http::Protocol::Http10;
-    buffer_ = std::make_unique<Buffer::OwnedImpl>("foo");
+    buffer_ = std::make_unique<Buffer::OwnedImpl>("foobar");
   };
 
   Network::Address::InstanceConstSharedPtr addr_;
@@ -65,9 +65,9 @@ TEST_F(CheckRequestUtilsTest, BasicHttp) {
   EXPECT_CALL(req_info_, protocol()).Times(2).WillRepeatedly(ReturnPointee(&protocol_));
   Protobuf::Map<ProtobufTypes::String, ProtobufTypes::String> empty;
 
-  CheckRequestUtils::createHttpCheck(&callbacks_, headers, std::move(empty), request, true);
+  CheckRequestUtils::createHttpCheck(&callbacks_, headers, std::move(empty), request, 4);
 
-  EXPECT_EQ("foo", request.attributes().request().http().body());
+  EXPECT_EQ("foob", request.attributes().request().http().body());
 }
 
 // Verify that createHttpCheck extract the proper attributes from the http request into CheckRequest
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 e06bd921873a8..4919075476cf0 100644
--- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc
+++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc
@@ -292,7 +292,43 @@ TEST_F(HttpFilterTest, RequestDataIsTooLarge) {
   EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(buffer2, false));
 }
 
-// Checks that filter buffers data and initiates the authorization request.
+// Checks that the filter initiates an authorization request when the buffer reaches max
+// request bytes and allow_partial_message is set to true.
+TEST_F(HttpFilterTest, RequestDataWithPartialMessage) {
+  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_));
+  EXPECT_CALL(*client_, check(_, _, _)).Times(1);
+
+  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));
+}
+
+// Checks that the filter buffers the data and initiates the authorization request.
 TEST_F(HttpFilterTest, AuthWithRequestData) {
   InSequence s;
 

From 5ec3a03960719b549e7c30be83208963c5f164eb Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Wed, 13 Mar 2019 19:48:02 -0700
Subject: [PATCH 09/17] code review changes

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../common/ext_authz/check_request_utils.cc   |  9 +++--
 .../common/ext_authz/ext_authz_http_impl.cc   |  2 +-
 .../filters/http/ext_authz/ext_authz.cc       | 14 ++++---
 .../filters/http/ext_authz/ext_authz_test.cc  | 38 ++++++++++++++++++-
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.cc b/source/extensions/filters/common/ext_authz/check_request_utils.cc
index 661ca25e9991b..7431a3e44efd9 100644
--- a/source/extensions/filters/common/ext_authz/check_request_utils.cc
+++ b/source/extensions/filters/common/ext_authz/check_request_utils.cc
@@ -119,9 +119,12 @@ void CheckRequestUtils::setHttpRequest(
   const Buffer::Instance* buffer = sdfc->decodingBuffer();
   if (max_request_bytes > 0 && buffer != nullptr) {
     const uint64_t len = std::min(buffer->length(), max_request_bytes);
-    std::unique_ptr<char[]> data(new char[len]);
-    buffer->copyOut(0, len, data.get());
-    httpreq.set_body(data.get(), len);
+    // std::unique_ptr<char[]> data(new char[len]);
+    std::string data;
+    data.reserve(len);
+    buffer->copyOut(0, len, &data[0]);
+    httpreq.set_body(std::move(data));
+    // httpreq.set_body(data.get(), len);
   }
 }
 
diff --git a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc
index 45a22607ad294..3d7760ece43d6 100644
--- a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc
+++ b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc
@@ -161,7 +161,7 @@ void RawHttpClientImpl::check(RequestCallbacks& callbacks,
   ASSERT(callbacks_ == nullptr);
   callbacks_ = &callbacks;
 
-  Http::HeaderMapPtr headers{};
+  Http::HeaderMapPtr headers;
   const uint64_t request_length = request.attributes().request().http().body().size();
   if (request_length > 0) {
     headers =
diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc
index 0463ca5eaacc3..fed83d6f60b9e 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.cc
+++ b/source/extensions/filters/http/ext_authz/ext_authz.cc
@@ -23,8 +23,7 @@ bool isHeaderOnlyMethod(absl::string_view method) {
        Http::Headers::get().MethodValues.Connect, Http::Headers::get().MethodValues.Options,
        Http::Headers::get().MethodValues.Trace});
 
-  return std::any_of(keys->begin(), keys->end(),
-                     [&method](const auto& key) { return key == method; });
+  return keys->find(method) != keys->end();
 }
 } // namespace
 
@@ -106,10 +105,7 @@ Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) {
     if (!end_stream && !isBufferFull()) {
       return Http::FilterDataStatus::StopIterationAndBuffer;
     }
-
-    // Call the authorization service.
-    ENVOY_STREAM_LOG(debug, "ext_authz finished buffering", *callbacks_);
-    initiateCall(*request_headers_);
+    return Http::FilterDataStatus::StopIterationAndWatermark;
   }
 
   return filter_return_ == FilterReturn::StopDecoding
@@ -118,6 +114,12 @@ Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) {
 }
 
 Http::FilterTrailersStatus Filter::decodeTrailers(Http::HeaderMap&) {
+  if (buffer_data_) {
+    // Call the authorization service.
+    ENVOY_STREAM_LOG(debug, "ext_authz finished buffering", *callbacks_);
+    initiateCall(*request_headers_);
+  }
+
   return filter_return_ == FilterReturn::StopDecoding ? Http::FilterTrailersStatus::StopIteration
                                                       : Http::FilterTrailersStatus::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 492caa291e942..59b8ad285b6e1 100644
--- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc
+++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc
@@ -326,7 +326,41 @@ TEST_F(HttpFilterTest, RequestDataWithPartialMessage) {
   EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false));
 
   data_.add("barfoo");
-  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, false));
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true));
+
+  EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_));
+}
+
+// Checks that the filter initiates the authorization process only when the filter decode trailers
+// is called.
+TEST_F(HttpFilterTest, RequestDataWithSmallBuffer) {
+  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_));
+  EXPECT_CALL(*client_, check(_, _, _)).Times(1);
+
+  EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(request_headers_, false));
+
+  data_.add("foo");
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false));
+
+  EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_));
 }
 
 // Checks that the filter buffers the data and initiates the authorization request.
@@ -360,7 +394,7 @@ TEST_F(HttpFilterTest, AuthWithRequestData) {
             filter_->decodeHeaders(request_headers_, false));
   Buffer::OwnedImpl buffer("foo");
   EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(buffer, false));
-  EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, true));
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true));
   EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_));
   EXPECT_EQ(1U, filter_callbacks_.clusterInfo()->statsScope().counter("ext_authz.ok").value());
 }

From 2c9145ec9fdb6ec959733d6f806d0cd66dc66c3d Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Thu, 14 Mar 2019 00:05:49 -0700
Subject: [PATCH 10/17] fixed segfault, added logic for moving the request data

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../common/ext_authz/check_request_utils.cc   | 22 ++++--
 .../ext_authz/check_request_utils_test.cc     | 73 +++++++++++++++----
 2 files changed, 76 insertions(+), 19 deletions(-)

diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.cc b/source/extensions/filters/common/ext_authz/check_request_utils.cc
index 7431a3e44efd9..94b865799f457 100644
--- a/source/extensions/filters/common/ext_authz/check_request_utils.cc
+++ b/source/extensions/filters/common/ext_authz/check_request_utils.cc
@@ -10,6 +10,7 @@
 #include "common/buffer/buffer_impl.h"
 #include "common/common/assert.h"
 #include "common/common/enum_to_int.h"
+#include "common/common/stack_array.h"
 #include "common/grpc/async_client_impl.h"
 #include "common/http/codes.h"
 #include "common/http/headers.h"
@@ -118,13 +119,24 @@ void CheckRequestUtils::setHttpRequest(
   // Set request body.
   const Buffer::Instance* buffer = sdfc->decodingBuffer();
   if (max_request_bytes > 0 && buffer != nullptr) {
-    const uint64_t len = std::min(buffer->length(), max_request_bytes);
-    // std::unique_ptr<char[]> data(new char[len]);
+    const uint64_t length = std::min(buffer->length(), max_request_bytes);
     std::string data;
-    data.reserve(len);
-    buffer->copyOut(0, len, &data[0]);
+    data.reserve(length);
+
+    const uint64_t num_slices = buffer->getRawSlices(nullptr, 0);
+    STACK_ARRAY(slices, Buffer::RawSlice, num_slices);
+    buffer->getRawSlices(slices.begin(), num_slices);
+
+    for (const Buffer::RawSlice& slice : slices) {
+      if (slice.len_ + data.size() <= length) {
+        data.append(static_cast<char*>(slice.mem_), slice.len_);
+      } else {
+        data.append(static_cast<char*>(slice.mem_), length - data.size());
+        break;
+      }
+    }
+
     httpreq.set_body(std::move(data));
-    // httpreq.set_body(data.get(), len);
   }
 }
 
diff --git a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
index 625674dd22154..1ff1268fe7406 100644
--- a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
+++ b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc
@@ -27,9 +27,30 @@ class CheckRequestUtilsTest : public testing::Test {
   CheckRequestUtilsTest() {
     addr_ = std::make_shared<Network::Address::Ipv4Instance>("1.2.3.4", 1111);
     protocol_ = Envoy::Http::Protocol::Http10;
-    buffer_ = std::make_unique<Buffer::OwnedImpl>("foobar");
+    buffer_ = CheckRequestUtilsTest::newTestBuffer(8192);
   };
 
+  void ExpectBasicHttp() {
+    EXPECT_CALL(callbacks_, connection()).Times(2).WillRepeatedly(Return(&connection_));
+    EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_));
+    EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_));
+    EXPECT_CALL(Const(connection_), ssl()).Times(2).WillRepeatedly(Return(&ssl_));
+    EXPECT_CALL(callbacks_, streamId()).Times(1).WillOnce(Return(0));
+    EXPECT_CALL(callbacks_, decodingBuffer()).WillOnce(Return(buffer_.get()));
+    EXPECT_CALL(callbacks_, streamInfo()).Times(3).WillRepeatedly(ReturnRef(req_info_));
+    EXPECT_CALL(req_info_, protocol()).Times(2).WillRepeatedly(ReturnPointee(&protocol_));
+  }
+
+  static Buffer::InstancePtr newTestBuffer(uint64_t size) {
+    auto buffer = std::make_unique<Buffer::OwnedImpl>();
+    while (buffer->length() < size) {
+      auto new_buffer =
+          Buffer::OwnedImpl("Lorem ipsum dolor sit amet, consectetuer adipiscing elit.");
+      buffer->add(new_buffer);
+    }
+    return std::move(buffer);
+  }
+
   Network::Address::InstanceConstSharedPtr addr_;
   absl::optional<Http::Protocol> protocol_;
   CheckRequestUtils check_request_generator_;
@@ -53,22 +74,46 @@ TEST_F(CheckRequestUtilsTest, BasicTcp) {
 }
 
 // Verify that createHttpCheck's dependencies are invoked when it's called.
+// Verify that check request object has no request data.
 TEST_F(CheckRequestUtilsTest, BasicHttp) {
-  Http::HeaderMapImpl headers;
-  envoy::service::auth::v2::CheckRequest request;
-  EXPECT_CALL(callbacks_, connection()).Times(2).WillRepeatedly(Return(&connection_));
-  EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_));
-  EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_));
-  EXPECT_CALL(Const(connection_), ssl()).Times(2).WillRepeatedly(Return(&ssl_));
-  EXPECT_CALL(callbacks_, streamId()).Times(1).WillOnce(Return(0));
-  EXPECT_CALL(callbacks_, decodingBuffer()).WillOnce(Return(buffer_.get()));
-  EXPECT_CALL(callbacks_, streamInfo()).Times(3).WillRepeatedly(ReturnRef(req_info_));
-  EXPECT_CALL(req_info_, protocol()).Times(2).WillRepeatedly(ReturnPointee(&protocol_));
-  Protobuf::Map<ProtobufTypes::String, ProtobufTypes::String> empty;
+  const uint64_t size = 0;
+  Http::HeaderMapImpl headers_;
+  envoy::service::auth::v2::CheckRequest request_;
+
+  ExpectBasicHttp();
+  CheckRequestUtils::createHttpCheck(&callbacks_, headers_,
+                                     Protobuf::Map<ProtobufTypes::String, ProtobufTypes::String>(),
+                                     request_, size);
+  ASSERT_EQ(size, request_.attributes().request().http().body().size());
+  EXPECT_EQ(buffer_->toString().substr(0, size), request_.attributes().request().http().body());
+}
 
-  CheckRequestUtils::createHttpCheck(&callbacks_, headers, std::move(empty), request, 4);
+// Verify that check request object has only a portion of the request data.
+TEST_F(CheckRequestUtilsTest, BasicHttpWithPartialBody) {
+  const uint64_t size = 4049;
+  Http::HeaderMapImpl headers_;
+  envoy::service::auth::v2::CheckRequest request_;
+
+  ExpectBasicHttp();
+  CheckRequestUtils::createHttpCheck(&callbacks_, headers_,
+                                     Protobuf::Map<ProtobufTypes::String, ProtobufTypes::String>(),
+                                     request_, size);
+  ASSERT_EQ(size, request_.attributes().request().http().body().size());
+  EXPECT_EQ(buffer_->toString().substr(0, size), request_.attributes().request().http().body());
+}
 
-  EXPECT_EQ("foob", request.attributes().request().http().body());
+// Verify that check request object has all the request data.
+TEST_F(CheckRequestUtilsTest, BasicHttpWithFullBody) {
+  Http::HeaderMapImpl headers_;
+  envoy::service::auth::v2::CheckRequest request_;
+
+  ExpectBasicHttp();
+  CheckRequestUtils::createHttpCheck(&callbacks_, headers_,
+                                     Protobuf::Map<ProtobufTypes::String, ProtobufTypes::String>(),
+                                     request_, buffer_->length());
+  ASSERT_EQ(buffer_->length(), request_.attributes().request().http().body().size());
+  EXPECT_EQ(buffer_->toString().substr(0, buffer_->length()),
+            request_.attributes().request().http().body());
 }
 
 // Verify that createHttpCheck extract the proper attributes from the http request into CheckRequest

From d60cd55beb677bcf031c9176f807862bb4cd8b1c Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Fri, 15 Mar 2019 19:05:29 -0700
Subject: [PATCH 11/17] removed the logic that copy the buffer slices

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../common/ext_authz/check_request_utils.cc   | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.cc b/source/extensions/filters/common/ext_authz/check_request_utils.cc
index 94b865799f457..b1ef8841adb11 100644
--- a/source/extensions/filters/common/ext_authz/check_request_utils.cc
+++ b/source/extensions/filters/common/ext_authz/check_request_utils.cc
@@ -10,7 +10,6 @@
 #include "common/buffer/buffer_impl.h"
 #include "common/common/assert.h"
 #include "common/common/enum_to_int.h"
-#include "common/common/stack_array.h"
 #include "common/grpc/async_client_impl.h"
 #include "common/http/codes.h"
 #include "common/http/headers.h"
@@ -119,23 +118,9 @@ void CheckRequestUtils::setHttpRequest(
   // Set request body.
   const Buffer::Instance* buffer = sdfc->decodingBuffer();
   if (max_request_bytes > 0 && buffer != nullptr) {
-    const uint64_t length = std::min(buffer->length(), max_request_bytes);
     std::string data;
-    data.reserve(length);
-
-    const uint64_t num_slices = buffer->getRawSlices(nullptr, 0);
-    STACK_ARRAY(slices, Buffer::RawSlice, num_slices);
-    buffer->getRawSlices(slices.begin(), num_slices);
-
-    for (const Buffer::RawSlice& slice : slices) {
-      if (slice.len_ + data.size() <= length) {
-        data.append(static_cast<char*>(slice.mem_), slice.len_);
-      } else {
-        data.append(static_cast<char*>(slice.mem_), length - data.size());
-        break;
-      }
-    }
-
+    data.resize(std::min(buffer->length(), max_request_bytes));
+    buffer->copyOut(0, data.size(), &data[0]);
     httpreq.set_body(std::move(data));
   }
 }

From 8939e7dbb475a0c20bb4f813fd78fbfc0903a2e9 Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Fri, 15 Mar 2019 19:47:12 -0700
Subject: [PATCH 12/17] changed the string initialization to the correct size

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../filters/common/ext_authz/check_request_utils.cc         | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.cc b/source/extensions/filters/common/ext_authz/check_request_utils.cc
index b1ef8841adb11..5496acc8c0832 100644
--- a/source/extensions/filters/common/ext_authz/check_request_utils.cc
+++ b/source/extensions/filters/common/ext_authz/check_request_utils.cc
@@ -118,9 +118,9 @@ void CheckRequestUtils::setHttpRequest(
   // Set request body.
   const Buffer::Instance* buffer = sdfc->decodingBuffer();
   if (max_request_bytes > 0 && buffer != nullptr) {
-    std::string data;
-    data.resize(std::min(buffer->length(), max_request_bytes));
-    buffer->copyOut(0, data.size(), &data[0]);
+    const uint64_t length = std::min(buffer->length(), max_request_bytes);
+    std::string data(length, 0);
+    buffer->copyOut(0, length, &data[0]);
     httpreq.set_body(std::move(data));
   }
 }

From 075d6190fb42190ff6334e3b0974c6afc3b05362 Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Thu, 21 Mar 2019 21:02:50 -0700
Subject: [PATCH 13/17] code review changes

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 docs/root/intro/version_history.rst | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
index 6e1e13cf053e6..305b3bf853f41 100644
--- a/docs/root/intro/version_history.rst
+++ b/docs/root/intro/version_history.rst
@@ -21,10 +21,9 @@ Version history
 * config: added support for :ref:`initial_fetch_timeout <envoy_api_field_core.ConfigSource.initial_fetch_timeout>`. The timeout is disabled by default.
 * cors: added :ref:`filter_enabled & shadow_enabled RuntimeFractionalPercent flags <cors-runtime>` to filter.
 * ext_authz: added support for buffering request body.
-* ext_authz: migrated from V2alpha to V2 and improved docs.
-* ext_authz: added an configurable option to make the gRPC service cross-compatible with V2Alpha. Note that this feature is already deprecated. It should be used for a short time, and only when transitioning from alpha to V2 release version. 
-* ext_authz: added an configurable option to make the gRPC service cross-compatible with V2Alpha. Note that this feature is already deprecated. It should be used for a short time, and only when transitioning from alpha to V2 release version.
-* ext_authz: migrated from V2alpha to V2 and improved the documentation.
+* ext_authz: migrated from v2alpha to v2 and improved docs.
+* ext_authz: added a configurable option to make the gRPC service cross-compatible with V2Alpha. Note that this feature is already deprecated. It should be used for a short time, and only when transitioning from alpha to V2 release version.
+* ext_authz: migrated from v2alpha to v2 and improved the documentation.
 * ext_authz: authorization request and response configuration has been separated into two distinct objects: :ref:`authorization request
   <envoy_api_field_config.filter.http.ext_authz.v2.HttpService.authorization_request>` and :ref:`authorization response
   <envoy_api_field_config.filter.http.ext_authz.v2.HttpService.authorization_response>`. In addition, :ref:`client headers

From 7fc5814ee120df5bb92df73f799203aaaa93435b Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Tue, 26 Mar 2019 20:30:06 -0700
Subject: [PATCH 14/17] replaced logic to inspect headers only reuquests

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../extensions/filters/http/ext_authz/BUILD   |  1 +
 .../filters/http/ext_authz/ext_authz.cc       | 34 ++++++-------------
 2 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/source/extensions/filters/http/ext_authz/BUILD b/source/extensions/filters/http/ext_authz/BUILD
index 423a8b51b338d..f5b4e7eacea12 100644
--- a/source/extensions/filters/http/ext_authz/BUILD
+++ b/source/extensions/filters/http/ext_authz/BUILD
@@ -24,6 +24,7 @@ envoy_cc_library(
         "//source/common/common:matchers_lib",
         "//source/common/common:minimal_logger_lib",
         "//source/common/http:codes_lib",
+        "//source/common/http:utility_lib",
         "//source/common/router:config_lib",
         "//source/extensions/filters/common/ext_authz:ext_authz_grpc_lib",
         "//source/extensions/filters/common/ext_authz:ext_authz_http_lib",
diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc
index fed83d6f60b9e..bd4c01b3fb786 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.cc
+++ b/source/extensions/filters/http/ext_authz/ext_authz.cc
@@ -3,6 +3,7 @@
 #include "common/common/assert.h"
 #include "common/common/enum_to_int.h"
 #include "common/http/codes.h"
+#include "common/http/utility.h"
 #include "common/router/config_impl.h"
 
 #include "extensions/filters/http/well_known_names.h"
@@ -14,19 +15,6 @@ namespace Extensions {
 namespace HttpFilters {
 namespace ExtAuthz {
 
-namespace {
-// Asserts that the HTTP method is for a header-only request. For details
-// https://tools.ietf.org/html/rfc7231#section-4.3.
-bool isHeaderOnlyMethod(absl::string_view method) {
-  const static absl::flat_hash_set<std::string>* keys = new absl::flat_hash_set<std::string>(
-      {Http::Headers::get().MethodValues.Get, Http::Headers::get().MethodValues.Head,
-       Http::Headers::get().MethodValues.Connect, Http::Headers::get().MethodValues.Options,
-       Http::Headers::get().MethodValues.Trace});
-
-  return keys->find(method) != keys->end();
-}
-} // namespace
-
 void FilterConfigPerRoute::merge(const FilterConfigPerRoute& other) {
   disabled_ = other.disabled_;
   auto begin_it = other.context_extensions_.begin();
@@ -82,11 +70,9 @@ void Filter::initiateCall(const Http::HeaderMap& headers) {
 }
 
 Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool end_stream) {
-  request_headers_ = &headers;
-  const auto* method = headers.Method();
   buffer_data_ = config_->withRequestBody() &&
-                 !(end_stream || (method && isHeaderOnlyMethod(method->value().getStringView())));
-
+                 !(end_stream || Http::Utility::isWebSocketUpgradeRequest(headers));
+  request_headers_ = &headers;
   if (buffer_data_) {
     ENVOY_STREAM_LOG(debug, "ext_authz is buffering", *callbacks_);
     if (!config_->allowPartialMessage()) {
@@ -102,10 +88,12 @@ 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()) {
+    if (end_stream || isBufferFull()) {
+      ENVOY_STREAM_LOG(debug, "ext_authz finished buffering", *callbacks_);
+      initiateCall(*request_headers_);
+    } else {
       return Http::FilterDataStatus::StopIterationAndBuffer;
     }
-    return Http::FilterDataStatus::StopIterationAndWatermark;
   }
 
   return filter_return_ == FilterReturn::StopDecoding
@@ -114,8 +102,7 @@ Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) {
 }
 
 Http::FilterTrailersStatus Filter::decodeTrailers(Http::HeaderMap&) {
-  if (buffer_data_) {
-    // Call the authorization service.
+  if (buffer_data_ && filter_return_ != FilterReturn::StopDecoding) {
     ENVOY_STREAM_LOG(debug, "ext_authz finished buffering", *callbacks_);
     initiateCall(*request_headers_);
   }
@@ -221,8 +208,9 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) {
 }
 
 bool Filter::isBufferFull() {
-  if (config_->allowPartialMessage()) {
-    return callbacks_->decodingBuffer()->length() >= config_->maxRequestBytes();
+  const auto* buffer = callbacks_->decodingBuffer();
+  if (config_->allowPartialMessage() && buffer != nullptr) {
+    return buffer->length() >= config_->maxRequestBytes();
   }
   return false;
 }

From 9ea7dab5e662f48ac3cd184821186fdb15d1499e Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Wed, 27 Mar 2019 19:37:01 -0700
Subject: [PATCH 15/17] added tests for upgrade requests

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../filters/http/ext_authz/ext_authz.cc       |   3 +-
 .../filters/http/ext_authz/ext_authz_test.cc  | 136 +++++++++++-------
 2 files changed, 86 insertions(+), 53 deletions(-)

diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc
index bd4c01b3fb786..edf4d79f4b945 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.cc
+++ b/source/extensions/filters/http/ext_authz/ext_authz.cc
@@ -71,7 +71,8 @@ void Filter::initiateCall(const Http::HeaderMap& headers) {
 
 Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool end_stream) {
   buffer_data_ = config_->withRequestBody() &&
-                 !(end_stream || Http::Utility::isWebSocketUpgradeRequest(headers));
+                 !(end_stream || Http::Utility::isWebSocketUpgradeRequest(headers) ||
+                   Http::Utility::isH2UpgradeRequest(headers));
   request_headers_ = &headers;
   if (buffer_data_) {
     ENVOY_STREAM_LOG(debug, "ext_authz is buffering", *callbacks_);
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 59b8ad285b6e1..64a2fbeffda0d 100644
--- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc
+++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc
@@ -21,6 +21,7 @@
 #include "test/mocks/http/mocks.h"
 #include "test/mocks/local_info/mocks.h"
 #include "test/mocks/network/mocks.h"
+#include "test/mocks/router/mocks.h"
 #include "test/mocks/runtime/mocks.h"
 #include "test/mocks/tracing/mocks.h"
 #include "test/mocks/upstream/mocks.h"
@@ -64,8 +65,8 @@ template <class T> class HttpFilterTestBase : public T {
   Filters::Common::ExtAuthz::MockClient* client_;
   std::unique_ptr<Filter> filter_;
   NiceMock<Http::MockStreamDecoderFilterCallbacks> filter_callbacks_;
-  Filters::Common::ExtAuthz::RequestCallbacks* request_callbacks_{};
-  Http::TestHeaderMapImpl request_headers_{};
+  Filters::Common::ExtAuthz::RequestCallbacks* request_callbacks_;
+  Http::TestHeaderMapImpl request_headers_;
   Buffer::OwnedImpl data_;
   Stats::IsolatedStoreImpl stats_store_;
   NiceMock<Runtime::MockLoader> runtime_;
@@ -327,7 +328,6 @@ TEST_F(HttpFilterTest, RequestDataWithPartialMessage) {
 
   data_.add("barfoo");
   EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true));
-
   EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_));
 }
 
@@ -348,7 +348,6 @@ TEST_F(HttpFilterTest, RequestDataWithSmallBuffer) {
 
   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_));
@@ -359,7 +358,6 @@ TEST_F(HttpFilterTest, RequestDataWithSmallBuffer) {
 
   data_.add("foo");
   EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false));
-
   EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_));
 }
 
@@ -371,65 +369,108 @@ TEST_F(HttpFilterTest, AuthWithRequestData) {
   grpc_service:
     envoy_grpc:
       cluster_name: "ext_authz_server"
-  failure_mode_allow: false
   with_request_body:
     max_request_bytes: 10
   )EOF");
 
-  ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_));
-  EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(1);
-  EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_));
-  EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_));
-
-  Filters::Common::ExtAuthz::Response response{};
-  response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
+  prepareCheck();
 
-  EXPECT_CALL(*client_, check(_, _, _))
+  EXPECT_CALL(*client_, check(_, _, testing::A<Tracing::Span&>()))
       .WillOnce(
           WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void {
-            callbacks.onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
+            request_callbacks_ = &callbacks;
           })));
-  EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0);
   EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
             filter_->decodeHeaders(request_headers_, false));
-  Buffer::OwnedImpl buffer("foo");
-  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(buffer, false));
+  data_.add("foo");
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false));
+  data_.add("bar");
   EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true));
-  EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_));
-  EXPECT_EQ(1U, filter_callbacks_.clusterInfo()->statsScope().counter("ext_authz.ok").value());
+  EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_));
 }
 
 // Checks that filter does not buffer data on header-only request.
-TEST_F(HttpFilterTest, AuthHeaderOnlyRequest) {
+TEST_F(HttpFilterTest, HeaderOnlyRequest) {
   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
   )EOF");
 
-  ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_));
-  EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(0);
-  EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_));
-  EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_));
+  prepareCheck();
 
-  Filters::Common::ExtAuthz::Response response{};
-  response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
+  EXPECT_CALL(*client_, check(_, _, testing::A<Tracing::Span&>()))
+      .WillOnce(
+          WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void {
+            request_callbacks_ = &callbacks;
+          })));
+  EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(request_headers_, true));
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, true));
+  EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_));
+}
 
-  EXPECT_CALL(*client_, check(_, _, _))
+// Checks that filter does not buffer data on upgrade WebSocket request.
+TEST_F(HttpFilterTest, UpgradeWebsocketRequest) {
+  InSequence s;
+
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_authz_server"
+  with_request_body:
+    max_request_bytes: 10
+  )EOF");
+
+  prepareCheck();
+
+  request_headers_.addCopy(Http::Headers::get().Connection,
+                           Http::Headers::get().ConnectionValues.Upgrade);
+  request_headers_.addCopy(Http::Headers::get().Upgrade,
+                           Http::Headers::get().UpgradeValues.WebSocket);
+
+  EXPECT_CALL(*client_, check(_, _, testing::A<Tracing::Span&>()))
       .WillOnce(
           WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void {
-            callbacks.onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
+            request_callbacks_ = &callbacks;
           })));
-  EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0);
-  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, true));
-  EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, true));
-  EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_));
-  EXPECT_EQ(1U, filter_callbacks_.clusterInfo()->statsScope().counter("ext_authz.ok").value());
+  EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(request_headers_, false));
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, false));
+  EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_));
+}
+
+// Checks that filter does not buffer data on upgrade H2 WebSocket request.
+TEST_F(HttpFilterTest, H2UpgradeRequest) {
+  InSequence s;
+
+  initialize(R"EOF(
+  grpc_service:
+    envoy_grpc:
+      cluster_name: "ext_authz_server"
+  with_request_body:
+    max_request_bytes: 10
+  )EOF");
+
+  prepareCheck();
+
+  request_headers_.addCopy(Http::Headers::get().Method, Http::Headers::get().MethodValues.Connect);
+  request_headers_.addCopy(Http::Headers::get().Protocol,
+                           Http::Headers::get().ProtocolStrings.Http2String);
+
+  EXPECT_CALL(*client_, check(_, _, testing::A<Tracing::Span&>()))
+      .WillOnce(
+          WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void {
+            request_callbacks_ = &callbacks;
+          })));
+  EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(request_headers_, false));
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndWatermark, filter_->decodeData(data_, false));
+  EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_));
 }
 
 // Checks that filter does not buffer data when is not the end of the stream, but header-only
@@ -441,31 +482,22 @@ TEST_F(HttpFilterTest, HeaderOnlyRequestWithStream) {
   grpc_service:
     envoy_grpc:
       cluster_name: "ext_authz_server"
-  failure_mode_allow: false
   with_request_body:
     max_request_bytes: 10
   )EOF");
 
-  request_headers_.addCopy(":method", "GET");
-
-  ON_CALL(filter_callbacks_, connection()).WillByDefault(Return(&connection_));
-  EXPECT_CALL(filter_callbacks_, setDecoderBufferLimit(_)).Times(0);
-  EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_));
-  EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_));
-
-  Filters::Common::ExtAuthz::Response response{};
-  response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
+  prepareCheck();
 
-  EXPECT_CALL(*client_, check(_, _, _))
+  EXPECT_CALL(*client_, check(_, _, testing::A<Tracing::Span&>()))
       .WillOnce(
           WithArgs<0>(Invoke([&](Filters::Common::ExtAuthz::RequestCallbacks& callbacks) -> void {
-            callbacks.onComplete(std::make_unique<Filters::Common::ExtAuthz::Response>(response));
+            request_callbacks_ = &callbacks;
           })));
-  EXPECT_CALL(filter_callbacks_, continueDecoding()).Times(0);
-  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false));
-  EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, true));
-  EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_headers_));
-  EXPECT_EQ(1U, filter_callbacks_.clusterInfo()->statsScope().counter("ext_authz.ok").value());
+
+  EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
+            filter_->decodeHeaders(request_headers_, false));
+  EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_->decodeData(data_, false));
+  EXPECT_EQ(Http::FilterTrailersStatus::StopIteration, filter_->decodeTrailers(request_headers_));
 }
 
 // -------------------

From 426dffc06eb1e7e1e32aa20a1fb6aaaab7df9d81 Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Wed, 27 Mar 2019 20:56:24 -0700
Subject: [PATCH 16/17] improved some debug and trace log messages

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 .../filters/http/ext_authz/ext_authz.cc       | 47 ++++++++++---------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc
index edf4d79f4b945..c4db29a4411ff 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.cc
+++ b/source/extensions/filters/http/ext_authz/ext_authz.cc
@@ -60,22 +60,22 @@ void Filter::initiateCall(const Http::HeaderMap& headers) {
       callbacks_, headers, std::move(context_extensions), check_request_,
       config_->maxRequestBytes());
 
+  ENVOY_STREAM_LOG(trace, "ext_authz filter calling authorization server", *callbacks_);
   state_ = State::Calling;
-  // Don't let the filter chain continue as we are going to invoke check call.
-  filter_return_ = FilterReturn::StopDecoding;
+  filter_return_ = FilterReturn::StopDecoding; // Don't let the filter chain continue as we are
+                                               // going to invoke check call.
   initiating_call_ = true;
-  ENVOY_STREAM_LOG(trace, "ext_authz filter calling authorization server", *callbacks_);
   client_->check(*this, check_request_, callbacks_->activeSpan());
   initiating_call_ = false;
 }
 
 Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool end_stream) {
+  request_headers_ = &headers;
   buffer_data_ = config_->withRequestBody() &&
                  !(end_stream || Http::Utility::isWebSocketUpgradeRequest(headers) ||
                    Http::Utility::isH2UpgradeRequest(headers));
-  request_headers_ = &headers;
   if (buffer_data_) {
-    ENVOY_STREAM_LOG(debug, "ext_authz is buffering", *callbacks_);
+    ENVOY_STREAM_LOG(debug, "ext_authz filter is buffering the request", *callbacks_);
     if (!config_->allowPartialMessage()) {
       callbacks_->setDecoderBufferLimit(config_->maxRequestBytes());
     }
@@ -90,7 +90,7 @@ 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 finished buffering", *callbacks_);
+      ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request", *callbacks_);
       initiateCall(*request_headers_);
     } else {
       return Http::FilterDataStatus::StopIterationAndBuffer;
@@ -104,7 +104,7 @@ Http::FilterDataStatus Filter::decodeData(Buffer::Instance&, bool end_stream) {
 
 Http::FilterTrailersStatus Filter::decodeTrailers(Http::HeaderMap&) {
   if (buffer_data_ && filter_return_ != FilterReturn::StopDecoding) {
-    ENVOY_STREAM_LOG(debug, "ext_authz finished buffering", *callbacks_);
+    ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request", *callbacks_);
     initiateCall(*request_headers_);
   }
 
@@ -151,29 +151,30 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) {
     break;
   }
 
-  ENVOY_STREAM_LOG(trace, "ext_authz received status code {}", *callbacks_,
+  ENVOY_STREAM_LOG(trace, "ext_authz filter received status code {}", *callbacks_,
                    enumToInt(response->status_code));
 
   // We fail open/fail close based of filter config
   // if there is an error contacting the service.
   if (response->status == CheckStatus::Denied ||
       (response->status == CheckStatus::Error && !config_->failureModeAllow())) {
-    ENVOY_STREAM_LOG(debug, "ext_authz rejected the request", *callbacks_);
-    ENVOY_STREAM_LOG(trace, "ext_authz downstream header(s):", *callbacks_);
-    callbacks_->sendLocalReply(response->status_code, response->body,
-                               [& headers = response->headers_to_add, &callbacks = *callbacks_](
-                                   Http::HeaderMap& response_headers) -> void {
-                                 for (const auto& header : headers) {
-                                   response_headers.remove(header.first);
-                                   response_headers.addCopy(header.first, header.second);
-                                   ENVOY_STREAM_LOG(trace, " '{}':'{}'", callbacks,
-                                                    header.first.get(), header.second);
-                                 }
-                               },
-                               absl::nullopt);
+    ENVOY_STREAM_LOG(debug, "ext_authz filter rejected the request", *callbacks_);
+    callbacks_->sendLocalReply(
+        response->status_code, response->body,
+        [& headers = response->headers_to_add,
+         &callbacks = *callbacks_](Http::HeaderMap& response_headers) -> void {
+          ENVOY_STREAM_LOG(trace,
+                           "ext_authz filter added header(s) to the local response:", callbacks);
+          for (const auto& header : headers) {
+            response_headers.remove(header.first);
+            response_headers.addCopy(header.first, header.second);
+            ENVOY_STREAM_LOG(trace, " '{}':'{}'", callbacks, header.first.get(), header.second);
+          }
+        },
+        absl::nullopt);
     callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::UnauthorizedExternalService);
   } else {
-    ENVOY_STREAM_LOG(debug, "ext_authz accepted the request", *callbacks_);
+    ENVOY_STREAM_LOG(debug, "ext_authz filter accepted the request", *callbacks_);
     // Let the filter chain continue.
     filter_return_ = FilterReturn::ContinueDecoding;
     if (config_->failureModeAllow() && response->status == CheckStatus::Error) {
@@ -182,7 +183,7 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) {
     }
     // Only send headers if the response is ok.
     if (response->status == CheckStatus::OK) {
-      ENVOY_STREAM_LOG(trace, "ext_authz upstream header(s):", *callbacks_);
+      ENVOY_STREAM_LOG(trace, "ext_authz filter added header(s) to the request:", *callbacks_);
       for (const auto& header : response->headers_to_add) {
         Http::HeaderEntry* header_to_modify = request_headers_->get(header.first);
         if (header_to_modify) {

From 2bf9abd0da4eb746e652ef54d0002cac95d47a1a Mon Sep 17 00:00:00 2001
From: Gabriel <gsagula@gmail.com>
Date: Wed, 27 Mar 2019 23:18:14 -0700
Subject: [PATCH 17/17] removed unused header

Signed-off-by: Gabriel <gsagula@gmail.com>
---
 source/extensions/filters/http/ext_authz/ext_authz.cc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/source/extensions/filters/http/ext_authz/ext_authz.cc b/source/extensions/filters/http/ext_authz/ext_authz.cc
index c4db29a4411ff..6a719bac889c3 100644
--- a/source/extensions/filters/http/ext_authz/ext_authz.cc
+++ b/source/extensions/filters/http/ext_authz/ext_authz.cc
@@ -8,8 +8,6 @@
 
 #include "extensions/filters/http/well_known_names.h"
 
-#include "absl/container/flat_hash_set.h"
-
 namespace Envoy {
 namespace Extensions {
 namespace HttpFilters {