http:: sending http/1.1 errors via HCM

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"
@@ -177,6 +178,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"
@@ -969,7 +970,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);
   }
@@ -987,6 +988,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

@@ -246,6 +246,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_;
@@ -324,7 +325,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;
 
   /**
@@ -499,6 +499,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",