diff --git a/BUILD b/BUILD index 576542619dc..484138207e5 100644 --- a/BUILD +++ b/BUILD @@ -956,6 +956,16 @@ grpc_cc_library( tags = ["grpc-autodeps"], ) +grpc_cc_library( + name = "unique_type_name", + hdrs = ["src/core/lib/gprpp/unique_type_name.h"], + external_deps = [ + "absl/strings", + ], + language = "c++", + deps = ["useful"], +) + grpc_cc_library( name = "gpr_codegen", language = "c++", @@ -2921,6 +2931,7 @@ grpc_cc_library( "slice_refcount", "sockaddr_utils", "time", + "unique_type_name", "uri_parser", "useful", "xds_orca_service_upb", @@ -3546,6 +3557,7 @@ grpc_cc_library( "ref_counted_ptr", "server_address", "time", + "unique_type_name", ], ) @@ -4922,6 +4934,7 @@ grpc_cc_library( "resource_quota_trace", "try_seq", "tsi_base", + "unique_type_name", ], ) @@ -5380,6 +5393,7 @@ grpc_cc_library( "slice", "sockaddr_utils", "tcp_connect_handshaker", + "unique_type_name", "uri_parser", ], ) @@ -5426,6 +5440,7 @@ grpc_cc_library( "slice", "sockaddr_utils", "time", + "unique_type_name", "uri_parser", "useful", ], diff --git a/CMakeLists.txt b/CMakeLists.txt index b63f67c8684..16c47bf2952 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1172,6 +1172,7 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx try_join_test) add_dependencies(buildtests_cxx try_seq_metadata_test) add_dependencies(buildtests_cxx try_seq_test) + add_dependencies(buildtests_cxx unique_type_name_test) add_dependencies(buildtests_cxx unknown_frame_bad_client_test) add_dependencies(buildtests_cxx uri_parser_test) add_dependencies(buildtests_cxx useful_test) @@ -17117,6 +17118,42 @@ target_link_libraries(try_seq_test ) +endif() +if(gRPC_BUILD_TESTS) + +add_executable(unique_type_name_test + test/core/gprpp/unique_type_name_test.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + +target_include_directories(unique_type_name_test + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_CURRENT_SOURCE_DIR}/include + ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} + ${_gRPC_RE2_INCLUDE_DIR} + ${_gRPC_SSL_INCLUDE_DIR} + ${_gRPC_UPB_GENERATED_DIR} + ${_gRPC_UPB_GRPC_GENERATED_DIR} + ${_gRPC_UPB_INCLUDE_DIR} + ${_gRPC_XXHASH_INCLUDE_DIR} + ${_gRPC_ZLIB_INCLUDE_DIR} + third_party/googletest/googletest/include + third_party/googletest/googletest + third_party/googletest/googlemock/include + third_party/googletest/googlemock + ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(unique_type_name_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + absl::str_format + absl::strings +) + + endif() if(gRPC_BUILD_TESTS) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index cf6b73308b0..7fbb0c63a98 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -744,6 +744,7 @@ libs: - src/core/lib/gprpp/single_set_ptr.h - src/core/lib/gprpp/table.h - src/core/lib/gprpp/time.h + - src/core/lib/gprpp/unique_type_name.h - src/core/lib/http/format_request.h - src/core/lib/http/httpcli.h - src/core/lib/http/httpcli_ssl_credentials.h @@ -1927,6 +1928,7 @@ libs: - src/core/lib/gprpp/single_set_ptr.h - src/core/lib/gprpp/table.h - src/core/lib/gprpp/time.h + - src/core/lib/gprpp/unique_type_name.h - src/core/lib/http/format_request.h - src/core/lib/http/httpcli.h - src/core/lib/http/parser.h @@ -8493,6 +8495,19 @@ targets: - absl/types:variant - absl/utility:utility uses_polling: false +- name: unique_type_name_test + gtest: true + build: test + language: c++ + headers: + - src/core/lib/gpr/useful.h + - src/core/lib/gprpp/unique_type_name.h + src: + - test/core/gprpp/unique_type_name_test.cc + deps: + - absl/strings:str_format + - absl/strings:strings + uses_polling: false - name: unknown_frame_bad_client_test gtest: true build: test diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index de3ab92b1b5..ae6c3b4afd6 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -713,6 +713,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/thd.h', 'src/core/lib/gprpp/time.h', 'src/core/lib/gprpp/time_util.h', + 'src/core/lib/gprpp/unique_type_name.h', 'src/core/lib/http/format_request.h', 'src/core/lib/http/httpcli.h', 'src/core/lib/http/httpcli_ssl_credentials.h', @@ -1528,6 +1529,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/thd.h', 'src/core/lib/gprpp/time.h', 'src/core/lib/gprpp/time_util.h', + 'src/core/lib/gprpp/unique_type_name.h', 'src/core/lib/http/format_request.h', 'src/core/lib/http/httpcli.h', 'src/core/lib/http/httpcli_ssl_credentials.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 6e71e8b89ba..497c0cac902 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1124,6 +1124,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/time.h', 'src/core/lib/gprpp/time_util.cc', 'src/core/lib/gprpp/time_util.h', + 'src/core/lib/gprpp/unique_type_name.h', 'src/core/lib/http/format_request.cc', 'src/core/lib/http/format_request.h', 'src/core/lib/http/httpcli.cc', @@ -2127,6 +2128,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/thd.h', 'src/core/lib/gprpp/time.h', 'src/core/lib/gprpp/time_util.h', + 'src/core/lib/gprpp/unique_type_name.h', 'src/core/lib/http/format_request.h', 'src/core/lib/http/httpcli.h', 'src/core/lib/http/httpcli_ssl_credentials.h', diff --git a/grpc.gemspec b/grpc.gemspec index 9e920ac3258..d7efe2b87a9 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1037,6 +1037,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/gprpp/time.h ) s.files += %w( src/core/lib/gprpp/time_util.cc ) s.files += %w( src/core/lib/gprpp/time_util.h ) + s.files += %w( src/core/lib/gprpp/unique_type_name.h ) s.files += %w( src/core/lib/http/format_request.cc ) s.files += %w( src/core/lib/http/format_request.h ) s.files += %w( src/core/lib/http/httpcli.cc ) diff --git a/package.xml b/package.xml index 6607046e1c7..1f6973713c2 100644 --- a/package.xml +++ b/package.xml @@ -1019,6 +1019,7 @@ + diff --git a/src/core/ext/filters/client_channel/lb_policy/oob_backend_metric.cc b/src/core/ext/filters/client_channel/lb_policy/oob_backend_metric.cc index e9b79473690..2cce61899e6 100644 --- a/src/core/ext/filters/client_channel/lb_policy/oob_backend_metric.cc +++ b/src/core/ext/filters/client_channel/lb_policy/oob_backend_metric.cc @@ -55,6 +55,7 @@ #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/error.h" #include "src/core/lib/iomgr/exec_ctx.h" @@ -68,8 +69,6 @@ namespace { TraceFlag grpc_orca_client_trace(false, "orca_client"); -constexpr char kProducerType[] = "orca"; - class OrcaWatcher; // This producer is registered with a subchannel. It creates a @@ -81,7 +80,12 @@ class OrcaProducer : public Subchannel::DataProducerInterface { void Orphan() override; - const char* type() override { return kProducerType; } + static UniqueTypeName Type() { + static UniqueTypeName::Factory kFactory("orca"); + return kFactory.Create(); + } + + UniqueTypeName type() const override { return Type(); } // Adds and removes watchers. void AddWatcher(OrcaWatcher* watcher); @@ -383,8 +387,8 @@ OrcaWatcher::~OrcaWatcher() { void OrcaWatcher::SetSubchannel(Subchannel* subchannel) { // Check if our producer is already registered with the subchannel. // If not, create a new one, which will register itself with the subchannel. - auto* p = - static_cast(subchannel->GetDataProducer(kProducerType)); + auto* p = static_cast( + subchannel->GetDataProducer(OrcaProducer::Type())); if (p != nullptr) producer_ = p->RefIfNonZero(); if (producer_ == nullptr) { producer_ = MakeRefCounted(subchannel->Ref()); diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc index f025147c9f7..5453946587c 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc @@ -55,6 +55,7 @@ #include "src/core/lib/gprpp/orphanable.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/iomgr/error.h" #include "src/core/lib/iomgr/pollset_set.h" #include "src/core/lib/iomgr/work_serializer.h" diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 7b0210757e7..ea2e2177618 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -25,6 +25,7 @@ #include #include #include +#include #include #include "absl/status/statusor.h" @@ -838,7 +839,7 @@ void Subchannel::RemoveDataProducer(DataProducerInterface* data_producer) { } Subchannel::DataProducerInterface* Subchannel::GetDataProducer( - const char* type) { + UniqueTypeName type) { MutexLock lock(&mu_); auto it = data_producer_map_.find(type); if (it == data_producer_map_.end()) return nullptr; diff --git a/src/core/ext/filters/client_channel/subchannel.h b/src/core/ext/filters/client_channel/subchannel.h index 528b6d67ad6..23b68f222e5 100644 --- a/src/core/ext/filters/client_channel/subchannel.h +++ b/src/core/ext/filters/client_channel/subchannel.h @@ -46,6 +46,7 @@ #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/iomgr/call_combiner.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/error.h" @@ -217,7 +218,7 @@ class Subchannel : public DualRefCounted { // contents for uniqueness; all instances for a given implementation // are expected to return the same string *instance*, not just the // same string contents. - virtual const char* type() = 0; + virtual UniqueTypeName type() const = 0; }; // Creates a subchannel. @@ -292,7 +293,7 @@ class Subchannel : public DualRefCounted { ABSL_LOCKS_EXCLUDED(mu_); void RemoveDataProducer(DataProducerInterface* data_producer) ABSL_LOCKS_EXCLUDED(mu_); - DataProducerInterface* GetDataProducer(const char* type) + DataProducerInterface* GetDataProducer(UniqueTypeName type) ABSL_LOCKS_EXCLUDED(mu_); private: @@ -432,7 +433,7 @@ class Subchannel : public DualRefCounted { int keepalive_time_ ABSL_GUARDED_BY(mu_) = -1; // Data producer map. - std::map data_producer_map_ + std::map data_producer_map_ ABSL_GUARDED_BY(mu_); }; diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc index b3741506ef5..ff939cacb13 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc @@ -51,6 +51,7 @@ #include "src/core/lib/debug/trace.h" #include "src/core/lib/gprpp/debug_location.h" #include "src/core/lib/gprpp/orphanable.h" +#include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/iomgr/endpoint.h" #include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/iomgr/resolved_address.h" @@ -411,7 +412,7 @@ grpc_channel* grpc_channel_create_from_fd(const char* target, int fd, (target, fd, creds, args)); // For now, we only support insecure channel credentials. if (creds == nullptr || - creds->type() != grpc_core::InsecureServerCredentials::Type()) { + creds->type() != grpc_core::InsecureCredentials::Type()) { return grpc_lame_client_channel_create( target, GRPC_STATUS_INTERNAL, "Failed to create client channel due to invalid creds"); diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index 47170430cf5..cd21d92760b 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -62,6 +62,7 @@ #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/sync.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/endpoint.h" #include "src/core/lib/iomgr/iomgr_fwd.h" @@ -1012,7 +1013,7 @@ grpc_channel_args* ModifyArgsForConnection(grpc_channel_args* args, if (security_connector == nullptr) { *error = GRPC_ERROR_CREATE_FROM_CPP_STRING( absl::StrCat("Unable to create secure server with credentials of type ", - server_credentials->type())); + server_credentials->type().name())); return args; } grpc_arg arg_to_add = @@ -1061,7 +1062,7 @@ int grpc_server_add_http2_port(grpc_server* server, const char* addr, if (sc == nullptr) { err = GRPC_ERROR_CREATE_FROM_CPP_STRING(absl::StrCat( "Unable to create secure server with credentials of type ", - creds->type())); + creds->type().name())); goto done; } grpc_arg args_to_add[2]; diff --git a/src/core/ext/xds/certificate_provider_store.cc b/src/core/ext/xds/certificate_provider_store.cc index 83001972df4..4e281a978c2 100644 --- a/src/core/ext/xds/certificate_provider_store.cc +++ b/src/core/ext/xds/certificate_provider_store.cc @@ -28,8 +28,10 @@ namespace grpc_core { // CertificateProviderStore::CertificateProviderWrapper // -const char* CertificateProviderStore::CertificateProviderWrapper::type() const { - return "Wrapper"; +UniqueTypeName CertificateProviderStore::CertificateProviderWrapper::type() + const { + static UniqueTypeName::Factory kFactory("Wrapper"); + return kFactory.Create(); } // If a certificate provider is created, the CertificateProviderStore diff --git a/src/core/ext/xds/certificate_provider_store.h b/src/core/ext/xds/certificate_provider_store.h index 9618625cfa3..eadc318902a 100644 --- a/src/core/ext/xds/certificate_provider_store.h +++ b/src/core/ext/xds/certificate_provider_store.h @@ -90,7 +90,7 @@ class CertificateProviderStore static_cast(this), other); } - const char* type() const override; + UniqueTypeName type() const override; absl::string_view key() const { return key_; } diff --git a/src/core/ext/xds/xds_certificate_provider.cc b/src/core/ext/xds/xds_certificate_provider.cc index 903d4cf3c11..f7dca9bc4bc 100644 --- a/src/core/ext/xds/xds_certificate_provider.cc +++ b/src/core/ext/xds/xds_certificate_provider.cc @@ -263,7 +263,10 @@ XdsCertificateProvider::~XdsCertificateProvider() { distributor_->SetWatchStatusCallback(nullptr); } -const char* XdsCertificateProvider::type() const { return "Xds"; } +UniqueTypeName XdsCertificateProvider::type() const { + static UniqueTypeName::Factory kFactory("Xds"); + return kFactory.Create(); +} bool XdsCertificateProvider::ProvidesRootCerts(const std::string& cert_name) { MutexLock lock(&mu_); diff --git a/src/core/ext/xds/xds_certificate_provider.h b/src/core/ext/xds/xds_certificate_provider.h index d0f585483f1..2631713d235 100644 --- a/src/core/ext/xds/xds_certificate_provider.h +++ b/src/core/ext/xds/xds_certificate_provider.h @@ -39,7 +39,7 @@ class XdsCertificateProvider : public grpc_tls_certificate_provider { return distributor_; } - const char* type() const override; + UniqueTypeName type() const override; bool ProvidesRootCerts(const std::string& cert_name); void UpdateRootCertNameAndDistributor( diff --git a/src/core/lib/gprpp/unique_type_name.h b/src/core/lib/gprpp/unique_type_name.h new file mode 100644 index 00000000000..884fe4147cf --- /dev/null +++ b/src/core/lib/gprpp/unique_type_name.h @@ -0,0 +1,104 @@ +// +// Copyright 2022 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#ifndef GRPC_CORE_LIB_GPRPP_UNIQUE_TYPE_NAME_H +#define GRPC_CORE_LIB_GPRPP_UNIQUE_TYPE_NAME_H + +#include + +#include + +#include "absl/strings/string_view.h" + +#include "src/core/lib/gpr/useful.h" + +// Provides a type name that is unique by instance rather than by +// string content. This is useful in cases where there are different +// implementations of a given interface that need to be differentiated from +// each other for down-casting purposes, where it is undesirable to provide +// a registry to avoid name collisions. +// +// Expected usage: +/* +// Interface has a virtual method that returns a UniqueTypeName. +class Interface { + public: + virtual ~Interface() = default; + virtual UniqueTypeName type() const = 0; +}; + +// Implementation uses a static factory instance to return the same +// UniqueTypeName for every instance. +class FooImplementation : public Interface { + public: + UniqueTypeName type() const override { + static UniqueTypeName::Factory kFactory("Foo"); + return kFactory.Create(); + } +}; +*/ + +namespace grpc_core { + +class UniqueTypeName { + public: + // Factory class. There should be a single static instance of this + // for each unique type name. + class Factory { + public: + explicit Factory(absl::string_view name) : name_(new std::string(name)) {} + + Factory(const Factory&) = delete; + Factory& operator=(const Factory&) = delete; + + UniqueTypeName Create() { return UniqueTypeName(*name_); } + + private: + std::string* name_; + }; + + // Copyable. + UniqueTypeName(const UniqueTypeName& other) : name_(other.name_) {} + UniqueTypeName& operator=(const UniqueTypeName& other) { + name_ = other.name_; + return *this; + } + + bool operator==(const UniqueTypeName& other) const { + return name_.data() == other.name_.data(); + } + bool operator!=(const UniqueTypeName& other) const { + return name_.data() != other.name_.data(); + } + bool operator<(const UniqueTypeName& other) const { + return name_.data() < other.name_.data(); + } + + int Compare(const UniqueTypeName& other) const { + return QsortCompare(name_.data(), other.name_.data()); + } + + absl::string_view name() const { return name_; } + + private: + explicit UniqueTypeName(absl::string_view name) : name_(name) {} + + absl::string_view name_; +}; + +} // namespace grpc_core + +#endif // GRPC_CORE_LIB_GPRPP_UNIQUE_TYPE_NAME_H diff --git a/src/core/lib/http/httpcli_security_connector.cc b/src/core/lib/http/httpcli_security_connector.cc index d652eeeb26f..44701d805cc 100644 --- a/src/core/lib/http/httpcli_security_connector.cc +++ b/src/core/lib/http/httpcli_security_connector.cc @@ -178,7 +178,10 @@ class HttpRequestSSLCredentials : public grpc_channel_credentials { return Ref(); } - const char* type() const override { return "HttpRequestSSL"; } + UniqueTypeName type() const override { + static UniqueTypeName::Factory kFactory("HttpRequestSSL"); + return kFactory.Create(); + } private: int cmp_impl(const grpc_channel_credentials* /* other */) const override { diff --git a/src/core/lib/security/credentials/alts/alts_credentials.cc b/src/core/lib/security/credentials/alts/alts_credentials.cc index 037c8b4f543..3b0a8f8d076 100644 --- a/src/core/lib/security/credentials/alts/alts_credentials.cc +++ b/src/core/lib/security/credentials/alts/alts_credentials.cc @@ -56,7 +56,10 @@ grpc_alts_credentials::create_security_connector( this->Ref(), std::move(call_creds), target_name); } -const char* grpc_alts_credentials::type() const { return "Alts"; } +grpc_core::UniqueTypeName grpc_alts_credentials::type() const { + static grpc_core::UniqueTypeName::Factory kFactory("Alts"); + return kFactory.Create(); +} grpc_alts_server_credentials::grpc_alts_server_credentials( const grpc_alts_credentials_options* options, @@ -79,7 +82,10 @@ grpc_alts_server_credentials::~grpc_alts_server_credentials() { gpr_free(handshaker_service_url_); } -const char* grpc_alts_server_credentials::type() const { return "Alts"; } +grpc_core::UniqueTypeName grpc_alts_server_credentials::type() const { + static grpc_core::UniqueTypeName::Factory kFactory("Alts"); + return kFactory.Create(); +} grpc_channel_credentials* grpc_alts_credentials_create_customized( const grpc_alts_credentials_options* options, diff --git a/src/core/lib/security/credentials/alts/alts_credentials.h b/src/core/lib/security/credentials/alts/alts_credentials.h index 1a333b9ea8f..86b2dd8ac05 100644 --- a/src/core/lib/security/credentials/alts/alts_credentials.h +++ b/src/core/lib/security/credentials/alts/alts_credentials.h @@ -39,7 +39,7 @@ class grpc_alts_credentials final : public grpc_channel_credentials { const char* target_name, const grpc_channel_args* args, grpc_channel_args** new_args) override; - const char* type() const override; + grpc_core::UniqueTypeName type() const override; const grpc_alts_credentials_options* options() const { return options_; } grpc_alts_credentials_options* mutable_options() { return options_; } @@ -66,7 +66,7 @@ class grpc_alts_server_credentials final : public grpc_server_credentials { grpc_core::RefCountedPtr create_security_connector(const grpc_channel_args* /* args */) override; - const char* type() const override; + grpc_core::UniqueTypeName type() const override; const grpc_alts_credentials_options* options() const { return options_; } grpc_alts_credentials_options* mutable_options() { return options_; } diff --git a/src/core/lib/security/credentials/composite/composite_credentials.cc b/src/core/lib/security/credentials/composite/composite_credentials.cc index 3a62f6f9512..4981e41f893 100644 --- a/src/core/lib/security/credentials/composite/composite_credentials.cc +++ b/src/core/lib/security/credentials/composite/composite_credentials.cc @@ -41,8 +41,9 @@ // grpc_composite_channel_credentials // -const char* grpc_composite_channel_credentials::type() const { - return "Composite"; +grpc_core::UniqueTypeName grpc_composite_channel_credentials::type() const { + static grpc_core::UniqueTypeName::Factory kFactory("Composite"); + return kFactory.Create(); } /* -- Composite call credentials. -- */ @@ -60,7 +61,10 @@ grpc_composite_call_credentials::GetRequestMetadata( }); } -const char* grpc_composite_call_credentials::Type() { return "Composite"; } +grpc_core::UniqueTypeName grpc_composite_call_credentials::Type() { + static grpc_core::UniqueTypeName::Factory kFactory("Composite"); + return kFactory.Create(); +} std::string grpc_composite_call_credentials::debug_string() { std::vector outputs; diff --git a/src/core/lib/security/credentials/composite/composite_credentials.h b/src/core/lib/security/credentials/composite/composite_credentials.h index 121c353ec89..3977e91fddd 100644 --- a/src/core/lib/security/credentials/composite/composite_credentials.h +++ b/src/core/lib/security/credentials/composite/composite_credentials.h @@ -56,7 +56,7 @@ class grpc_composite_channel_credentials : public grpc_channel_credentials { return inner_creds_->update_arguments(args); } - const char* type() const override; + grpc_core::UniqueTypeName type() const override; const grpc_channel_credentials* inner_creds() const { return inner_creds_.get(); @@ -99,9 +99,9 @@ class grpc_composite_call_credentials : public grpc_call_credentials { const CallCredentialsList& inner() const { return inner_; } std::string debug_string() override; - static const char* Type(); + static grpc_core::UniqueTypeName Type(); - const char* type() const override { return Type(); } + grpc_core::UniqueTypeName type() const override { return Type(); } private: int cmp_impl(const grpc_call_credentials* other) const override { diff --git a/src/core/lib/security/credentials/credentials.h b/src/core/lib/security/credentials/credentials.h index 536c00c90d0..ae55a1e72d0 100644 --- a/src/core/lib/security/credentials/credentials.h +++ b/src/core/lib/security/credentials/credentials.h @@ -31,6 +31,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gprpp/ref_counted.h" +#include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/iomgr/polling_entity.h" #include "src/core/lib/promise/arena_promise.h" #include "src/core/lib/security/context/security_context.h" @@ -139,9 +140,7 @@ struct grpc_channel_credentials // as equal (assuming other channel args match). int cmp(const grpc_channel_credentials* other) const { GPR_ASSERT(other != nullptr); - // Intentionally uses grpc_core::QsortCompare instead of strcmp as a safety - // against different grpc_channel_credentials types using the same name. - int r = grpc_core::QsortCompare(type(), other->type()); + int r = type().Compare(other->type()); if (r != 0) return r; return cmp_impl(other); } @@ -150,7 +149,7 @@ struct grpc_channel_credentials // implementation for down-casting purposes. Every creds implementation should // use a unique string instance, which should be returned by all instances of // that creds implementation. - virtual const char* type() const = 0; + virtual grpc_core::UniqueTypeName type() const = 0; private: // Implementation for `cmp` method intended to be overridden by subclasses. @@ -222,9 +221,7 @@ struct grpc_call_credentials // credentials as effectively the same.. int cmp(const grpc_call_credentials* other) const { GPR_ASSERT(other != nullptr); - // Intentionally uses grpc_core::QsortCompare instead of strcmp as a safety - // against different grpc_call_credentials types using the same name. - int r = grpc_core::QsortCompare(type(), other->type()); + int r = type().Compare(other->type()); if (r != 0) return r; return cmp_impl(other); } @@ -237,7 +234,7 @@ struct grpc_call_credentials // implementation for down-casting purposes. Every creds implementation should // use a unique string instance, which should be returned by all instances of // that creds implementation. - virtual const char* type() const = 0; + virtual grpc_core::UniqueTypeName type() const = 0; private: // Implementation for `cmp` method intended to be overridden by subclasses. @@ -266,7 +263,7 @@ struct grpc_server_credentials virtual grpc_core::RefCountedPtr create_security_connector(const grpc_channel_args* args) = 0; - virtual const char* type() const = 0; + virtual grpc_core::UniqueTypeName type() const = 0; const grpc_auth_metadata_processor& auth_metadata_processor() const { return processor_; diff --git a/src/core/lib/security/credentials/fake/fake_credentials.cc b/src/core/lib/security/credentials/fake/fake_credentials.cc index 6403de3ea08..ebb5a1289ae 100644 --- a/src/core/lib/security/credentials/fake/fake_credentials.cc +++ b/src/core/lib/security/credentials/fake/fake_credentials.cc @@ -35,6 +35,7 @@ /* -- Fake transport security credentials. -- */ namespace { + class grpc_fake_channel_credentials final : public grpc_channel_credentials { public: grpc_core::RefCountedPtr @@ -46,7 +47,10 @@ class grpc_fake_channel_credentials final : public grpc_channel_credentials { this->Ref(), std::move(call_creds), target, args); } - const char* type() const override { return "Fake"; } + grpc_core::UniqueTypeName type() const override { + static grpc_core::UniqueTypeName::Factory kFactory("Fake"); + return kFactory.Create(); + } private: int cmp_impl(const grpc_channel_credentials* other) const override { @@ -63,7 +67,10 @@ class grpc_fake_server_credentials final : public grpc_server_credentials { return grpc_fake_server_security_connector_create(this->Ref()); } - const char* type() const override { return "Fake"; } + grpc_core::UniqueTypeName type() const override { + static grpc_core::UniqueTypeName::Factory kFactory("Fake"); + return kFactory.Create(); + } }; } // namespace @@ -101,7 +108,10 @@ grpc_md_only_test_credentials::GetRequestMetadata( return grpc_core::Immediate(std::move(initial_metadata)); } -const char* grpc_md_only_test_credentials::Type() { return "MdOnlyTest"; } +grpc_core::UniqueTypeName grpc_md_only_test_credentials::Type() { + static grpc_core::UniqueTypeName::Factory kFactory("MdOnlyTest"); + return kFactory.Create(); +} grpc_call_credentials* grpc_md_only_test_credentials_create( const char* md_key, const char* md_value) { diff --git a/src/core/lib/security/credentials/fake/fake_credentials.h b/src/core/lib/security/credentials/fake/fake_credentials.h index 299f58c4626..032194376b6 100644 --- a/src/core/lib/security/credentials/fake/fake_credentials.h +++ b/src/core/lib/security/credentials/fake/fake_credentials.h @@ -70,9 +70,9 @@ class grpc_md_only_test_credentials : public grpc_call_credentials { std::string debug_string() override { return "MD only Test Credentials"; } - static const char* Type(); + static grpc_core::UniqueTypeName Type(); - const char* type() const override { return Type(); } + grpc_core::UniqueTypeName type() const override { return Type(); } private: int cmp_impl(const grpc_call_credentials* other) const override { diff --git a/src/core/lib/security/credentials/google_default/google_default_credentials.cc b/src/core/lib/security/credentials/google_default/google_default_credentials.cc index 9cee1691e08..bc91f012cfa 100644 --- a/src/core/lib/security/credentials/google_default/google_default_credentials.cc +++ b/src/core/lib/security/credentials/google_default/google_default_credentials.cc @@ -131,8 +131,10 @@ grpc_google_default_channel_credentials::update_arguments( return args.SetIfUnset(GRPC_ARG_DNS_ENABLE_SRV_QUERIES, true); } -const char* grpc_google_default_channel_credentials::type() const { - return "GoogleDefault"; +grpc_core::UniqueTypeName grpc_google_default_channel_credentials::type() + const { + static grpc_core::UniqueTypeName::Factory kFactory("GoogleDefault"); + return kFactory.Create(); } static void on_metadata_server_detection_http_response( diff --git a/src/core/lib/security/credentials/google_default/google_default_credentials.h b/src/core/lib/security/credentials/google_default/google_default_credentials.h index ef6867d460b..0a4016a33c8 100644 --- a/src/core/lib/security/credentials/google_default/google_default_credentials.h +++ b/src/core/lib/security/credentials/google_default/google_default_credentials.h @@ -58,7 +58,7 @@ class grpc_google_default_channel_credentials grpc_core::ChannelArgs update_arguments(grpc_core::ChannelArgs args) override; - const char* type() const override; + grpc_core::UniqueTypeName type() const override; const grpc_channel_credentials* alts_creds() const { return alts_creds_.get(); diff --git a/src/core/lib/security/credentials/iam/iam_credentials.cc b/src/core/lib/security/credentials/iam/iam_credentials.cc index f2f5a2a39d6..95ee77f6e27 100644 --- a/src/core/lib/security/credentials/iam/iam_credentials.cc +++ b/src/core/lib/security/credentials/iam/iam_credentials.cc @@ -56,7 +56,10 @@ grpc_google_iam_credentials::grpc_google_iam_credentials( "GoogleIAMCredentials{Token:%s,AuthoritySelector:%s}", token != nullptr ? "present" : "absent", authority_selector)) {} -const char* grpc_google_iam_credentials::Type() { return "Iam"; } +grpc_core::UniqueTypeName grpc_google_iam_credentials::Type() { + static grpc_core::UniqueTypeName::Factory kFactory("Iam"); + return kFactory.Create(); +} grpc_call_credentials* grpc_google_iam_credentials_create( const char* token, const char* authority_selector, void* reserved) { diff --git a/src/core/lib/security/credentials/iam/iam_credentials.h b/src/core/lib/security/credentials/iam/iam_credentials.h index e717fd190b9..4d42df3ba24 100644 --- a/src/core/lib/security/credentials/iam/iam_credentials.h +++ b/src/core/lib/security/credentials/iam/iam_credentials.h @@ -36,9 +36,9 @@ class grpc_google_iam_credentials : public grpc_call_credentials { std::string debug_string() override { return debug_string_; } - static const char* Type(); + static grpc_core::UniqueTypeName Type(); - const char* type() const override { return Type(); } + grpc_core::UniqueTypeName type() const override { return Type(); } private: int cmp_impl(const grpc_call_credentials* other) const override { diff --git a/src/core/lib/security/credentials/insecure/insecure_credentials.cc b/src/core/lib/security/credentials/insecure/insecure_credentials.cc index 55dff87befa..90916898f37 100644 --- a/src/core/lib/security/credentials/insecure/insecure_credentials.cc +++ b/src/core/lib/security/credentials/insecure/insecure_credentials.cc @@ -33,7 +33,10 @@ InsecureCredentials::create_security_connector( Ref(), std::move(request_metadata_creds)); } -const char* InsecureCredentials::Type() { return "Insecure"; } +UniqueTypeName InsecureCredentials::Type() { + static UniqueTypeName::Factory kFactory("Insecure"); + return kFactory.Create(); +} int InsecureCredentials::cmp_impl( const grpc_channel_credentials* /* other */) const { @@ -47,7 +50,10 @@ InsecureServerCredentials::create_security_connector( return MakeRefCounted(Ref()); } -const char* InsecureServerCredentials::Type() { return "Insecure"; } +UniqueTypeName InsecureServerCredentials::Type() { + static auto* kFactory = new UniqueTypeName::Factory("Insecure"); + return kFactory->Create(); +} } // namespace grpc_core diff --git a/src/core/lib/security/credentials/insecure/insecure_credentials.h b/src/core/lib/security/credentials/insecure/insecure_credentials.h index ff0139feccd..be947cd95ad 100644 --- a/src/core/lib/security/credentials/insecure/insecure_credentials.h +++ b/src/core/lib/security/credentials/insecure/insecure_credentials.h @@ -34,9 +34,9 @@ class InsecureCredentials final : public grpc_channel_credentials { const char* /* target_name */, const grpc_channel_args* /* args */, grpc_channel_args** /* new_args */) override; - static const char* Type(); + static UniqueTypeName Type(); - const char* type() const override { return Type(); } + UniqueTypeName type() const override { return Type(); } private: int cmp_impl(const grpc_channel_credentials* other) const override; @@ -47,9 +47,9 @@ class InsecureServerCredentials final : public grpc_server_credentials { RefCountedPtr create_security_connector( const grpc_channel_args* /* args */) override; - static const char* Type(); + static UniqueTypeName Type(); - const char* type() const override { return Type(); } + UniqueTypeName type() const override { return Type(); } }; } // namespace grpc_core diff --git a/src/core/lib/security/credentials/jwt/jwt_credentials.cc b/src/core/lib/security/credentials/jwt/jwt_credentials.cc index 2115a083d7d..30b6cc79c18 100644 --- a/src/core/lib/security/credentials/jwt/jwt_credentials.cc +++ b/src/core/lib/security/credentials/jwt/jwt_credentials.cc @@ -118,8 +118,9 @@ grpc_service_account_jwt_access_credentials:: gpr_mu_init(&cache_mu_); } -const char* grpc_service_account_jwt_access_credentials::Type() { - return "Jwt"; +grpc_core::UniqueTypeName grpc_service_account_jwt_access_credentials::Type() { + static grpc_core::UniqueTypeName::Factory kFactory("Jwt"); + return kFactory.Create(); } grpc_core::RefCountedPtr diff --git a/src/core/lib/security/credentials/jwt/jwt_credentials.h b/src/core/lib/security/credentials/jwt/jwt_credentials.h index a296fc3a222..d873ba0e72a 100644 --- a/src/core/lib/security/credentials/jwt/jwt_credentials.h +++ b/src/core/lib/security/credentials/jwt/jwt_credentials.h @@ -52,9 +52,9 @@ class grpc_service_account_jwt_access_credentials static_cast(gpr_timespec_to_micros(jwt_lifetime_))))); }; - static const char* Type(); + static grpc_core::UniqueTypeName Type(); - const char* type() const override { return Type(); } + grpc_core::UniqueTypeName type() const override { return Type(); } private: int cmp_impl(const grpc_call_credentials* other) const override { diff --git a/src/core/lib/security/credentials/local/local_credentials.cc b/src/core/lib/security/credentials/local/local_credentials.cc index b03f3865820..2d4d4c6e3d1 100644 --- a/src/core/lib/security/credentials/local/local_credentials.cc +++ b/src/core/lib/security/credentials/local/local_credentials.cc @@ -36,7 +36,10 @@ grpc_local_credentials::create_security_connector( this->Ref(), std::move(request_metadata_creds), args, target_name); } -const char* grpc_local_credentials::type() const { return "Local"; } +grpc_core::UniqueTypeName grpc_local_credentials::type() const { + static grpc_core::UniqueTypeName::Factory kFactory("Local"); + return kFactory.Create(); +} grpc_core::RefCountedPtr grpc_local_server_credentials::create_security_connector( @@ -44,7 +47,10 @@ grpc_local_server_credentials::create_security_connector( return grpc_local_server_security_connector_create(this->Ref()); } -const char* grpc_local_server_credentials::type() const { return "Local"; } +grpc_core::UniqueTypeName grpc_local_server_credentials::type() const { + static grpc_core::UniqueTypeName::Factory kFactory("Local"); + return kFactory.Create(); +} grpc_local_credentials::grpc_local_credentials( grpc_local_connect_type connect_type) diff --git a/src/core/lib/security/credentials/local/local_credentials.h b/src/core/lib/security/credentials/local/local_credentials.h index 5e1bb6a11ff..187d23a69a9 100644 --- a/src/core/lib/security/credentials/local/local_credentials.h +++ b/src/core/lib/security/credentials/local/local_credentials.h @@ -37,7 +37,7 @@ class grpc_local_credentials final : public grpc_channel_credentials { const char* target_name, const grpc_channel_args* args, grpc_channel_args** new_args) override; - const char* type() const override; + grpc_core::UniqueTypeName type() const override; grpc_local_connect_type connect_type() const { return connect_type_; } @@ -60,7 +60,7 @@ class grpc_local_server_credentials final : public grpc_server_credentials { grpc_core::RefCountedPtr create_security_connector(const grpc_channel_args* /* args */) override; - const char* type() const override; + grpc_core::UniqueTypeName type() const override; grpc_local_connect_type connect_type() const { return connect_type_; } diff --git a/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc b/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc index d354cd682c4..730485802fd 100644 --- a/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc +++ b/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc @@ -346,8 +346,9 @@ std::string grpc_oauth2_token_fetcher_credentials::debug_string() { return "OAuth2TokenFetcherCredentials"; } -const char* grpc_oauth2_token_fetcher_credentials::type() const { - return "Oauth2"; +grpc_core::UniqueTypeName grpc_oauth2_token_fetcher_credentials::type() const { + static grpc_core::UniqueTypeName::Factory kFactory("Oauth2"); + return kFactory.Create(); } // @@ -475,8 +476,9 @@ std::string grpc_google_refresh_token_credentials::debug_string() { grpc_oauth2_token_fetcher_credentials::debug_string()); } -const char* grpc_google_refresh_token_credentials::type() const { - return "GoogleRefreshToken"; +grpc_core::UniqueTypeName grpc_google_refresh_token_credentials::type() const { + static grpc_core::UniqueTypeName::Factory kFactory("GoogleRefreshToken"); + return kFactory.Create(); } static std::string create_loggable_refresh_token( @@ -712,7 +714,10 @@ grpc_access_token_credentials::GetRequestMetadata( return grpc_core::Immediate(std::move(initial_metadata)); } -const char* grpc_access_token_credentials::Type() { return "AccessToken"; } +grpc_core::UniqueTypeName grpc_access_token_credentials::Type() { + static grpc_core::UniqueTypeName::Factory kFactory("AccessToken"); + return kFactory.Create(); +} grpc_access_token_credentials::grpc_access_token_credentials( const char* access_token) diff --git a/src/core/lib/security/credentials/oauth2/oauth2_credentials.h b/src/core/lib/security/credentials/oauth2/oauth2_credentials.h index a93045868c0..4b4c8e6e52b 100644 --- a/src/core/lib/security/credentials/oauth2/oauth2_credentials.h +++ b/src/core/lib/security/credentials/oauth2/oauth2_credentials.h @@ -102,7 +102,7 @@ class grpc_oauth2_token_fetcher_credentials : public grpc_call_credentials { grpc_error_handle error); std::string debug_string() override; - const char* type() const override; + grpc_core::UniqueTypeName type() const override; protected: virtual void fetch_oauth2(grpc_credentials_metadata_request* req, @@ -138,7 +138,7 @@ class grpc_google_refresh_token_credentials final std::string debug_string() override; - const char* type() const override; + grpc_core::UniqueTypeName type() const override; protected: void fetch_oauth2(grpc_credentials_metadata_request* req, @@ -162,9 +162,9 @@ class grpc_access_token_credentials final : public grpc_call_credentials { std::string debug_string() override; - static const char* Type(); + static grpc_core::UniqueTypeName Type(); - const char* type() const override { return Type(); } + grpc_core::UniqueTypeName type() const override { return Type(); } private: int cmp_impl(const grpc_call_credentials* other) const override { diff --git a/src/core/lib/security/credentials/plugin/plugin_credentials.cc b/src/core/lib/security/credentials/plugin/plugin_credentials.cc index 28198133e79..99206ae1b71 100644 --- a/src/core/lib/security/credentials/plugin/plugin_credentials.cc +++ b/src/core/lib/security/credentials/plugin/plugin_credentials.cc @@ -60,7 +60,10 @@ std::string grpc_plugin_credentials::debug_string() { return debug_str; } -const char* grpc_plugin_credentials::type() const { return "Plugin"; } +grpc_core::UniqueTypeName grpc_plugin_credentials::type() const { + static grpc_core::UniqueTypeName::Factory kFactory("Plugin"); + return kFactory.Create(); +} absl::StatusOr grpc_plugin_credentials::PendingRequest::ProcessPluginResult( diff --git a/src/core/lib/security/credentials/plugin/plugin_credentials.h b/src/core/lib/security/credentials/plugin/plugin_credentials.h index 4be8a56fbef..6d5d5c44921 100644 --- a/src/core/lib/security/credentials/plugin/plugin_credentials.h +++ b/src/core/lib/security/credentials/plugin/plugin_credentials.h @@ -41,7 +41,7 @@ struct grpc_plugin_credentials final : public grpc_call_credentials { std::string debug_string() override; - const char* type() const override; + grpc_core::UniqueTypeName type() const override; private: class PendingRequest : public grpc_core::RefCounted { diff --git a/src/core/lib/security/credentials/ssl/ssl_credentials.cc b/src/core/lib/security/credentials/ssl/ssl_credentials.cc index 49b1b2626f2..c35e55f50a8 100644 --- a/src/core/lib/security/credentials/ssl/ssl_credentials.cc +++ b/src/core/lib/security/credentials/ssl/ssl_credentials.cc @@ -82,7 +82,10 @@ grpc_ssl_credentials::create_security_connector( return sc; } -const char* grpc_ssl_credentials::Type() { return "Ssl"; } +grpc_core::UniqueTypeName grpc_ssl_credentials::Type() { + static grpc_core::UniqueTypeName::Factory kFactory("Ssl"); + return kFactory.Create(); +} void grpc_ssl_credentials::build_config( const char* pem_root_certs, grpc_ssl_pem_key_cert_pair* pem_key_cert_pair, @@ -186,7 +189,10 @@ grpc_ssl_server_credentials::create_security_connector( return grpc_ssl_server_security_connector_create(this->Ref()); } -const char* grpc_ssl_server_credentials::Type() { return "Ssl"; } +grpc_core::UniqueTypeName grpc_ssl_server_credentials::Type() { + static grpc_core::UniqueTypeName::Factory kFactory("Ssl"); + return kFactory.Create(); +} tsi_ssl_pem_key_cert_pair* grpc_convert_grpc_to_tsi_cert_pairs( const grpc_ssl_pem_key_cert_pair* pem_key_cert_pairs, diff --git a/src/core/lib/security/credentials/ssl/ssl_credentials.h b/src/core/lib/security/credentials/ssl/ssl_credentials.h index 0fe700d1eef..64d88ef8aec 100644 --- a/src/core/lib/security/credentials/ssl/ssl_credentials.h +++ b/src/core/lib/security/credentials/ssl/ssl_credentials.h @@ -37,9 +37,9 @@ class grpc_ssl_credentials : public grpc_channel_credentials { const char* target, const grpc_channel_args* args, grpc_channel_args** new_args) override; - static const char* Type(); + static grpc_core::UniqueTypeName Type(); - const char* type() const override { return Type(); } + grpc_core::UniqueTypeName type() const override { return Type(); } // TODO(mattstev): Plumb to wrapped languages. Until then, setting the TLS // version should be done for testing purposes only. @@ -80,9 +80,9 @@ class grpc_ssl_server_credentials final : public grpc_server_credentials { grpc_core::RefCountedPtr create_security_connector(const grpc_channel_args* /* args */) override; - static const char* Type(); + static grpc_core::UniqueTypeName Type(); - const char* type() const override { return Type(); } + grpc_core::UniqueTypeName type() const override { return Type(); } bool has_cert_config_fetcher() const { return certificate_config_fetcher_.cb != nullptr; diff --git a/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc b/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc index 9ee718067f3..b967eacd0b2 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc +++ b/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc @@ -85,6 +85,11 @@ StaticDataCertificateProvider::~StaticDataCertificateProvider() { distributor_->SetWatchStatusCallback(nullptr); } +UniqueTypeName StaticDataCertificateProvider::type() const { + static UniqueTypeName::Factory kFactory("StaticData"); + return kFactory.Create(); +} + namespace { gpr_timespec TimeoutSecondsToDeadline(int64_t seconds) { @@ -177,6 +182,11 @@ FileWatcherCertificateProvider::~FileWatcherCertificateProvider() { refresh_thread_.Join(); } +UniqueTypeName FileWatcherCertificateProvider::type() const { + static UniqueTypeName::Factory kFactory("FileWatcher"); + return kFactory.Create(); +} + void FileWatcherCertificateProvider::ForceUpdate() { absl::optional root_certificate; absl::optional pem_key_cert_pairs; diff --git a/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h b/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h index 293a55e25c5..598473c87af 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h +++ b/src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h @@ -30,6 +30,7 @@ #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/thd.h" +#include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/iomgr/load_file.h" #include "src/core/lib/iomgr/pollset_set.h" #include "src/core/lib/security/credentials/tls/grpc_tls_certificate_distributor.h" @@ -62,10 +63,7 @@ struct grpc_tls_certificate_provider // used but they compare as equal (assuming other channel args match). int Compare(const grpc_tls_certificate_provider* other) const { GPR_ASSERT(other != nullptr); - // Intentionally uses grpc_core::QsortCompare instead of strcmp as a safety - // against different grpc_tls_certificate_provider types using the same - // name. - int r = grpc_core::QsortCompare(type(), other->type()); + int r = type().Compare(other->type()); if (r != 0) return r; return CompareImpl(other); } @@ -74,7 +72,7 @@ struct grpc_tls_certificate_provider // implementation for down-casting purposes. Every provider implementation // should use a unique string instance, which should be returned by all // instances of that provider implementation. - virtual const char* type() const = 0; + virtual grpc_core::UniqueTypeName type() const = 0; private: // Implementation for `Compare` method intended to be overridden by @@ -99,7 +97,7 @@ class StaticDataCertificateProvider final return distributor_; } - const char* type() const override { return "StaticData"; } + UniqueTypeName type() const override; private: struct WatcherInfo { @@ -138,7 +136,7 @@ class FileWatcherCertificateProvider final return distributor_; } - const char* type() const override { return "FileWatcher"; } + UniqueTypeName type() const override; private: struct WatcherInfo { diff --git a/src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc b/src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc index 7b554c6a42c..556a7bac82f 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc +++ b/src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc @@ -60,6 +60,11 @@ bool ExternalCertificateVerifier::Verify( return is_done; } +UniqueTypeName ExternalCertificateVerifier::type() const { + static UniqueTypeName::Factory kFactory("External"); + return kFactory.Create(); +} + void ExternalCertificateVerifier::OnVerifyDone( grpc_tls_custom_verification_check_request* request, void* callback_arg, grpc_status_code status, const char* error_details) { @@ -84,6 +89,15 @@ void ExternalCertificateVerifier::OnVerifyDone( } } +// +// NoOpCertificateVerifier +// + +UniqueTypeName NoOpCertificateVerifier::type() const { + static UniqueTypeName::Factory kFactory("NoOp"); + return kFactory.Create(); +} + // // HostNameCertificateVerifier // @@ -152,6 +166,11 @@ bool HostNameCertificateVerifier::Verify( return true; // synchronous check } +UniqueTypeName HostNameCertificateVerifier::type() const { + static UniqueTypeName::Factory kFactory("Hostname"); + return kFactory.Create(); +} + } // namespace grpc_core // diff --git a/src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h b/src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h index af776f8d581..ecf406b7fd8 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h +++ b/src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h @@ -29,6 +29,7 @@ #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/thd.h" +#include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/iomgr/load_file.h" #include "src/core/lib/iomgr/pollset_set.h" #include "src/core/lib/security/credentials/tls/grpc_tls_certificate_distributor.h" @@ -57,7 +58,7 @@ struct grpc_tls_certificate_verifier // verifiers as effectively the same. int Compare(const grpc_tls_certificate_verifier* other) const { GPR_ASSERT(other != nullptr); - int r = grpc_core::QsortCompare(type(), other->type()); + int r = type().Compare(other->type()); if (r != 0) return r; return CompareImpl(other); } @@ -66,7 +67,7 @@ struct grpc_tls_certificate_verifier // implementation for down-casting purposes. Every verifier implementation // should use a unique string instance, which should be returned by all // instances of that verifier implementation. - virtual const char* type() const = 0; + virtual grpc_core::UniqueTypeName type() const = 0; private: // Implementation for `Compare` method intended to be overridden by @@ -99,7 +100,7 @@ class ExternalCertificateVerifier : public grpc_tls_certificate_verifier { external_verifier_->cancel(external_verifier_->user_data, request); } - const char* type() const override { return "External"; } + UniqueTypeName type() const override; private: int CompareImpl(const grpc_tls_certificate_verifier* other) const override { @@ -133,7 +134,7 @@ class NoOpCertificateVerifier : public grpc_tls_certificate_verifier { }; void Cancel(grpc_tls_custom_verification_check_request*) override {} - const char* type() const override { return "NoOp"; } + UniqueTypeName type() const override; private: int CompareImpl( @@ -152,7 +153,7 @@ class HostNameCertificateVerifier : public grpc_tls_certificate_verifier { absl::Status* sync_status) override; void Cancel(grpc_tls_custom_verification_check_request*) override {} - const char* type() const override { return "Hostname"; } + UniqueTypeName type() const override; private: int CompareImpl( diff --git a/src/core/lib/security/credentials/tls/tls_credentials.cc b/src/core/lib/security/credentials/tls/tls_credentials.cc index b92de4cada1..ab2742001cb 100644 --- a/src/core/lib/security/credentials/tls/tls_credentials.cc +++ b/src/core/lib/security/credentials/tls/tls_credentials.cc @@ -106,6 +106,11 @@ TlsCredentials::create_security_connector( return sc; } +grpc_core::UniqueTypeName TlsCredentials::type() const { + static grpc_core::UniqueTypeName::Factory kFactory("Tls"); + return kFactory.Create(); +} + int TlsCredentials::cmp_impl(const grpc_channel_credentials* other) const { const TlsCredentials* o = static_cast(other); if (*options_ == *o->options_) return 0; @@ -126,6 +131,11 @@ TlsServerCredentials::create_security_connector( CreateTlsServerSecurityConnector(this->Ref(), options_); } +grpc_core::UniqueTypeName TlsServerCredentials::type() const { + static grpc_core::UniqueTypeName::Factory kFactory("Tls"); + return kFactory.Create(); +} + /** -- Wrapper APIs declared in grpc_security.h -- **/ grpc_channel_credentials* grpc_tls_credentials_create( diff --git a/src/core/lib/security/credentials/tls/tls_credentials.h b/src/core/lib/security/credentials/tls/tls_credentials.h index 0e719caecba..4356ca750bf 100644 --- a/src/core/lib/security/credentials/tls/tls_credentials.h +++ b/src/core/lib/security/credentials/tls/tls_credentials.h @@ -38,7 +38,7 @@ class TlsCredentials final : public grpc_channel_credentials { const char* target_name, const grpc_channel_args* args, grpc_channel_args** new_args) override; - const char* type() const override { return "Tls"; } + grpc_core::UniqueTypeName type() const override; grpc_tls_credentials_options* options() const { return options_.get(); } @@ -57,7 +57,7 @@ class TlsServerCredentials final : public grpc_server_credentials { grpc_core::RefCountedPtr create_security_connector(const grpc_channel_args* /* args */) override; - const char* type() const override { return "Tls"; } + grpc_core::UniqueTypeName type() const override; grpc_tls_credentials_options* options() const { return options_.get(); } diff --git a/src/core/lib/security/credentials/xds/xds_credentials.cc b/src/core/lib/security/credentials/xds/xds_credentials.cc index c391fc2a20b..680177ffcde 100644 --- a/src/core/lib/security/credentials/xds/xds_credentials.cc +++ b/src/core/lib/security/credentials/xds/xds_credentials.cc @@ -105,7 +105,10 @@ int XdsCertificateVerifier::CompareImpl( return cluster_name_.compare(o->cluster_name_); } -const char* XdsCertificateVerifier::type() const { return "Xds"; } +UniqueTypeName XdsCertificateVerifier::type() const { + static UniqueTypeName::Factory kFactory("Xds"); + return kFactory.Create(); +} bool TestOnlyXdsVerifySubjectAlternativeNames( const char* const* subject_alternative_names, @@ -182,7 +185,10 @@ XdsCredentials::create_security_connector( std::move(call_creds), target_name, temp_args.args, new_args); } -const char* XdsCredentials::Type() { return "Xds"; } +UniqueTypeName XdsCredentials::Type() { + static UniqueTypeName::Factory kFactory("Xds"); + return kFactory.Create(); +} // // XdsServerCredentials @@ -220,7 +226,10 @@ XdsServerCredentials::create_security_connector(const grpc_channel_args* args) { return fallback_credentials_->create_security_connector(args); } -const char* XdsServerCredentials::Type() { return "Xds"; } +UniqueTypeName XdsServerCredentials::Type() { + static UniqueTypeName::Factory kFactory("Xds"); + return kFactory.Create(); +} } // namespace grpc_core diff --git a/src/core/lib/security/credentials/xds/xds_credentials.h b/src/core/lib/security/credentials/xds/xds_credentials.h index 4d308b88615..573c98f4bcf 100644 --- a/src/core/lib/security/credentials/xds/xds_credentials.h +++ b/src/core/lib/security/credentials/xds/xds_credentials.h @@ -41,7 +41,7 @@ class XdsCertificateVerifier : public grpc_tls_certificate_verifier { absl::Status* sync_status) override; void Cancel(grpc_tls_custom_verification_check_request*) override; - const char* type() const override; + UniqueTypeName type() const override; private: int CompareImpl(const grpc_tls_certificate_verifier* other) const override; @@ -60,9 +60,9 @@ class XdsCredentials final : public grpc_channel_credentials { RefCountedPtr call_creds, const char* target_name, const grpc_channel_args* args, grpc_channel_args** new_args) override; - static const char* Type(); + static UniqueTypeName Type(); - const char* type() const override { return Type(); } + UniqueTypeName type() const override { return Type(); } private: int cmp_impl(const grpc_channel_credentials* other) const override { @@ -82,9 +82,9 @@ class XdsServerCredentials final : public grpc_server_credentials { RefCountedPtr create_security_connector( const grpc_channel_args* /* args */) override; - static const char* Type(); + static UniqueTypeName Type(); - const char* type() const override { return Type(); } + UniqueTypeName type() const override { return Type(); } private: RefCountedPtr fallback_credentials_; diff --git a/test/core/gprpp/BUILD b/test/core/gprpp/BUILD index 564c1565b7c..b61ac9f6e98 100644 --- a/test/core/gprpp/BUILD +++ b/test/core/gprpp/BUILD @@ -358,3 +358,19 @@ grpc_cc_test( "//test/core/util:grpc_suppressions", ], ) + +grpc_cc_test( + name = "unique_type_name_test", + srcs = ["unique_type_name_test.cc"], + external_deps = [ + "gtest", + "absl/strings:str_format", + ], + language = "c++", + uses_event_engine = False, + uses_polling = False, + deps = [ + "//:unique_type_name", + "//test/core/util:grpc_suppressions", + ], +) diff --git a/test/core/gprpp/unique_type_name_test.cc b/test/core/gprpp/unique_type_name_test.cc new file mode 100644 index 00000000000..22642fbbc13 --- /dev/null +++ b/test/core/gprpp/unique_type_name_test.cc @@ -0,0 +1,100 @@ +// Copyright 2022 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "src/core/lib/gprpp/unique_type_name.h" + +#include +#include + +#include "absl/strings/str_format.h" + +namespace grpc_core { + +// Teach gtest to print names usefully. +std::ostream& operator<<(std::ostream& os, const UniqueTypeName& name) { + return os << absl::StrFormat("%s (%p)", name.name(), name.name().data()); +} + +namespace { + +class Interface { + public: + virtual ~Interface() = default; + virtual UniqueTypeName type() const = 0; +}; + +class Foo : public Interface { + public: + UniqueTypeName type() const override { + static UniqueTypeName::Factory kFactory("Foo"); + return kFactory.Create(); + } +}; + +class Bar : public Interface { + public: + UniqueTypeName type() const override { + static UniqueTypeName::Factory kFactory("Bar"); + return kFactory.Create(); + } +}; + +// Uses the same string as Foo. +class Foo2 : public Interface { + public: + UniqueTypeName type() const override { + static UniqueTypeName::Factory kFactory("Foo"); + return kFactory.Create(); + } +}; + +TEST(UniqueTypeNameTest, MultipleInstancesShareName) { + Foo foo1; + Foo foo2; + EXPECT_EQ(foo1.type(), foo2.type()); + EXPECT_EQ(0, foo1.type().Compare(foo2.type())); +} + +TEST(UniqueTypeNameTest, DifferentImplsDoNotShareName) { + Foo foo; + Bar bar; + EXPECT_NE(foo.type(), bar.type()); + EXPECT_NE(0, foo.type().Compare(bar.type())); +} + +TEST(UniqueTypeNameTest, MultipleInstancesOfSameStringAreNotEqual) { + Foo foo1; + Foo2 foo2; + EXPECT_NE(foo1.type(), foo2.type()); + EXPECT_NE(0, foo1.type().Compare(foo2.type())); +} + +TEST(UniqueTypeNameTest, CanUseAsMapKey) { + Foo foo; + Bar bar; + std::map m; + m[foo.type()] = 1; + m[bar.type()] = 2; + EXPECT_THAT(m, + ::testing::UnorderedElementsAre(::testing::Pair(foo.type(), 1), + ::testing::Pair(bar.type(), 2))); +} + +} // namespace +} // namespace grpc_core + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/core/security/credentials_test.cc b/test/core/security/credentials_test.cc index b40b2fb224f..823244095a0 100644 --- a/test/core/security/credentials_test.cc +++ b/test/core/security/credentials_test.cc @@ -45,6 +45,7 @@ #include "src/core/lib/gpr/tmpfile.h" #include "src/core/lib/gprpp/host_port.h" #include "src/core/lib/gprpp/time.h" +#include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/http/httpcli.h" #include "src/core/lib/http/httpcli_ssl_credentials.h" #include "src/core/lib/iomgr/error.h" @@ -432,8 +433,6 @@ TEST(CredentialsTest, TestOauth2TokenFetcherCredsParsingMissingTokenLifetime) { grpc_http_response_destroy(&response); } -namespace { - class RequestMetadataState : public RefCounted { public: static RefCountedPtr NewInstance( @@ -548,8 +547,6 @@ class RequestMetadataState : public RefCounted { ActivityPtr activity_; }; -} // namespace - TEST(CredentialsTest, TestGoogleIamCreds) { ExecCtx exec_ctx; auto state = RequestMetadataState::NewInstance( @@ -574,7 +571,7 @@ TEST(CredentialsTest, TestAccessTokenCreds) { "authorization: Bearer blah"); grpc_call_credentials* creds = grpc_access_token_credentials_create("blah", nullptr); - GPR_ASSERT(strcmp(creds->type(), grpc_access_token_credentials::Type()) == 0); + GPR_ASSERT(creds->type() == grpc_access_token_credentials::Type()); /* Check security level. */ GPR_ASSERT(creds->min_security_level() == GRPC_PRIVACY_AND_INTEGRITY); state->RunRequestMetadataTest(creds, kTestUrlScheme, kTestAuthority, @@ -582,20 +579,24 @@ TEST(CredentialsTest, TestAccessTokenCreds) { creds->Unref(); } -namespace { class check_channel_oauth2 final : public grpc_channel_credentials { public: RefCountedPtr create_security_connector( RefCountedPtr call_creds, const char* /*target*/, const grpc_channel_args* /*args*/, grpc_channel_args** /*new_args*/) override { - GPR_ASSERT(strcmp(type(), "check_channel_oauth2") == 0); + GPR_ASSERT(type() == Type()); GPR_ASSERT(call_creds != nullptr); GPR_ASSERT(call_creds->type() == grpc_access_token_credentials::Type()); return nullptr; } - const char* type() const override { return "check_channel_oauth2"; } + static UniqueTypeName Type() { + static UniqueTypeName::Factory kFactory("check_channel_oauth2"); + return kFactory.Create(); + } + + UniqueTypeName type() const override { return Type(); } private: int cmp_impl(const grpc_channel_credentials* other) const override { @@ -604,7 +605,6 @@ class check_channel_oauth2 final : public grpc_channel_credentials { other); } }; -} // namespace TEST(CredentialsTest, TestChannelOauth2CompositeCreds) { ExecCtx exec_ctx; @@ -663,14 +663,13 @@ TEST(CredentialsTest, TestOauth2GoogleIamCompositeCreds) { composite_creds->Unref(); } -namespace { class check_channel_oauth2_google_iam final : public grpc_channel_credentials { public: RefCountedPtr create_security_connector( RefCountedPtr call_creds, const char* /*target*/, const grpc_channel_args* /*args*/, grpc_channel_args** /*new_args*/) override { - GPR_ASSERT(strcmp(type(), "check_channel_oauth2_google_iam") == 0); + GPR_ASSERT(type() == Type()); GPR_ASSERT(call_creds != nullptr); GPR_ASSERT(call_creds->type() == grpc_composite_call_credentials::Type()); const grpc_composite_call_credentials::CallCredentialsList& creds_list = @@ -681,10 +680,13 @@ class check_channel_oauth2_google_iam final : public grpc_channel_credentials { return nullptr; } - const char* type() const override { - return "check_channel_oauth2_google_iam"; + static UniqueTypeName Type() { + static UniqueTypeName::Factory kFactory("check_channel_oauth2_google_iam"); + return kFactory.Create(); } + UniqueTypeName type() const override { return Type(); } + private: int cmp_impl(const grpc_channel_credentials* other) const override { // TODO(yashykt): Check if we can do something better here @@ -692,7 +694,6 @@ class check_channel_oauth2_google_iam final : public grpc_channel_credentials { other); } }; -} // namespace TEST(CredentialsTest, TestChannelOauth2GoogleIamCompositeCreds) { ExecCtx exec_ctx; @@ -1813,7 +1814,10 @@ struct fake_call_creds : public grpc_call_credentials { return Immediate(std::move(initial_metadata)); } - const char* type() const override { return "fake"; } + UniqueTypeName type() const override { + static UniqueTypeName::Factory kFactory("fake"); + return kFactory.Create(); + } private: int cmp_impl(const grpc_call_credentials* other) const override { diff --git a/test/core/security/tls_security_connector_test.cc b/test/core/security/tls_security_connector_test.cc index ab66282e750..10f371ab2c9 100644 --- a/test/core/security/tls_security_connector_test.cc +++ b/test/core/security/tls_security_connector_test.cc @@ -28,6 +28,7 @@ #include #include +#include "src/core/lib/gprpp/unique_type_name.h" #include "src/core/lib/iomgr/load_file.h" #include "src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h" #include "src/core/lib/security/credentials/tls/tls_credentials.h" @@ -120,7 +121,10 @@ class TlsTestCertificateProvider : public grpc_tls_certificate_provider { return distributor_; } - const char* type() const override { return "tls_test"; } + UniqueTypeName type() const override { + static UniqueTypeName::Factory kFactory("tls_test"); + return kFactory.Create(); + } private: int CompareImpl(const grpc_tls_certificate_provider* other) const override { diff --git a/test/core/xds/certificate_provider_store_test.cc b/test/core/xds/certificate_provider_store_test.cc index de0d64ec6bd..b89173c0e93 100644 --- a/test/core/xds/certificate_provider_store_test.cc +++ b/test/core/xds/certificate_provider_store_test.cc @@ -23,6 +23,7 @@ #include #include "src/core/ext/xds/certificate_provider_registry.h" +#include "src/core/lib/gprpp/unique_type_name.h" #include "test/core/util/test_config.h" namespace grpc_core { @@ -44,7 +45,10 @@ class FakeCertificateProvider : public grpc_tls_certificate_provider { return nullptr; } - const char* type() const override { return "fake"; } + UniqueTypeName type() const override { + static UniqueTypeName::Factory kFactory("fake"); + return kFactory.Create(); + } private: int CompareImpl(const grpc_tls_certificate_provider* other) const override { diff --git a/test/cpp/end2end/xds/xds_end2end_test.cc b/test/cpp/end2end/xds/xds_end2end_test.cc index 9cac8d78df6..ae7e2772402 100644 --- a/test/cpp/end2end/xds/xds_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_end2end_test.cc @@ -200,7 +200,10 @@ class FakeCertificateProvider final : public grpc_tls_certificate_provider { return distributor_; } - const char* type() const override { return "fake"; } + grpc_core::UniqueTypeName type() const override { + static grpc_core::UniqueTypeName::Factory kFactory("fake"); + return kFactory.Create(); + } private: int CompareImpl(const grpc_tls_certificate_provider* other) const override { diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 127187f80f1..840060259eb 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2020,6 +2020,7 @@ src/core/lib/gprpp/time.cc \ src/core/lib/gprpp/time.h \ src/core/lib/gprpp/time_util.cc \ src/core/lib/gprpp/time_util.h \ +src/core/lib/gprpp/unique_type_name.h \ src/core/lib/http/format_request.cc \ src/core/lib/http/format_request.h \ src/core/lib/http/httpcli.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index de24b7787db..c52654b0514 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1813,6 +1813,7 @@ src/core/lib/gprpp/time.cc \ src/core/lib/gprpp/time.h \ src/core/lib/gprpp/time_util.cc \ src/core/lib/gprpp/time_util.h \ +src/core/lib/gprpp/unique_type_name.h \ src/core/lib/http/format_request.cc \ src/core/lib/http/format_request.h \ src/core/lib/http/httpcli.cc \ diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index b0291b8110f..7915df81f25 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -7427,6 +7427,30 @@ ], "uses_polling": false }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "unique_type_name_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": false + }, { "args": [], "benchmark": false,