From 7a7df5d8887dfe673eef51ce396feab4bff9383f Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Mon, 16 Sep 2024 14:18:19 -0400 Subject: [PATCH] router: removing an exception (#35605) Risk Level: low Testing: updated tests Docs Changes: n/a Release Notes: n/a https://github.com/envoyproxy/envoy-mobile/issues/176 Signed-off-by: Alyssa Wilk --- source/common/router/config_impl.cc | 29 ++++++++++++++++++++++------- source/common/router/config_impl.h | 11 +++++++---- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 7c70a5f42e54..a3c9b8ba2047 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -380,9 +380,20 @@ Upstream::RetryPrioritySharedPtr RetryPolicyImpl::retryPriority() const { *validation_visitor_, num_retries_); } +absl::StatusOr> InternalRedirectPolicyImpl::create( + const envoy::config::route::v3::InternalRedirectPolicy& policy_config, + ProtobufMessage::ValidationVisitor& validator, absl::string_view current_route_name) { + absl::Status creation_status = absl::OkStatus(); + auto ret = std::unique_ptr(new InternalRedirectPolicyImpl( + policy_config, validator, current_route_name, creation_status)); + RETURN_IF_NOT_OK(creation_status); + return ret; +} + InternalRedirectPolicyImpl::InternalRedirectPolicyImpl( const envoy::config::route::v3::InternalRedirectPolicy& policy_config, - ProtobufMessage::ValidationVisitor& validator, absl::string_view current_route_name) + ProtobufMessage::ValidationVisitor& validator, absl::string_view current_route_name, + absl::Status& creation_status) : current_route_name_(current_route_name), redirect_response_codes_(buildRedirectResponseCodes(policy_config)), max_internal_redirects_( @@ -397,7 +408,9 @@ InternalRedirectPolicyImpl::InternalRedirectPolicyImpl( } for (const auto& header : policy_config.response_headers_to_copy()) { if (!Http::HeaderUtility::isModifiableHeader(header)) { - throwEnvoyExceptionOrPanic(":-prefixed headers or Hosts may not be specified here."); + creation_status = + absl::InvalidArgumentError(":-prefixed headers or Hosts may not be specified here."); + return; } response_headers_to_copy_.emplace_back(header); } @@ -560,7 +573,8 @@ RouteEntryImplBase::RouteEntryImplBase(const CommonVirtualHostSharedPtr& vhost, : nullptr), hedge_policy_(buildHedgePolicy(vhost->hedgePolicy(), route.route())), internal_redirect_policy_( - buildInternalRedirectPolicy(route.route(), validator, route.name())), + THROW_OR_RETURN_VALUE(buildInternalRedirectPolicy(route.route(), validator, route.name()), + std::unique_ptr)), config_headers_( Http::HeaderUtility::buildHeaderDataVector(route.match().headers(), factory_context)), dynamic_metadata_([&]() { @@ -1186,12 +1200,13 @@ absl::StatusOr> RouteEntryImplBase::buildRetryP return nullptr; } -std::unique_ptr RouteEntryImplBase::buildInternalRedirectPolicy( +absl::StatusOr> +RouteEntryImplBase::buildInternalRedirectPolicy( const envoy::config::route::v3::RouteAction& route_config, ProtobufMessage::ValidationVisitor& validator, absl::string_view current_route_name) const { if (route_config.has_internal_redirect_policy()) { - return std::make_unique(route_config.internal_redirect_policy(), - validator, current_route_name); + return InternalRedirectPolicyImpl::create(route_config.internal_redirect_policy(), validator, + current_route_name); } envoy::config::route::v3::InternalRedirectPolicy policy_config; switch (route_config.internal_redirect_action()) { @@ -1205,7 +1220,7 @@ std::unique_ptr RouteEntryImplBase::buildInternalRed if (route_config.has_max_internal_redirects()) { *policy_config.mutable_max_internal_redirects() = route_config.max_internal_redirects(); } - return std::make_unique(policy_config, validator, current_route_name); + return InternalRedirectPolicyImpl::create(policy_config, validator, current_route_name); } RouteEntryImplBase::OptionalTimeouts RouteEntryImplBase::buildOptionalTimeouts( diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index c8dce5ceec36..0b3ffdaefdc0 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -598,11 +598,11 @@ class RouteTracingImpl : public RouteTracing { */ class InternalRedirectPolicyImpl : public InternalRedirectPolicy { public: + static absl::StatusOr> + create(const envoy::config::route::v3::InternalRedirectPolicy& policy_config, + ProtobufMessage::ValidationVisitor& validator, absl::string_view current_route_name); // Constructor that enables internal redirect with policy_config controlling the configurable // behaviors. - InternalRedirectPolicyImpl(const envoy::config::route::v3::InternalRedirectPolicy& policy_config, - ProtobufMessage::ValidationVisitor& validator, - absl::string_view current_route_name); // Default constructor that disables internal redirect. InternalRedirectPolicyImpl() = default; @@ -620,6 +620,9 @@ class InternalRedirectPolicyImpl : public InternalRedirectPolicy { bool isCrossSchemeRedirectAllowed() const override { return allow_cross_scheme_redirect_; } private: + InternalRedirectPolicyImpl(const envoy::config::route::v3::InternalRedirectPolicy& policy_config, + ProtobufMessage::ValidationVisitor& validator, + absl::string_view current_route_name, absl::Status& creation_status); absl::flat_hash_set buildRedirectResponseCodes( const envoy::config::route::v3::InternalRedirectPolicy& policy_config) const; @@ -1175,7 +1178,7 @@ class RouteEntryImplBase : public RouteEntryAndRoute, ProtobufMessage::ValidationVisitor& validation_visitor, Server::Configuration::ServerFactoryContext& factory_context) const; - std::unique_ptr + absl::StatusOr> buildInternalRedirectPolicy(const envoy::config::route::v3::RouteAction& route_config, ProtobufMessage::ValidationVisitor& validator, absl::string_view current_route_name) const;