diff --git a/source/common/router/BUILD b/source/common/router/BUILD index d9313aa228cc..f5872f5560d5 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -84,6 +84,7 @@ envoy_cc_library( "//include/envoy/thread_local:thread_local_interface", "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", + "//source/common/common:utility_lib", "//source/common/config:rds_json_lib", "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index 88ac8d4ed3eb..7ea8bb18dc41 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -11,6 +11,7 @@ #include "common/common/assert.h" #include "common/common/fmt.h" +#include "common/common/utility.h" #include "common/config/rds_json.h" #include "common/config/subscription_factory.h" #include "common/config/utility.h" @@ -97,7 +98,7 @@ Router::ConfigConstSharedPtr RdsRouteConfigProviderImpl::config() { void RdsRouteConfigProviderImpl::onConfigUpdate(const ResourceVector& resources, const std::string& version_info) { - last_updated_ = factory_context_.systemTimeSource().currentTime(); + last_updated_ = ProdSystemTimeSource::instance_.currentTime(); if (resources.empty()) { ENVOY_LOG(debug, "Missing RouteConfiguration for {} in onConfigUpdate()", route_config_name_); diff --git a/test/integration/ads_integration_test.cc b/test/integration/ads_integration_test.cc index d0a72ed610d5..c98ae664651b 100644 --- a/test/integration/ads_integration_test.cc +++ b/test/integration/ads_integration_test.cc @@ -198,9 +198,10 @@ class AdsIntegrationTest : public AdsIntegrationBaseTest, fake_upstreams_[0]->localAddress()->ip()->port())); } - envoy::api::v2::Listener buildListener(const std::string& name, const std::string& route_config) { - return TestUtility::parseYaml( - fmt::format(R"EOF( + envoy::api::v2::Listener buildListener(const std::string& name, const std::string& route_config, + const std::string& stat_prefix = "ads_test") { + return TestUtility::parseYaml(fmt::format( + R"EOF( name: {} address: socket_address: @@ -210,14 +211,14 @@ class AdsIntegrationTest : public AdsIntegrationBaseTest, filters: - name: envoy.http_connection_manager config: - stat_prefix: ads_test + stat_prefix: {} codec_type: HTTP2 rds: route_config_name: {} config_source: {{ ads: {{}} }} http_filters: [{{ name: envoy.router }}] )EOF", - name, Network::Test::getLoopbackAddressString(ipVersion()), route_config)); + name, Network::Test::getLoopbackAddressString(ipVersion()), stat_prefix, route_config)); } envoy::api::v2::RouteConfiguration buildRouteConfig(const std::string& name, @@ -471,6 +472,37 @@ TEST_P(AdsIntegrationTest, Failure) { makeSingleRequest(); } +// Regression test for the use-after-free crash when processing RDS update (#3953). +TEST_P(AdsIntegrationTest, RdsCrash) { + initialize(); + + // Send initial configuration. + sendDiscoveryResponse(Config::TypeUrl::get().Cluster, + {buildCluster("cluster_0")}, "1"); + sendDiscoveryResponse( + Config::TypeUrl::get().ClusterLoadAssignment, {buildClusterLoadAssignment("cluster_0")}, "1"); + sendDiscoveryResponse( + Config::TypeUrl::get().Listener, {buildListener("listener_0", "route_config_0")}, "1"); + sendDiscoveryResponse( + Config::TypeUrl::get().RouteConfiguration, {buildRouteConfig("route_config_0", "cluster_0")}, + "1"); + test_server_->waitForCounterGe("listener_manager.listener_create_success", 1); + + // Validate that we can process a request. + makeSingleRequest(); + + // Update existing LDS (change stat_prefix). + sendDiscoveryResponse( + Config::TypeUrl::get().Listener, {buildListener("listener_0", "route_config_0", "rds_crash")}, + "2"); + test_server_->waitForCounterGe("listener_manager.listener_create_success", 2); + + // Update existing RDS (no changes). + sendDiscoveryResponse( + Config::TypeUrl::get().RouteConfiguration, {buildRouteConfig("route_config_0", "cluster_0")}, + "2"); +} + class AdsFailIntegrationTest : public AdsIntegrationBaseTest, public Grpc::GrpcClientIntegrationParamTest { public: