From dbc48ced37903c55cd155b0140bc30d18f4db486 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Thu, 16 Jul 2020 17:01:04 -0400 Subject: [PATCH] http: stream error on invalid messaging (#11748) This unifies HTTP/1.1 and HTTP/2 stream error on invalid messaging. Previously HTTP/1.1 defaulted permissive and HTTP/2 defaulted to strict. This defaults both to strict, resetting connections on invalid requests. This will have a major latency impact if downstream is sending a mix of valid and invalid requests over HTTP/1.1 Additional Description: This change is runtime guarded per default behavioral change rules. It can also be reverted by setting the default to permissive (for prior HTTP/1 behvior) then overriding HTTP/2 to struct (for prior HTTP/2 behavior). This works in conjunction with #11714, as the HTTP connection manager enforces the strictness, so the responses need to be sent via the HTTP connection manager to have strictness applied correctly. Risk Level: High (HCM changes) Testing: new unit tests, updated integration tests Docs Changes: n/a Release Notes: inline Runtime guard: envoy.reloadable_features.hcm_stream_error_on_invalid_message Fixes #9846 Signed-off-by: Alyssa Wilk Signed-off-by: scheler --- api/envoy/config/core/v3/protocol.proto | 21 +++++- api/envoy/config/core/v4alpha/protocol.proto | 11 +++- .../v3/http_connection_manager.proto | 19 +++++- .../v4alpha/http_connection_manager.proto | 19 +++++- .../best_practices/level_two.rst | 16 ++--- docs/root/version_history/current.rst | 1 + .../envoy/config/core/v3/protocol.proto | 21 +++++- .../envoy/config/core/v4alpha/protocol.proto | 21 +++++- .../v3/http_connection_manager.proto | 19 +++++- .../v4alpha/http_connection_manager.proto | 19 +++++- source/common/http/BUILD | 1 + source/common/http/conn_manager_config.h | 6 ++ source/common/http/conn_manager_impl.cc | 8 +++ source/common/http/http2/codec_impl.cc | 2 +- source/common/http/utility.cc | 20 ++++++ source/common/http/utility.h | 4 ++ source/common/runtime/runtime_features.cc | 1 + .../network/http_connection_manager/config.cc | 6 +- .../network/http_connection_manager/config.h | 4 ++ source/server/admin/admin.h | 1 + test/common/http/BUILD | 1 + .../http/conn_manager_impl_fuzz_test.cc | 4 ++ test/common/http/conn_manager_impl_test.cc | 6 +- test/common/http/conn_manager_utility_test.cc | 1 + test/common/http/http2/codec_impl_test.cc | 3 +- test/common/http/utility_test.cc | 65 +++++++++++++++++++ test/integration/http2_integration_test.cc | 8 ++- test/integration/integration_test.cc | 14 ++++ test/integration/protocol_integration_test.cc | 14 ++-- test/integration/stats_integration_test.cc | 6 +- test/mocks/runtime/mocks.cc | 4 ++ 31 files changed, 313 insertions(+), 33 deletions(-) diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index 339601feab3d..0ab6289e9659 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -159,7 +159,7 @@ message Http1ProtocolOptions { bool enable_trailers = 5; } -// [#next-free-field: 14] +// [#next-free-field: 15] message Http2ProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.Http2ProtocolOptions"; @@ -280,8 +280,25 @@ message Http2ProtocolOptions { // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, // when this option is enabled, only the offending stream is terminated. // + // This is overridden by HCM :ref:`stream_error_on_invalid_http_messaging + // ` + // iff present. + // + // This is deprecated in favor of :ref:`override_stream_error_on_invalid_http_message + // ` + // + // See `RFC7540, sec. 8.1 `_ for details. + bool stream_error_on_invalid_http_messaging = 12 [deprecated = true]; + + // Allows invalid HTTP messaging and headers. When this option is disabled (default), then + // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, + // when this option is enabled, only the offending stream is terminated. + // + // This overrides any HCM :ref:`stream_error_on_invalid_http_messaging + // ` + // // See `RFC7540, sec. 8.1 `_ for details. - bool stream_error_on_invalid_http_messaging = 12; + google.protobuf.BoolValue override_stream_error_on_invalid_http_message = 14; // [#not-implemented-hide:] // Specifies SETTINGS frame parameters to be sent to the peer, with two exceptions: diff --git a/api/envoy/config/core/v4alpha/protocol.proto b/api/envoy/config/core/v4alpha/protocol.proto index 2ec9244124bd..955c29335a3f 100644 --- a/api/envoy/config/core/v4alpha/protocol.proto +++ b/api/envoy/config/core/v4alpha/protocol.proto @@ -159,7 +159,7 @@ message Http1ProtocolOptions { bool enable_trailers = 5; } -// [#next-free-field: 14] +// [#next-free-field: 15] message Http2ProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.config.core.v3.Http2ProtocolOptions"; @@ -180,6 +180,10 @@ message Http2ProtocolOptions { google.protobuf.UInt32Value value = 2 [(validate.rules).message = {required: true}]; } + reserved 12; + + reserved "stream_error_on_invalid_http_messaging"; + // `Maximum table size `_ // (in octets) that the encoder is permitted to use for the dynamic HPACK table. Valid values // range from 0 to 4294967295 (2^32 - 1) and defaults to 4096. 0 effectively disables header @@ -280,8 +284,11 @@ message Http2ProtocolOptions { // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, // when this option is enabled, only the offending stream is terminated. // + // This overrides any HCM :ref:`stream_error_on_invalid_http_messaging + // ` + // // See `RFC7540, sec. 8.1 `_ for details. - bool stream_error_on_invalid_http_messaging = 12; + google.protobuf.BoolValue override_stream_error_on_invalid_http_message = 14; // [#not-implemented-hide:] // Specifies SETTINGS frame parameters to be sent to the peer, with two exceptions: diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index f2a80959c33b..a23fcc99e07c 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -36,7 +36,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 40] +// [#next-free-field: 41] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"; @@ -540,6 +540,23 @@ message HttpConnectionManager { // route with :ref:`domains` match set to `example`. Defaults to `false`. Note that port removal is not part // of `HTTP spec `_ and is provided for convenience. bool strip_matching_host_port = 39; + + // Governs Envoy's behavior when receiving invalid HTTP from downstream. + // If this option is false (default), Envoy will err on the conservative side handling HTTP + // errors, terminating both HTTP/1.1 and HTTP/2 connections when receiving an invalid request. + // If this option is set to true, Envoy will be more permissive, only resetting the invalid + // stream in the case of HTTP/2 and leaving the connection open where possible (if the entire + // request is read for HTTP/1.1) + // In general this should be true for deployments receiving trusted traffic (L2 Envoys, + // 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 + // :ref:`override_stream_error_on_invalid_http_message + // ` + // *not* the deprecated but similarly named :ref:`stream_error_on_invalid_http_messaging + // ` + google.protobuf.BoolValue stream_error_on_invalid_http_message = 40; } // The configuration to customize local reply returned by Envoy. diff --git a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index aaf146e1f568..bdf3618ba328 100644 --- a/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -35,7 +35,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 40] +// [#next-free-field: 41] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; @@ -539,6 +539,23 @@ message HttpConnectionManager { // route with :ref:`domains` match set to `example`. Defaults to `false`. Note that port removal is not part // of `HTTP spec `_ and is provided for convenience. bool strip_matching_host_port = 39; + + // Governs Envoy's behavior when receiving invalid HTTP from downstream. + // If this option is false (default), Envoy will err on the conservative side handling HTTP + // errors, terminating both HTTP/1.1 and HTTP/2 connections when receiving an invalid request. + // If this option is set to true, Envoy will be more permissive, only resetting the invalid + // stream in the case of HTTP/2 and leaving the connection open where possible (if the entire + // request is read for HTTP/1.1) + // In general this should be true for deployments receiving trusted traffic (L2 Envoys, + // 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 + // :ref:`override_stream_error_on_invalid_http_message + // ` + // *not* the deprecated but similarly named :ref:`stream_error_on_invalid_http_messaging + // ` + google.protobuf.BoolValue stream_error_on_invalid_http_message = 40; } // The configuration to customize local reply returned by Envoy. diff --git a/docs/root/configuration/best_practices/level_two.rst b/docs/root/configuration/best_practices/level_two.rst index c38dae7cdb68..44c52ace8a8e 100644 --- a/docs/root/configuration/best_practices/level_two.rst +++ b/docs/root/configuration/best_practices/level_two.rst @@ -5,14 +5,14 @@ Configuring Envoy as a level two proxy Envoy is a production-ready proxy, however, the default settings that are tailored for the edge use case may need to be adjusted when using Envoy in a multi-level deployment as a -"level two" HTTP/2 proxy. +"level two" proxy. .. image:: /_static/multilevel_deployment.svg **In summary, if you run level two Envoy version 1.11.1 or greater which terminates -HTTP/2, we strongly advise you to change the HTTP/2 configuration of your level +HTTP/2, we strongly advise you to change the HttpConnectionManager configuration of your level two Envoy, by setting its downstream** -:ref:`validation of HTTP/2 messaging option ` +:ref:`validation of HTTP messaging option ` **to true.** If there is an invalid HTTP/2 request and this option is not set, the Envoy in @@ -29,9 +29,7 @@ user has insight into what traffic will bypass level one checks, they could spra “bad” traffic across the level one fleet, causing serious disruption to other users’ traffic. -Please note that the -:ref:`validation of HTTP/2 messaging option ` -is planned to be deprecated and replaced with mandatory configuration in the HttpConnectionManager, to ensure -that what is now an easily overlooked option would need to be configured, ideally -appropriately for the given Envoy deployment. Please refer to the -https://github.com/envoyproxy/envoy/issues/9285 for more information. +This configuration option also has implications for invalid HTTP/1.1 though slightly less +severe ones. For Envoy L1s, invalid HTTP/1 requests will also result in connection +reset. If the option is set to true, and the request is completely read, the connection +will persist and can be reused for a subsequent request. diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index b49649b892d3..eb3465f99eeb 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -10,6 +10,7 @@ Minor Behavior Changes *Changes that may cause incompatibilities for some users, but should not for most* * compressor: always insert `Vary` headers for compressible resources even if it's decided not to compress a response due to incompatible `Accept-Encoding` value. The `Vary` header needs to be inserted to let a caching proxy in front of Envoy know that the requested resource still can be served with compression applied. +* http: added HCM level configuration of :ref:`error handling on invalid messaging ` 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 ` to true to restore prior HTTP/1.1 beavior and setting the *new* HTTP/2 configuration :ref:`override_stream_error_on_invalid_http_message ` to false to retain prior HTTP/2 behavior. * http: the per-stream FilterState maintained by the HTTP connection manager will now provide read/write access to the downstream connection FilterState. As such, code that relies on interacting with this might see a change in behavior. * logging: nghttp2 log messages no longer appear at trace level unless `ENVOY_NGHTTP2_TRACE` is set diff --git a/generated_api_shadow/envoy/config/core/v3/protocol.proto b/generated_api_shadow/envoy/config/core/v3/protocol.proto index 339601feab3d..0ab6289e9659 100644 --- a/generated_api_shadow/envoy/config/core/v3/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v3/protocol.proto @@ -159,7 +159,7 @@ message Http1ProtocolOptions { bool enable_trailers = 5; } -// [#next-free-field: 14] +// [#next-free-field: 15] message Http2ProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.Http2ProtocolOptions"; @@ -280,8 +280,25 @@ message Http2ProtocolOptions { // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, // when this option is enabled, only the offending stream is terminated. // + // This is overridden by HCM :ref:`stream_error_on_invalid_http_messaging + // ` + // iff present. + // + // This is deprecated in favor of :ref:`override_stream_error_on_invalid_http_message + // ` + // + // See `RFC7540, sec. 8.1 `_ for details. + bool stream_error_on_invalid_http_messaging = 12 [deprecated = true]; + + // Allows invalid HTTP messaging and headers. When this option is disabled (default), then + // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, + // when this option is enabled, only the offending stream is terminated. + // + // This overrides any HCM :ref:`stream_error_on_invalid_http_messaging + // ` + // // See `RFC7540, sec. 8.1 `_ for details. - bool stream_error_on_invalid_http_messaging = 12; + google.protobuf.BoolValue override_stream_error_on_invalid_http_message = 14; // [#not-implemented-hide:] // Specifies SETTINGS frame parameters to be sent to the peer, with two exceptions: diff --git a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto index 2ec9244124bd..cc1b99d0a048 100644 --- a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto @@ -159,7 +159,7 @@ message Http1ProtocolOptions { bool enable_trailers = 5; } -// [#next-free-field: 14] +// [#next-free-field: 15] message Http2ProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.config.core.v3.Http2ProtocolOptions"; @@ -280,8 +280,25 @@ message Http2ProtocolOptions { // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, // when this option is enabled, only the offending stream is terminated. // + // This is overridden by HCM :ref:`stream_error_on_invalid_http_messaging + // ` + // iff present. + // + // This is deprecated in favor of :ref:`override_stream_error_on_invalid_http_message + // ` + // + // See `RFC7540, sec. 8.1 `_ for details. + bool hidden_envoy_deprecated_stream_error_on_invalid_http_messaging = 12 [deprecated = true]; + + // Allows invalid HTTP messaging and headers. When this option is disabled (default), then + // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, + // when this option is enabled, only the offending stream is terminated. + // + // This overrides any HCM :ref:`stream_error_on_invalid_http_messaging + // ` + // // See `RFC7540, sec. 8.1 `_ for details. - bool stream_error_on_invalid_http_messaging = 12; + google.protobuf.BoolValue override_stream_error_on_invalid_http_message = 14; // [#not-implemented-hide:] // Specifies SETTINGS frame parameters to be sent to the peer, with two exceptions: diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index 54e531ceb6a0..322212670988 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -36,7 +36,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 40] +// [#next-free-field: 41] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"; @@ -543,6 +543,23 @@ message HttpConnectionManager { // of `HTTP spec `_ and is provided for convenience. bool strip_matching_host_port = 39; + // Governs Envoy's behavior when receiving invalid HTTP from downstream. + // If this option is false (default), Envoy will err on the conservative side handling HTTP + // errors, terminating both HTTP/1.1 and HTTP/2 connections when receiving an invalid request. + // If this option is set to true, Envoy will be more permissive, only resetting the invalid + // stream in the case of HTTP/2 and leaving the connection open where possible (if the entire + // request is read for HTTP/1.1) + // In general this should be true for deployments receiving trusted traffic (L2 Envoys, + // 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 + // :ref:`override_stream_error_on_invalid_http_message + // ` + // *not* the deprecated but similarly named :ref:`stream_error_on_invalid_http_messaging + // ` + google.protobuf.BoolValue stream_error_on_invalid_http_message = 40; + google.protobuf.Duration hidden_envoy_deprecated_idle_timeout = 11 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; } diff --git a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index aaf146e1f568..bdf3618ba328 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -35,7 +35,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 40] +// [#next-free-field: 41] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; @@ -539,6 +539,23 @@ message HttpConnectionManager { // route with :ref:`domains` match set to `example`. Defaults to `false`. Note that port removal is not part // of `HTTP spec `_ and is provided for convenience. bool strip_matching_host_port = 39; + + // Governs Envoy's behavior when receiving invalid HTTP from downstream. + // If this option is false (default), Envoy will err on the conservative side handling HTTP + // errors, terminating both HTTP/1.1 and HTTP/2 connections when receiving an invalid request. + // If this option is set to true, Envoy will be more permissive, only resetting the invalid + // stream in the case of HTTP/2 and leaving the connection open where possible (if the entire + // request is read for HTTP/1.1) + // In general this should be true for deployments receiving trusted traffic (L2 Envoys, + // 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 + // :ref:`override_stream_error_on_invalid_http_message + // ` + // *not* the deprecated but similarly named :ref:`stream_error_on_invalid_http_messaging + // ` + google.protobuf.BoolValue stream_error_on_invalid_http_message = 40; } // The configuration to customize local reply returned by Envoy. diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 54de0782f5f6..5a07a56b9f00 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -358,6 +358,7 @@ envoy_cc_library( "//source/common/json:json_loader_lib", "//source/common/network:utility_lib", "//source/common/protobuf:utility_lib", + "//source/common/runtime:runtime_features_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) diff --git a/source/common/http/conn_manager_config.h b/source/common/http/conn_manager_config.h index faf691b6fc20..b67afc95a64c 100644 --- a/source/common/http/conn_manager_config.h +++ b/source/common/http/conn_manager_config.h @@ -409,6 +409,12 @@ class ConnectionManagerConfig { */ virtual bool proxy100Continue() const PURE; + /** + * @return bool supplies if the HttpConnectionManager should handle invalid HTTP with a stream + * error or connection error. + */ + virtual bool streamErrorOnInvalidHttpMessaging() const PURE; + /** * @return supplies the http1 settings. */ diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 4a76e84be282..df958ed68530 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -1521,6 +1521,14 @@ void ConnectionManagerImpl::ActiveStream::sendLocalReply( // a no-op. createFilterChain(); + // 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) { + state_.saw_connection_close_ = true; + } + stream_info_.setResponseCodeDetails(details); Utility::sendLocalReply( state_.destroyed_, diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index 4a94bb3aafdf..b25b8fa4bef5 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -492,7 +492,7 @@ ConnectionImpl::ConnectionImpl(Network::Connection& connection, CodecStats& stat max_headers_count_(max_headers_count), per_stream_buffer_limit_(http2_options.initial_stream_window_size().value()), stream_error_on_invalid_http_messaging_( - http2_options.stream_error_on_invalid_http_messaging()), + http2_options.override_stream_error_on_invalid_http_message().value()), flood_detected_(false), max_outbound_frames_(http2_options.max_outbound_frames().value()), frame_buffer_releasor_([this]() { releaseOutboundFrame(); }), max_outbound_control_frames_(http2_options.max_outbound_control_frames().value()), diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index bee04a98cabe..12006e143fa2 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -21,6 +21,7 @@ #include "common/http/message_impl.h" #include "common/network/utility.h" #include "common/protobuf/utility.h" +#include "common/runtime/runtime_features.h" #include "absl/strings/match.h" #include "absl/strings/numbers.h" @@ -141,12 +142,31 @@ const uint32_t OptionsLimits::DEFAULT_MAX_CONSECUTIVE_INBOUND_FRAMES_WITH_EMPTY_ const uint32_t OptionsLimits::DEFAULT_MAX_INBOUND_PRIORITY_FRAMES_PER_STREAM; const uint32_t OptionsLimits::DEFAULT_MAX_INBOUND_WINDOW_UPDATE_FRAMES_PER_DATA_FRAME_SENT; +envoy::config::core::v3::Http2ProtocolOptions +initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options, + bool hcm_stream_error_set, + const Protobuf::BoolValue& hcm_stream_error) { + auto ret = initializeAndValidateOptions(options); + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.hcm_stream_error_on_invalid_message") && + !options.has_override_stream_error_on_invalid_http_message() && hcm_stream_error_set) { + ret.mutable_override_stream_error_on_invalid_http_message()->set_value( + hcm_stream_error.value()); + } + return ret; +} + envoy::config::core::v3::Http2ProtocolOptions initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options) { envoy::config::core::v3::Http2ProtocolOptions options_clone(options); // This will throw an exception when a custom parameter and a named parameter collide. validateCustomSettingsParameters(options); + if (!options.has_override_stream_error_on_invalid_http_message()) { + options_clone.mutable_override_stream_error_on_invalid_http_message()->set_value( + options.stream_error_on_invalid_http_messaging()); + } + if (!options_clone.has_hpack_table_size()) { options_clone.mutable_hpack_table_size()->set_value(OptionsLimits::DEFAULT_HPACK_TABLE_SIZE); } diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 4fef6cc23327..492193c4e2ff 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -116,6 +116,10 @@ struct OptionsLimits { envoy::config::core::v3::Http2ProtocolOptions initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options); +envoy::config::core::v3::Http2ProtocolOptions +initializeAndValidateOptions(const envoy::config::core::v3::Http2ProtocolOptions& options, + bool hcm_stream_error_set, + const Protobuf::BoolValue& hcm_stream_error); } // namespace Utility } // namespace Http2 diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 8c1864ff69a2..ba4817e19a53 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -73,6 +73,7 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.preserve_query_string_in_path_redirects", "envoy.reloadable_features.preserve_upstream_date", "envoy.reloadable_features.stop_faking_paths", + "envoy.reloadable_features.hcm_stream_error_on_invalid_message", "envoy.reloadable_features.strict_1xx_and_204_response_headers", }; diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 3f8abde5ae24..190db7f475b4 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -178,7 +178,9 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( skip_xff_append_(config.skip_xff_append()), via_(config.via()), route_config_provider_manager_(route_config_provider_manager), scoped_routes_config_provider_manager_(scoped_routes_config_provider_manager), - http2_options_(Http2::Utility::initializeAndValidateOptions(config.http2_protocol_options())), + http2_options_(Http2::Utility::initializeAndValidateOptions( + config.http2_protocol_options(), config.has_stream_error_on_invalid_http_message(), + config.stream_error_on_invalid_http_message())), http1_settings_(Http::Utility::parseHttp1Settings(config.http_protocol_options())), max_request_headers_kb_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( config, max_request_headers_kb, Http::DEFAULT_MAX_REQUEST_HEADERS_KB)), @@ -202,6 +204,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( listener_stats_(Http::ConnectionManagerImpl::generateListenerStats(stats_prefix_, context_.listenerScope())), proxy_100_continue_(config.proxy_100_continue()), + stream_error_on_invalid_http_messaging_( + PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, stream_error_on_invalid_http_message, false)), delayed_close_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, delayed_close_timeout, 1000)), #ifdef ENVOY_NORMALIZE_PATH_BY_DEFAULT normalize_path_(PROTOBUF_GET_WRAPPED_OR_DEFAULT( diff --git a/source/extensions/filters/network/http_connection_manager/config.h b/source/extensions/filters/network/http_connection_manager/config.h index f10ee8ccee48..b522fad49b66 100644 --- a/source/extensions/filters/network/http_connection_manager/config.h +++ b/source/extensions/filters/network/http_connection_manager/config.h @@ -157,6 +157,9 @@ class HttpConnectionManagerConfig : Logger::Loggable, const absl::optional& userAgent() override { return user_agent_; } Http::ConnectionManagerListenerStats& listenerStats() override { return listener_stats_; } bool proxy100Continue() const override { return proxy_100_continue_; } + bool streamErrorOnInvalidHttpMessaging() const override { + return stream_error_on_invalid_http_messaging_; + } const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return normalize_path_; } bool shouldMergeSlashes() const override { return merge_slashes_; } @@ -228,6 +231,7 @@ class HttpConnectionManagerConfig : Logger::Loggable, Http::DateProvider& date_provider_; Http::ConnectionManagerListenerStats listener_stats_; const bool proxy_100_continue_; + const bool stream_error_on_invalid_http_messaging_; std::chrono::milliseconds delayed_close_timeout_; const bool normalize_path_; const bool merge_slashes_; diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index 278aa30c342b..a1e32e34c74b 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -170,6 +170,7 @@ class AdminImpl : public Admin, const Http::TracingConnectionManagerConfig* tracingConfig() override { return nullptr; } Http::ConnectionManagerListenerStats& listenerStats() override { return listener_->stats_; } bool proxy100Continue() const override { return false; } + bool streamErrorOnInvalidHttpMessaging() const override { return false; } const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return true; } bool shouldMergeSlashes() const override { return true; } diff --git a/test/common/http/BUILD b/test/common/http/BUILD index ec2987c48403..13e7911ee08f 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -369,6 +369,7 @@ envoy_cc_test( "//source/common/network:address_lib", "//test/mocks/http:http_mocks", "//test/mocks/upstream:upstream_mocks", + "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], diff --git a/test/common/http/conn_manager_impl_fuzz_test.cc b/test/common/http/conn_manager_impl_fuzz_test.cc index 7c85051528ab..ef957d113c00 100644 --- a/test/common/http/conn_manager_impl_fuzz_test.cc +++ b/test/common/http/conn_manager_impl_fuzz_test.cc @@ -187,6 +187,9 @@ class FuzzConfig : public ConnectionManagerConfig { const TracingConnectionManagerConfig* tracingConfig() override { return tracing_config_.get(); } ConnectionManagerListenerStats& listenerStats() override { return listener_stats_; } bool proxy100Continue() const override { return proxy_100_continue_; } + bool streamErrorOnInvalidHttpMessaging() const override { + return stream_error_on_invalid_http_messaging_; + } const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return false; } bool shouldMergeSlashes() const override { return false; } @@ -234,6 +237,7 @@ class FuzzConfig : public ConnectionManagerConfig { Tracing::HttpTracerSharedPtr http_tracer_{std::make_shared>()}; TracingConnectionManagerConfigPtr tracing_config_; bool proxy_100_continue_{true}; + bool stream_error_on_invalid_http_messaging_ = false; bool preserve_external_request_id_{false}; Http::Http1Settings http1_settings_; Http::DefaultInternalAddressConfig internal_address_config_; diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index efd7222b9123..5fc4624d37d4 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -95,7 +95,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan POOL_HISTOGRAM(fake_stats_))}, "", fake_stats_), tracing_stats_{CONN_MAN_TRACING_STATS(POOL_COUNTER(fake_stats_))}, - listener_stats_{CONN_MAN_LISTENER_STATS(POOL_COUNTER(fake_listener_stats_))}, + listener_stats_({CONN_MAN_LISTENER_STATS(POOL_COUNTER(fake_listener_stats_))}), request_id_extension_(RequestIDExtensionFactory::defaultInstance(random_)), local_reply_(LocalReply::Factory::createDefault()) { @@ -348,6 +348,9 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan const TracingConnectionManagerConfig* tracingConfig() override { return tracing_config_.get(); } ConnectionManagerListenerStats& listenerStats() override { return listener_stats_; } bool proxy100Continue() const override { return proxy_100_continue_; } + bool streamErrorOnInvalidHttpMessaging() const override { + return stream_error_on_invalid_http_messaging_; + } const Http::Http1Settings& http1Settings() const override { return http1_settings_; } bool shouldNormalizePath() const override { return normalize_path_; } bool shouldMergeSlashes() const override { return merge_slashes_; } @@ -410,6 +413,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan Stats::IsolatedStoreImpl fake_listener_stats_; ConnectionManagerListenerStats listener_stats_; bool proxy_100_continue_ = false; + bool stream_error_on_invalid_http_messaging_ = false; bool preserve_external_request_id_ = false; Http::Http1Settings http1_settings_; bool normalize_path_ = false; diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 97d680b67c93..4b4348710844 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -135,6 +135,7 @@ class MockConnectionManagerConfig : public ConnectionManagerConfig { MOCK_METHOD(Tracing::HttpTracerSharedPtr, tracer, ()); MOCK_METHOD(ConnectionManagerListenerStats&, listenerStats, ()); MOCK_METHOD(bool, proxy100Continue, (), (const)); + MOCK_METHOD(bool, streamErrorOnInvalidHttpMessaging, (), (const)); MOCK_METHOD(const Http::Http1Settings&, http1Settings, (), (const)); MOCK_METHOD(bool, shouldNormalizePath, (), (const)); MOCK_METHOD(bool, shouldMergeSlashes, (), (const)); diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 55bc37182288..1deb3c412284 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -182,7 +182,8 @@ class Http2CodecImplTestFixture { (tp.has_value()) ? ::testing::get(*tp) : CommonUtility::OptionsLimits::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE); options.set_allow_metadata(allow_metadata_); - options.set_stream_error_on_invalid_http_messaging(stream_error_on_invalid_http_messaging_); + options.mutable_override_stream_error_on_invalid_http_message()->set_value( + stream_error_on_invalid_http_messaging_); options.mutable_max_outbound_frames()->set_value(max_outbound_frames_); options.mutable_max_outbound_control_frames()->set_value(max_outbound_control_frames_); options.mutable_max_consecutive_inbound_frames_with_empty_payload()->set_value( diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index 47c12303d0b6..0c1dd6c1277c 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -15,6 +15,7 @@ #include "test/mocks/http/mocks.h" #include "test/test_common/printers.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -355,6 +356,70 @@ initial_connection_window_size: 65535 } } +TEST(HttpUtility, ValidateStreamErrors) { + // Both false, the result should be false. + envoy::config::core::v3::Http2ProtocolOptions http2_options; + EXPECT_FALSE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options) + .override_stream_error_on_invalid_http_message() + .value()); + + // If the new value is not present, the legacy value is respected. + http2_options.set_stream_error_on_invalid_http_messaging(true); + EXPECT_TRUE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options) + .override_stream_error_on_invalid_http_message() + .value()); + + // If the new value is present, it is used. + http2_options.mutable_override_stream_error_on_invalid_http_message()->set_value(true); + http2_options.set_stream_error_on_invalid_http_messaging(false); + EXPECT_TRUE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options) + .override_stream_error_on_invalid_http_message() + .value()); + + // Invert values - the new value should still be used. + http2_options.mutable_override_stream_error_on_invalid_http_message()->set_value(false); + http2_options.set_stream_error_on_invalid_http_messaging(true); + EXPECT_FALSE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options) + .override_stream_error_on_invalid_http_message() + .value()); +} + +TEST(HttpUtility, ValidateStreamErrorsWithHcm) { + envoy::config::core::v3::Http2ProtocolOptions http2_options; + http2_options.set_stream_error_on_invalid_http_messaging(true); + EXPECT_TRUE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options) + .override_stream_error_on_invalid_http_message() + .value()); + + // If the HCM value is present it will take precedence over the old value. + Protobuf::BoolValue hcm_value; + hcm_value.set_value(false); + EXPECT_FALSE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options, true, hcm_value) + .override_stream_error_on_invalid_http_message() + .value()); + // The HCM value will be ignored if initializeAndValidateOptions is told it is not present. + EXPECT_TRUE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options, false, hcm_value) + .override_stream_error_on_invalid_http_message() + .value()); + + // The override_stream_error_on_invalid_http_message takes precedence over the + // global one. + http2_options.mutable_override_stream_error_on_invalid_http_message()->set_value(true); + EXPECT_TRUE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options, true, hcm_value) + .override_stream_error_on_invalid_http_message() + .value()); + + { + // With runtime flipped, override is ignored. + TestScopedRuntime scoped_runtime; + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.reloadable_features.hcm_stream_error_on_invalid_message", "false"}}); + EXPECT_TRUE(Envoy::Http2::Utility::initializeAndValidateOptions(http2_options, true, hcm_value) + .override_stream_error_on_invalid_http_message() + .value()); + } +} + TEST(HttpUtility, getLastAddressFromXFF) { { const std::string first_address = "192.0.2.10"; diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index f663fcc0ef2c..73e0e615aa7a 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -1664,7 +1664,9 @@ TEST_P(Http2FloodMitigationTest, RST_STREAM) { config_helper_.addConfigModifier( [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) -> void { - hcm.mutable_http2_protocol_options()->set_stream_error_on_invalid_http_messaging(true); + hcm.mutable_http2_protocol_options() + ->mutable_override_stream_error_on_invalid_http_message() + ->set_value(true); }); beginSession(); @@ -1841,7 +1843,9 @@ TEST_P(Http2FloodMitigationTest, ZerolenHeaderAllowed) { config_helper_.addConfigModifier( [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) -> void { - hcm.mutable_http2_protocol_options()->set_stream_error_on_invalid_http_messaging(true); + hcm.mutable_http2_protocol_options() + ->mutable_override_stream_error_on_invalid_http_message() + ->set_value(true); }); autonomous_upstream_ = true; beginSession(); diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index e89400b690c8..37dce6d1a0e9 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -717,6 +717,10 @@ TEST_P(IntegrationTest, PipelineWithTrailers) { // an inline sendLocalReply to make sure the "kick" works under the call stack // of dispatch as well as when a response is proxied from upstream. TEST_P(IntegrationTest, PipelineInline) { + // When deprecating this flag, set hcm.mutable_stream_error_on_invalid_http_message true. + config_helper_.addRuntimeOverride("envoy.reloadable_features.hcm_stream_error_on_invalid_message", + "false"); + autonomous_upstream_ = true; initialize(); std::string response; @@ -1223,6 +1227,11 @@ TEST_P(UpstreamEndpointIntegrationTest, TestUpstreamEndpointAddress) { // Send continuous pipelined requests while not reading responses, to check // HTTP/1.1 response flood protection. TEST_P(IntegrationTest, TestFlood) { + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) -> void { + hcm.mutable_stream_error_on_invalid_http_message()->set_value(true); + }); initialize(); // Set up a raw connection to easily send requests without reading responses. @@ -1300,6 +1309,11 @@ TEST_P(IntegrationTest, TestFloodUpstreamErrors) { // Make sure flood protection doesn't kick in with many requests sent serially. TEST_P(IntegrationTest, TestManyBadRequests) { + config_helper_.addConfigModifier( + [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) -> void { + hcm.mutable_stream_error_on_invalid_http_message()->set_value(true); + }); initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 074181c73699..e41cd9eabe90 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -1114,7 +1114,9 @@ TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLengthAllowed) { config_helper_.addConfigModifier( [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) -> void { - hcm.mutable_http2_protocol_options()->set_stream_error_on_invalid_http_messaging(true); + hcm.mutable_http2_protocol_options() + ->mutable_override_stream_error_on_invalid_http_message() + ->set_value(true); }); initialize(); @@ -1170,7 +1172,9 @@ TEST_P(DownstreamProtocolIntegrationTest, MultipleContentLengthsAllowed) { config_helper_.addConfigModifier( [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) -> void { - hcm.mutable_http2_protocol_options()->set_stream_error_on_invalid_http_messaging(true); + hcm.mutable_http2_protocol_options() + ->mutable_override_stream_error_on_invalid_http_message() + ->set_value(true); }); initialize(); @@ -1958,7 +1962,7 @@ TEST_P(DownstreamProtocolIntegrationTest, ConnectIsBlocked) { } } -// Make sure that with stream_error_on_invalid_http_messaging true, CONNECT +// Make sure that with override_stream_error_on_invalid_http_message true, CONNECT // results in stream teardown not connection teardown. TEST_P(DownstreamProtocolIntegrationTest, ConnectStreamRejection) { if (downstreamProtocol() == Http::CodecClient::Type::HTTP1) { @@ -1967,7 +1971,9 @@ TEST_P(DownstreamProtocolIntegrationTest, ConnectStreamRejection) { config_helper_.addConfigModifier( [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& hcm) -> void { - hcm.mutable_http2_protocol_options()->set_stream_error_on_invalid_http_messaging(true); + hcm.mutable_http2_protocol_options() + ->mutable_override_stream_error_on_invalid_http_message() + ->set_value(true); }); initialize(); diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 904a7f94504d..9b339b8e4dc1 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -283,6 +283,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // 2020/06/29 11751 44715 46000 Improve time complexity of removing callback handle // in callback manager. // 2020/07/07 11252 44971 46000 Introduce Least Request LB active request bias config + // 2020/07/15 11748 45003 46000 Stream error on invalid messaging // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -300,7 +301,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // We only run the exact test for ipv6 because ipv4 in some cases may allocate a // different number of bytes. We still run the approximate test. if (ip_version_ != Network::Address::IpVersion::v6) { - EXPECT_MEMORY_EQ(m_per_cluster, 44971); + EXPECT_MEMORY_EQ(m_per_cluster, 45003); } EXPECT_MEMORY_LE(m_per_cluster, 46000); // Round up to allow platform variations. } @@ -355,6 +356,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // 2020/06/29 11751 36827 38000 Improve time complexity of removing callback handle. // in callback manager. // 2020/07/07 11252 37083 38000 Introduce Least Request LB active request bias config + // 2020/07/15 11748 37115 38000 Stream error on invalid messaging // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -372,7 +374,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // We only run the exact test for ipv6 because ipv4 in some cases may allocate a // different number of bytes. We still run the approximate test. if (ip_version_ != Network::Address::IpVersion::v6) { - EXPECT_MEMORY_EQ(m_per_cluster, 37083); + EXPECT_MEMORY_EQ(m_per_cluster, 37115); } EXPECT_MEMORY_LE(m_per_cluster, 38000); // Round up to allow platform variations. } diff --git a/test/mocks/runtime/mocks.cc b/test/mocks/runtime/mocks.cc index 3afd1bbbf743..d2f22d414c8f 100644 --- a/test/mocks/runtime/mocks.cc +++ b/test/mocks/runtime/mocks.cc @@ -4,6 +4,7 @@ #include "gtest/gtest.h" using testing::_; +using testing::NiceMock; using testing::Return; using testing::ReturnArg; @@ -20,6 +21,9 @@ MockSnapshot::MockSnapshot() { MockSnapshot::~MockSnapshot() = default; MockLoader::MockLoader() { + ON_CALL(*this, threadsafeSnapshot()).WillByDefault(testing::Invoke([]() { + return std::make_shared>(); + })); ON_CALL(*this, snapshot()).WillByDefault(ReturnRef(snapshot_)); ON_CALL(*this, getRootScope()).WillByDefault(ReturnRef(store_)); }