diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index efbaef04aaae..e2fb633376ce 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -347,57 +347,28 @@ void ConnectionImpl::StreamImpl::onMetadataDecoded(MetadataMapPtr&& metadata_map decoder_->decodeMetadata(std::move(metadata_map_ptr)); } -namespace { - -const char InvalidHttpMessagingOverrideKey[] = - "envoy.reloadable_features.http2_protocol_options.stream_error_on_invalid_http_messaging"; -const char MaxOutboundFramesOverrideKey[] = - "envoy.reloadable_features.http2_protocol_options.max_outbound_frames"; -const char MaxOutboundControlFramesOverrideKey[] = - "envoy.reloadable_features.http2_protocol_options.max_outbound_control_frames"; -const char MaxConsecutiveInboundFramesWithEmptyPayloadOverrideKey[] = - "envoy.reloadable_features.http2_protocol_options." - "max_consecutive_inbound_frames_with_empty_payload"; -const char MaxInboundPriorityFramesPerStreamOverrideKey[] = - "envoy.reloadable_features.http2_protocol_options.max_inbound_priority_frames_per_stream"; -const char MaxInboundWindowUpdateFramesPerDataFrameSentOverrideKey[] = - "envoy.reloadable_features.http2_protocol_options." - "max_inbound_window_update_frames_per_data_frame_sent"; - -bool checkRuntimeOverride(bool config_value, const char* override_key) { - return Runtime::runtimeFeatureEnabled(override_key) ? true : config_value; -} - -} // namespace - ConnectionImpl::ConnectionImpl(Network::Connection& connection, Stats::Scope& stats, const Http2Settings& http2_settings, const uint32_t max_headers_kb, const uint32_t max_headers_count) : stats_{ALL_HTTP2_CODEC_STATS(POOL_COUNTER_PREFIX(stats, "http2."))}, connection_(connection), max_headers_kb_(max_headers_kb), max_headers_count_(max_headers_count), per_stream_buffer_limit_(http2_settings.initial_stream_window_size_), - stream_error_on_invalid_http_messaging_(checkRuntimeOverride( - http2_settings.stream_error_on_invalid_http_messaging_, InvalidHttpMessagingOverrideKey)), - flood_detected_(false), - max_outbound_frames_( - Runtime::getInteger(MaxOutboundFramesOverrideKey, http2_settings.max_outbound_frames_)), + stream_error_on_invalid_http_messaging_( + http2_settings.stream_error_on_invalid_http_messaging_), + flood_detected_(false), max_outbound_frames_(http2_settings.max_outbound_frames_), frame_buffer_releasor_([this](const Buffer::OwnedBufferFragmentImpl* fragment) { releaseOutboundFrame(fragment); }), - max_outbound_control_frames_(Runtime::getInteger( - MaxOutboundControlFramesOverrideKey, http2_settings.max_outbound_control_frames_)), + max_outbound_control_frames_(http2_settings.max_outbound_control_frames_), control_frame_buffer_releasor_([this](const Buffer::OwnedBufferFragmentImpl* fragment) { releaseOutboundControlFrame(fragment); }), max_consecutive_inbound_frames_with_empty_payload_( - Runtime::getInteger(MaxConsecutiveInboundFramesWithEmptyPayloadOverrideKey, - http2_settings.max_consecutive_inbound_frames_with_empty_payload_)), + http2_settings.max_consecutive_inbound_frames_with_empty_payload_), max_inbound_priority_frames_per_stream_( - Runtime::getInteger(MaxInboundPriorityFramesPerStreamOverrideKey, - http2_settings.max_inbound_priority_frames_per_stream_)), - max_inbound_window_update_frames_per_data_frame_sent_(Runtime::getInteger( - MaxInboundWindowUpdateFramesPerDataFrameSentOverrideKey, - http2_settings.max_inbound_window_update_frames_per_data_frame_sent_)), + http2_settings.max_inbound_priority_frames_per_stream_), + max_inbound_window_update_frames_per_data_frame_sent_( + http2_settings.max_inbound_window_update_frames_per_data_frame_sent_), dispatching_(false), raised_goaway_(false), pending_deferred_reset_(false) {} ConnectionImpl::~ConnectionImpl() { nghttp2_session_del(session_); } diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 0f2432524e26..b4c116604e2e 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -45,8 +45,6 @@ constexpr const char* runtime_features[] = { constexpr const char* disabled_runtime_features[] = { // Sentinel and test flag. "envoy.reloadable_features.test_feature_false", - // Should be removed as part of https://github.com/envoyproxy/envoy/issues/8993 - "envoy.reloadable_features.http2_protocol_options.stream_error_on_invalid_http_messaging", }; RuntimeFeatures::RuntimeFeatures() { diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index 56f5a7194891..5a28c0d8f35c 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -34,18 +34,6 @@ namespace Envoy { namespace Runtime { namespace { -// Before ASSERTS were added to ensure runtime features were restricted to -// booleans, several integer features were added. isLegacyFeatures exempts -// existing integer features from the checks until they can be cleaned up. -// This includes -// envoy.reloadable_features.max_[request|response]_headers_count from -// include/envoy/http/codec.h as well as the http2_protocol_options overrides in -// source/common/http/http2/codec_impl.cc -bool isLegacyFeature(absl::string_view feature) { - return absl::StartsWith(feature, "envoy.reloadable_features.http2_protocol_options.") || - absl::StartsWith(feature, "envoy.reloadable_features.max_re"); -} - bool isRuntimeFeature(absl::string_view feature) { return RuntimeFeaturesDefaults::get().enabledByDefault(feature) || RuntimeFeaturesDefaults::get().existsButDisabled(feature); @@ -288,7 +276,7 @@ bool SnapshotImpl::featureEnabled(const std::string& key, } uint64_t SnapshotImpl::getInteger(const std::string& key, uint64_t default_value) const { - ASSERT(isLegacyFeature(key) || !isRuntimeFeature(key)); + ASSERT(!isRuntimeFeature(key)); auto entry = values_.find(key); if (entry == values_.end() || !entry->second.uint_value_) { return default_value; diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 87e6995d9975..85e95250da6c 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -17,7 +17,6 @@ #include "test/mocks/protobuf/mocks.h" #include "test/mocks/thread_local/mocks.h" #include "test/test_common/printers.h" -#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "codec_impl_test_util.h" @@ -234,10 +233,6 @@ class Http2CodecImplTest : public ::testing::TestWithParammergeValues(...) - TestScopedRuntime scoped_runtime_; }; TEST_P(Http2CodecImplTest, ShutdownNotice) { @@ -481,25 +476,6 @@ TEST_P(Http2CodecImplTest, InvalidHeadersFrameAllowed) { server_wrapper_.dispatch(Buffer::OwnedImpl(), *server_); } -TEST_P(Http2CodecImplTest, InvalidHeadersFrameOverridden) { - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.http2_protocol_options.stream_error_on_invalid_http_messaging", - "true"}}); - initialize(); - - MockStreamCallbacks request_callbacks; - request_encoder_->getStream().addCallbacks(request_callbacks); - - ON_CALL(client_connection_, write(_, _)) - .WillByDefault( - Invoke([&](Buffer::Instance& data, bool) -> void { server_wrapper_.buffer_.add(data); })); - - request_encoder_->encodeHeaders(TestHeaderMapImpl{}, true); - EXPECT_CALL(server_stream_callbacks_, onResetStream(StreamResetReason::LocalReset, _)); - EXPECT_CALL(request_callbacks, onResetStream(StreamResetReason::RemoteReset, _)); - server_wrapper_.dispatch(Buffer::OwnedImpl(), *server_); -} - TEST_P(Http2CodecImplTest, TrailingHeaders) { initialize(); @@ -1305,9 +1281,7 @@ TEST_P(Http2CodecImplTest, PingFlood) { // Verify that codec allows PING flood when mitigation is disabled TEST_P(Http2CodecImplTest, PingFloodMitigationDisabled) { - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.http2_protocol_options.max_outbound_control_frames", - "2147483647"}}); + max_outbound_control_frames_ = 2147483647; initialize(); TestHeaderMapImpl request_headers; @@ -1432,8 +1406,7 @@ TEST_P(Http2CodecImplTest, ResponseDataFlood) { // Verify that codec allows outbound DATA flood when mitigation is disabled TEST_P(Http2CodecImplTest, ResponseDataFloodMitigationDisabled) { - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.http2_protocol_options.max_outbound_frames", "2147483647"}}); + max_outbound_control_frames_ = 2147483647; initialize(); TestHeaderMapImpl request_headers; @@ -1540,9 +1513,7 @@ TEST_P(Http2CodecImplTest, PriorityFlood) { } TEST_P(Http2CodecImplTest, PriorityFloodOverride) { - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.http2_protocol_options.max_inbound_priority_frames_per_stream", - "2147483647"}}); + max_inbound_priority_frames_per_stream_ = 2147483647; priorityFlood(); EXPECT_NO_THROW(client_->sendPendingFrames()); @@ -1554,10 +1525,7 @@ TEST_P(Http2CodecImplTest, WindowUpdateFlood) { } TEST_P(Http2CodecImplTest, WindowUpdateFloodOverride) { - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.http2_protocol_options.max_inbound_window_update_frames_per_" - "data_frame_sent", - "2147483647"}}); + max_inbound_window_update_frames_per_data_frame_sent_ = 2147483647; windowUpdateFlood(); EXPECT_NO_THROW(client_->sendPendingFrames()); } @@ -1570,10 +1538,7 @@ TEST_P(Http2CodecImplTest, EmptyDataFlood) { } TEST_P(Http2CodecImplTest, EmptyDataFloodOverride) { - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.http2_protocol_options.max_consecutive_inbound_frames_with_" - "empty_payload", - "2147483647"}}); + max_consecutive_inbound_frames_with_empty_payload_ = 2147483647; Buffer::OwnedImpl data; emptyDataFlood(data); EXPECT_CALL(request_decoder_, decodeData(_, false))