http:: sending http/1.1 errors via HCM (envoyproxy#11714)

Solving the problem where early response errors look different, and don't have HCM tracking, by sending them through the hcm.
This means early responses will get all of the usual Envoy standard header additions, get accounted for in listener stats, be visible in access logs, etc.

Risk Level: high (changes to HTTP early response path)
Testing: new unit tests
Docs Changes: n/a
Release Notes: inline
Runtime guard: envoy.reloadable_features.early_errors_via_hcm
Fixes envoyproxy#8545
Part of envoyproxy#9846

Signed-off-by: Alyssa Wilk <[email protected]>

docs/root/version_history/current.rst

@@ -16,6 +16,7 @@ Minor Behavior Changes
 * build: run as non-root inside Docker containers. Existing behaviour can be restored by setting the environment variable `ENVOY_UID` to `0`. `ENVOY_UID` and `ENVOY_GID` can be used to set the envoy user's `uid` and `gid` respectively.
 * health check: in the health check filter the :ref:`percentage of healthy servers in upstream clusters <envoy_api_field_config.filter.http.health_check.v2.HealthCheck.cluster_min_healthy_percentages>` is now interpreted as an integer.
 * hot restart: added the option :option:`--use-dynamic-base-id` to select an unused base ID at startup and the option :option:`--base-id-path` to write the base id to a file (for reuse with later hot restarts).
+* http: changed early error path for HTTP/1.1 so that responses consistently flow through the http connection manager, and the http filter chains. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.early_errors_via_hcm` to false.
 * http: fixed several bugs with applying correct connection close behavior across the http connection manager, health checker, and connection pool. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_connection_close` to false.
 * http: fixed a bug where the upgrade header was not cleared on responses to non-upgrade requests.
   Can be reverted temporarily by setting runtime feature `envoy.reloadable_features.fix_upgrade_response` to false.

include/envoy/http/BUILD

@@ -35,6 +35,7 @@ envoy_cc_library(
         ":metadata_interface",
         ":protocol_interface",
         "//include/envoy/buffer:buffer_interface",
+        "//include/envoy/grpc:status",
         "//include/envoy/network:address_interface",
         "//source/common/http:status_lib",
     ],

include/envoy/http/codec.h

@@ -6,6 +6,7 @@
 
 #include "envoy/buffer/buffer.h"
 #include "envoy/common/pure.h"
+#include "envoy/grpc/status.h"
 #include "envoy/http/header_map.h"
 #include "envoy/http/metadata_interface.h"
 #include "envoy/http/protocol.h"
@@ -185,6 +186,22 @@ class RequestDecoder : public virtual StreamDecoder {
    * @param trailers supplies the decoded trailers.
    */
   virtual void decodeTrailers(RequestTrailerMapPtr&& trailers) PURE;
+
+  /**
+   * Called if the codec needs to send a protocol error.
+   * @param is_grpc_request indicates if the request is a gRPC request
+   * @param code supplies the HTTP error code to send.
+   * @param body supplies an optional body to send with the local reply.
+   * @param modify_headers supplies a way to edit headers before they are sent downstream.
+   * @param is_head_request indicates if the request is a HEAD request
+   * @param grpc_status an optional gRPC status for gRPC requests
+   * @param details details about the source of the error, for debug purposes
+   */
+  virtual void sendLocalReply(bool is_grpc_request, Code code, absl::string_view body,
+                              const std::function<void(ResponseHeaderMap& headers)>& modify_headers,
+                              bool is_head_request,
+                              const absl::optional<Grpc::Status::GrpcStatus> grpc_status,
+                              absl::string_view details) PURE;
 };
 
 /**

source/common/http/conn_manager_impl.h

@@ -495,7 +495,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
                         const std::function<void(ResponseHeaderMap& headers)>& modify_headers,
                         bool is_head_request,
                         const absl::optional<Grpc::Status::GrpcStatus> grpc_status,
-                        absl::string_view details);
+                        absl::string_view details) override;
     void encode100ContinueHeaders(ActiveStreamEncoderFilter* filter, ResponseHeaderMap& headers);
     // As with most of the encode functions, this runs encodeHeaders on various
     // filters before calling encodeHeadersInternal which does final header munging and passes the

source/common/http/http1/BUILD

@@ -32,6 +32,7 @@ envoy_cc_library(
         "//source/common/common:statusor_lib",
         "//source/common/common:thread_lib",
         "//source/common/common:utility_lib",
+        "//source/common/grpc:common_lib",
         "//source/common/http:codec_helper_lib",
         "//source/common/http:codes_lib",
         "//source/common/http:exception_lib",

source/common/http/http1/codec_impl.cc

@@ -11,6 +11,7 @@
 
 #include "common/common/enum_to_int.h"
 #include "common/common/utility.h"
+#include "common/grpc/common.h"
 #include "common/http/exception.h"
 #include "common/http/header_utility.h"
 #include "common/http/headers.h"
@@ -964,7 +965,7 @@ void ServerConnectionImpl::onResetStream(StreamResetReason reason) {
   active_request_.reset();
 }
 
-void ServerConnectionImpl::sendProtocolError(absl::string_view details) {
+void ServerConnectionImpl::sendProtocolErrorOld(absl::string_view details) {
   if (active_request_.has_value()) {
     active_request_.value().response_encoder_.setDetails(details);
   }
@@ -982,6 +983,35 @@ void ServerConnectionImpl::sendProtocolError(absl::string_view details) {
   }
 }
 
+void ServerConnectionImpl::sendProtocolError(absl::string_view details) {
+  if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.early_errors_via_hcm")) {
+    sendProtocolErrorOld(details);
+    return;
+  }
+  // We do this here because we may get a protocol error before we have a logical stream.
+  if (!active_request_.has_value()) {
+    onMessageBeginBase();
+  }
+  ASSERT(active_request_.has_value());
+
+  active_request_.value().response_encoder_.setDetails(details);
+  if (!active_request_.value().response_encoder_.startedResponse()) {
+    // Note that the correctness of is_grpc_request and is_head_request is best-effort.
+    // If headers have not been fully parsed they may not be inferred correctly.
+    bool is_grpc_request = false;
+    if (absl::holds_alternative<RequestHeaderMapPtr>(headers_or_trailers_) &&
+        absl::get<RequestHeaderMapPtr>(headers_or_trailers_) != nullptr) {
+      is_grpc_request =
+          Grpc::Common::isGrpcRequestHeaders(*absl::get<RequestHeaderMapPtr>(headers_or_trailers_));
+    }
+    const bool is_head_request = parser_.method == HTTP_HEAD;
+    active_request_->request_decoder_->sendLocalReply(is_grpc_request, error_code_,
+                                                      CodeUtility::toString(error_code_), nullptr,
+                                                      is_head_request, absl::nullopt, details);
+    return;
+  }
+}
+
 void ServerConnectionImpl::onAboveHighWatermark() {
   if (active_request_.has_value()) {
     active_request_.value().response_encoder_.runHighWatermarkCallbacks();

source/common/http/http1/codec_impl.h

@@ -242,6 +242,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
                  HeaderKeyFormatterPtr&& header_key_formatter, bool enable_trailers);
 
   bool resetStreamCalled() { return reset_stream_called_; }
+  void onMessageBeginBase();
 
   Network::Connection& connection_;
   CodecStats& stats_;
@@ -320,7 +321,6 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
    * Called when a request/response is beginning. A base routine happens first then a virtual
    * dispatch is invoked.
    */
-  void onMessageBeginBase();
   virtual void onMessageBegin() PURE;
 
   /**
@@ -495,6 +495,8 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
     }
   }
 
+  void sendProtocolErrorOld(absl::string_view details);
+
   void releaseOutboundResponse(const Buffer::OwnedBufferFragmentImpl* fragment);
   void maybeAddSentinelBufferFragment(Buffer::WatermarkBuffer& output_buffer) override;
   void doFloodProtectionChecks() const;

source/common/runtime/runtime_features.cc

@@ -62,6 +62,7 @@ constexpr const char* runtime_features[] = {
     // Begin alphabetically sorted section.
     "envoy.deprecated_features.allow_deprecated_extension_names",
     "envoy.reloadable_features.disallow_unbounded_access_logs",
+    "envoy.reloadable_features.early_errors_via_hcm",
     "envoy.reloadable_features.enable_deprecated_v2_api_warning",
     "envoy.reloadable_features.ext_authz_http_service_enable_case_sensitive_string_matcher",
     "envoy.reloadable_features.fix_upgrade_response",