http: stream error on invalid HTTP messaging for HTTP/1

api/envoy/config/core/v3/protocol.proto

@@ -100,7 +100,7 @@ message HttpProtocolOptions {
   HeadersWithUnderscoresAction headers_with_underscores_action = 5;
 }
 
-// [#next-free-field: 6]
+// [#next-free-field: 7]
 message Http1ProtocolOptions {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.api.v2.core.Http1ProtocolOptions";
@@ -157,6 +157,16 @@ message Http1ProtocolOptions {
   //   - Not a response to a HEAD request.
   //   - The content length header is not present.
   bool enable_trailers = 5;
+
+  // Allows invalid HTTP messaging. When this option is false, then Envoy will terminate
+  // HTTP/1.1 connections upon receiving an invalid HTTP message. However,
+  // when this option is true, then Envoy will leave the HTTP/1.1 connection
+  // open where possible.
+  // If set, this overrides any HCM :ref:`stream_error_on_invalid_http_messaging
+  // <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>`;
+  // this is overridden by any HCM :ref:`stream_error_on_invalid_http_messaging
+  // <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>`, which defaults to false.
+  google.protobuf.BoolValue override_stream_error_on_invalid_http_message = 6;
 }
 
 // [#next-free-field: 15]

api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto

@@ -552,7 +552,8 @@ message HttpConnectionManager {
   // company-internal mesh) and false when receiving untrusted traffic (edge deployments).
   //
   // If different behaviors for invalid_http_message for HTTP/1 and HTTP/2 are
-  // desired, one *must* use the new HTTP/2 option
+  // desired, one should use the new HTTP/1 option :ref:`override_stream_error_on_invalid_http_message
+  // <envoy_v3_api_field_config.core.v3.Http1ProtocolOptions.override_stream_error_on_invalid_http_message>` or the new HTTP/2 option
   // :ref:`override_stream_error_on_invalid_http_message
   // <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.override_stream_error_on_invalid_http_message>`
   // *not* the deprecated but similarly named :ref:`stream_error_on_invalid_http_messaging

docs/root/version_history/current.rst

@@ -17,7 +17,7 @@ Minor Behavior Changes
 * http: added :ref:`contains <envoy_api_msg_type.matcher.StringMatcher>` a new string matcher type which matches if the value of the string has the substring mentioned in contains matcher.
 * http: added :ref:`contains <envoy_api_msg_route.HeaderMatcher>` a new header matcher type which matches if the value of the header has the substring mentioned in contains matcher.
 * http: added :ref:`headers_to_add <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.ResponseMapper.headers_to_add>` to :ref:`local reply mapper <config_http_conn_man_local_reply>` to allow its users to add/append/override response HTTP headers to local replies.
-* http: added HCM level configuration of :ref:`error handling on invalid messaging <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>` which substantially changes Envoy's behavior when encountering invalid HTTP/1.1 defaulting to closing the connection instead of allowing reuse. This can temporarily be reverted by setting `envoy.reloadable_features.hcm_stream_error_on_invalid_message` to false, or permanently reverted by setting the :ref:`HCM option <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>` to true to restore prior HTTP/1.1 beavior and setting the *new* HTTP/2 configuration :ref:`override_stream_error_on_invalid_http_message <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.override_stream_error_on_invalid_http_message>` to false to retain prior HTTP/2 behavior.
+* http: added HCM level configuration of :ref:`error handling on invalid messaging <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>` which substantially changes Envoy's behavior when encountering invalid HTTP/1.1 defaulting to closing the connection instead of allowing reuse. This can temporarily be reverted by setting `envoy.reloadable_features.hcm_stream_error_on_invalid_message` to false, or permanently reverted by setting the HTTP/1 configuration :ref:`override_stream_error_on_invalid_http_message <envoy_v3_api_field_config.core.v3.Http1ProtocolOptions.override_stream_error_on_invalid_http_message>` to true to restore prior HTTP/1.1 behavior (i.e. connection isn't terminated) and to retain prior HTTP/2 behavior (i.e. connection is terminated).
 * http: changed Envoy to send error headers and body when possible. This behavior may be temporarily reverted by setting `envoy.reloadable_features.allow_response_for_timeout` to false.
 * http: changed empty trailers encoding behavior by sending empty data with ``end_stream`` true (instead of sending empty trailers) for HTTP/2. This behavior can be reverted temporarily by setting runtime feature ``envoy.reloadable_features.http2_skip_encoding_empty_trailers`` to false.
 * http: clarified and enforced 1xx handling. Multiple 100-continue headers are coalesced when proxying. 1xx headers other than {100, 101} are dropped.

include/envoy/http/codec.h

@@ -142,6 +142,12 @@ class ResponseEncoder : public virtual StreamEncoder {
    * @param trailers supplies the trailers to encode.
    */
   virtual void encodeTrailers(const ResponseTrailerMap& trailers) PURE;
+
+  /**
+   * Indicates whether invalid HTTP messaging should be handled with a stream error or a connection
+   * error; if value is missing, then HCM drives behaviour on invalid HTTP messaging.
+   */
+  virtual bool streamErrorOnInvalidHttpMessage() const PURE;
 };
 
 /**
@@ -386,6 +392,11 @@ struct Http1Settings {
 
   // How header keys should be formatted when serializing HTTP/1.1 headers.
   HeaderKeyFormat header_key_format_{HeaderKeyFormat::Default};
+
+  // Behaviour on invalid HTTP messaging:
+  // - if true, the HTTP/1.1 connection is left open (where possible)
+  // - if false, the HTTP/1.1 connection is terminated
+  bool stream_error_on_invalid_http_message_{false};
 };
 
 /**

source/common/http/conn_manager_impl.cc

@@ -1170,8 +1170,8 @@ void ConnectionManagerImpl::ActiveStream::onLocalReply(Code code) {
   // The BadRequest error code indicates there has been a messaging error.
   if (Runtime::runtimeFeatureEnabled(
           "envoy.reloadable_features.hcm_stream_error_on_invalid_message") &&
-      !connection_manager_.config_.streamErrorOnInvalidHttpMessaging() &&
-      code == Http::Code::BadRequest && connection_manager_.codec_->protocol() < Protocol::Http2) {
+      code == Http::Code::BadRequest && connection_manager_.codec_->protocol() < Protocol::Http2 &&
+      !response_encoder_->streamErrorOnInvalidHttpMessage()) {
     state_.saw_connection_close_ = true;
   }
 }

source/common/http/http1/codec_impl.h

@@ -123,8 +123,10 @@ class StreamEncoderImpl : public virtual StreamEncoder,
  */
 class ResponseEncoderImpl : public StreamEncoderImpl, public ResponseEncoder {
 public:
-  ResponseEncoderImpl(ConnectionImpl& connection, HeaderKeyFormatter* header_key_formatter)
-      : StreamEncoderImpl(connection, header_key_formatter) {}
+  ResponseEncoderImpl(ConnectionImpl& connection, HeaderKeyFormatter* header_key_formatter,
+                      bool stream_error_on_invalid_http_message)
+      : StreamEncoderImpl(connection, header_key_formatter),
+        stream_error_on_invalid_http_message_(stream_error_on_invalid_http_message) {}
 
   bool startedResponse() { return started_response_; }
 
@@ -133,8 +135,13 @@ class ResponseEncoderImpl : public StreamEncoderImpl, public ResponseEncoder {
   void encodeHeaders(const ResponseHeaderMap& headers, bool end_stream) override;
   void encodeTrailers(const ResponseTrailerMap& trailers) override { encodeTrailersBase(trailers); }
 
+  bool streamErrorOnInvalidHttpMessage() const override {
+    return stream_error_on_invalid_http_message_;
+  }
+
 private:
   bool started_response_{};
+  const bool stream_error_on_invalid_http_message_;
 };
 
 /**
@@ -462,8 +469,9 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
    * An active HTTP/1.1 request.
    */
   struct ActiveRequest {
-    ActiveRequest(ConnectionImpl& connection, HeaderKeyFormatter* header_key_formatter)
-        : response_encoder_(connection, header_key_formatter) {}
+    ActiveRequest(ServerConnectionImpl& connection, HeaderKeyFormatter* header_key_formatter)
+        : response_encoder_(connection, header_key_formatter,
+                            connection.codec_settings_.stream_error_on_invalid_http_message_) {}
 
     HeaderString request_url_;
     RequestDecoder* request_decoder_{};

source/common/http/http1/codec_impl_legacy.h

@@ -126,8 +126,10 @@ class StreamEncoderImpl : public virtual StreamEncoder,
 class ResponseEncoderImpl : public StreamEncoderImpl, public ResponseEncoder {
 public:
   ResponseEncoderImpl(ConnectionImpl& connection,
-                      Http::Http1::HeaderKeyFormatter* header_key_formatter)
-      : StreamEncoderImpl(connection, header_key_formatter) {}
+                      Http::Http1::HeaderKeyFormatter* header_key_formatter,
+                      bool stream_error_on_invalid_http_message)
+      : StreamEncoderImpl(connection, header_key_formatter),
+        stream_error_on_invalid_http_message_(stream_error_on_invalid_http_message) {}
 
   bool startedResponse() { return started_response_; }
 
@@ -136,8 +138,13 @@ class ResponseEncoderImpl : public StreamEncoderImpl, public ResponseEncoder {
   void encodeHeaders(const ResponseHeaderMap& headers, bool end_stream) override;
   void encodeTrailers(const ResponseTrailerMap& trailers) override { encodeTrailersBase(trailers); }
 
+  bool streamErrorOnInvalidHttpMessage() const override {
+    return stream_error_on_invalid_http_message_;
+  }
+
 private:
   bool started_response_{};
+  const bool stream_error_on_invalid_http_message_;
 };
 
 /**
@@ -435,8 +442,10 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
    * An active HTTP/1.1 request.
    */
   struct ActiveRequest {
-    ActiveRequest(ConnectionImpl& connection, Http::Http1::HeaderKeyFormatter* header_key_formatter)
-        : response_encoder_(connection, header_key_formatter) {}
+    ActiveRequest(ServerConnectionImpl& connection,
+                  Http::Http1::HeaderKeyFormatter* header_key_formatter)
+        : response_encoder_(connection, header_key_formatter,
+                            connection.codec_settings_.stream_error_on_invalid_http_message_) {}
 
     HeaderString request_url_;
     RequestDecoder* request_decoder_{};

source/common/http/http2/codec_impl.h

@@ -391,6 +391,10 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
 
     RequestDecoder* request_decoder_{};
     absl::variant<RequestHeaderMapPtr, RequestTrailerMapPtr> headers_or_trailers_;
+
+    bool streamErrorOnInvalidHttpMessage() const override {
+      return parent_.stream_error_on_invalid_http_messaging_;
+    }
   };
 
   using ServerStreamImplPtr = std::unique_ptr<ServerStreamImpl>;

source/common/http/http2/codec_impl_legacy.h

@@ -359,7 +359,9 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
    */
   struct ServerStreamImpl : public StreamImpl, public ResponseEncoder {
     ServerStreamImpl(ConnectionImpl& parent, uint32_t buffer_limit)
-        : StreamImpl(parent, buffer_limit), headers_or_trailers_(RequestHeaderMapImpl::create()) {}
+        : StreamImpl(parent, buffer_limit), headers_or_trailers_(RequestHeaderMapImpl::create()) {
+      stream_error_on_invalid_http_message_ = parent.stream_error_on_invalid_http_messaging_;
+    }
 
     // StreamImpl
     void submitHeaders(const std::vector<nghttp2_nv>& final_headers,
@@ -391,6 +393,11 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
 
     RequestDecoder* request_decoder_{};
     absl::variant<RequestHeaderMapPtr, RequestTrailerMapPtr> headers_or_trailers_;
+    bool stream_error_on_invalid_http_message_;
+
+    bool streamErrorOnInvalidHttpMessage() const override {
+      return stream_error_on_invalid_http_message_;
+    }
   };
 
   using ServerStreamImplPtr = std::unique_ptr<ServerStreamImpl>;

source/common/http/utility.cc

@@ -414,6 +414,25 @@ Utility::parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOptions&
   return ret;
 }
 
+Http1Settings
+Utility::parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOptions& config,
+                            bool hcm_stream_error_set,
+                            const Protobuf::BoolValue& hcm_stream_error) {
+  Http1Settings ret = parseHttp1Settings(config);
+
+  if (config.has_override_stream_error_on_invalid_http_message()) {
+    // override_stream_error_on_invalid_http_message, if set, takes precedence over any HCM
+    // stream_error_on_invalid_http_message
+    ret.stream_error_on_invalid_http_message_ =
+        config.override_stream_error_on_invalid_http_message().value();
+  } else if (hcm_stream_error_set) {
+    // HCM, if set, overrides override_stream_error_on_invalid_http_message
+    ret.stream_error_on_invalid_http_message_ = hcm_stream_error.value();
+  }
+
+  return ret;
+}
+
 void Utility::sendLocalReply(const bool& is_reset, StreamDecoderFilterCallbacks& callbacks,
                              const LocalReplyData& local_reply_data) {
   sendLocalReply(

source/common/http/utility.h

@@ -269,6 +269,10 @@ bool isWebSocketUpgradeRequest(const RequestHeaderMap& headers);
  */
 Http1Settings parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOptions& config);
 
+Http1Settings parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOptions& config,
+                                 bool hcm_stream_error_set,
+                                 const Protobuf::BoolValue& hcm_stream_error);
+
 struct EncodeFunctions {
   // Function to rewrite locally generated response.
   std::function<void(ResponseHeaderMap& response_headers, Code& code, std::string& body,