From 3f19e0cc02ba2007209ea7e6298593323e371913 Mon Sep 17 00:00:00 2001
From: Yash Tibrewal <yashkt@google.com>
Date: Tue, 22 Feb 2022 08:03:23 -0800
Subject: [PATCH] Revert "Revert "CompositeChannelCredentials: Comparator
 implementation ( #28902)" (#28919)" (#28918)

* CompositeChannelCredentials: Comparator implementation retry

* Fix test
---
 .../composite/composite_credentials.cc        |  4 ++
 .../composite/composite_credentials.h         | 13 +++--
 test/core/security/credentials_test.cc        | 48 +++++++++++++++++++
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/src/core/lib/security/credentials/composite/composite_credentials.cc b/src/core/lib/security/credentials/composite/composite_credentials.cc
index 59462c8f0eb..dad03165ae9 100644
--- a/src/core/lib/security/credentials/composite/composite_credentials.cc
+++ b/src/core/lib/security/credentials/composite/composite_credentials.cc
@@ -35,6 +35,10 @@
 #include "src/core/lib/iomgr/polling_entity.h"
 #include "src/core/lib/surface/api_trace.h"
 
+namespace grpc_core {
+const char kCredentialsTypeComposite[] = "composite";
+}  // namespace grpc_core
+
 /* -- Composite call credentials. -- */
 
 static void composite_call_metadata_cb(void* arg, grpc_error_handle error);
diff --git a/src/core/lib/security/credentials/composite/composite_credentials.h b/src/core/lib/security/credentials/composite/composite_credentials.h
index fe2da4bd0ac..ab6bbe35dca 100644
--- a/src/core/lib/security/credentials/composite/composite_credentials.h
+++ b/src/core/lib/security/credentials/composite/composite_credentials.h
@@ -30,12 +30,16 @@
 
 /* -- Composite channel credentials. -- */
 
+namespace grpc_core {
+extern const char kCredentialsTypeComposite[];
+}
+
 class grpc_composite_channel_credentials : public grpc_channel_credentials {
  public:
   grpc_composite_channel_credentials(
       grpc_core::RefCountedPtr<grpc_channel_credentials> channel_creds,
       grpc_core::RefCountedPtr<grpc_call_credentials> call_creds)
-      : grpc_channel_credentials(channel_creds->type()),
+      : grpc_channel_credentials(grpc_core::kCredentialsTypeComposite),
         inner_creds_(std::move(channel_creds)),
         call_creds_(std::move(call_creds)) {}
 
@@ -64,9 +68,10 @@ class grpc_composite_channel_credentials : public grpc_channel_credentials {
 
  private:
   int cmp_impl(const grpc_channel_credentials* other) const override {
-    // TODO(yashykt): Check if we can do something better here
-    return grpc_core::QsortCompare(
-        static_cast<const grpc_channel_credentials*>(this), other);
+    auto* o = static_cast<const grpc_composite_channel_credentials*>(other);
+    int r = inner_creds_->cmp(o->inner_creds_.get());
+    if (r != 0) return r;
+    return call_creds_->cmp(o->call_creds_.get());
   }
 
   grpc_core::RefCountedPtr<grpc_channel_credentials> inner_creds_;
diff --git a/test/core/security/credentials_test.cc b/test/core/security/credentials_test.cc
index 82436c3de3f..2990a359222 100644
--- a/test/core/security/credentials_test.cc
+++ b/test/core/security/credentials_test.cc
@@ -3629,6 +3629,54 @@ TEST(CredentialsTest, TestHttpRequestSSLCredentialsSingleton) {
   EXPECT_EQ(creds_1, creds_2);
 }
 
+TEST(CredentialsTest, TestCompositeChannelCredsCompareSuccess) {
+  auto* insecure_creds = grpc_insecure_credentials_create();
+  auto fake_creds = grpc_core::MakeRefCounted<fake_call_creds>();
+  auto* composite_creds_1 = grpc_composite_channel_credentials_create(
+      insecure_creds, fake_creds.get(), nullptr);
+  auto* composite_creds_2 = grpc_composite_channel_credentials_create(
+      insecure_creds, fake_creds.get(), nullptr);
+  EXPECT_EQ(composite_creds_1->cmp(composite_creds_2), 0);
+  EXPECT_EQ(composite_creds_2->cmp(composite_creds_1), 0);
+  grpc_channel_credentials_release(insecure_creds);
+  grpc_channel_credentials_release(composite_creds_1);
+  grpc_channel_credentials_release(composite_creds_2);
+}
+
+TEST(CredentialsTest,
+     TestCompositeChannelCredsCompareFailureDifferentChannelCreds) {
+  auto* insecure_creds = grpc_insecure_credentials_create();
+  auto* fake_channel_creds = grpc_fake_transport_security_credentials_create();
+  auto fake_creds = grpc_core::MakeRefCounted<fake_call_creds>();
+  auto* composite_creds_1 = grpc_composite_channel_credentials_create(
+      insecure_creds, fake_creds.get(), nullptr);
+  auto* composite_creds_2 = grpc_composite_channel_credentials_create(
+      fake_channel_creds, fake_creds.get(), nullptr);
+  EXPECT_NE(composite_creds_1->cmp(composite_creds_2), 0);
+  EXPECT_NE(composite_creds_2->cmp(composite_creds_1), 0);
+  grpc_channel_credentials_release(insecure_creds);
+  grpc_channel_credentials_release(fake_channel_creds);
+  grpc_channel_credentials_release(composite_creds_1);
+  grpc_channel_credentials_release(composite_creds_2);
+}
+
+TEST(CredentialsTest,
+     TestCompositeChannelCredsCompareFailureDifferentCallCreds) {
+  auto* insecure_creds = grpc_insecure_credentials_create();
+  auto fake_creds = grpc_core::MakeRefCounted<fake_call_creds>();
+  auto* md_creds = grpc_md_only_test_credentials_create("key", "value", false);
+  auto* composite_creds_1 = grpc_composite_channel_credentials_create(
+      insecure_creds, fake_creds.get(), nullptr);
+  auto* composite_creds_2 = grpc_composite_channel_credentials_create(
+      insecure_creds, md_creds, nullptr);
+  EXPECT_NE(composite_creds_1->cmp(composite_creds_2), 0);
+  EXPECT_NE(composite_creds_2->cmp(composite_creds_1), 0);
+  grpc_channel_credentials_release(insecure_creds);
+  grpc_call_credentials_release(md_creds);
+  grpc_channel_credentials_release(composite_creds_1);
+  grpc_channel_credentials_release(composite_creds_2);
+}
+
 TEST(CredentialsTest, TestXdsCredentialsCompareSucces) {
   auto* insecure_creds = grpc_insecure_credentials_create();
   auto* xds_creds_1 = grpc_xds_credentials_create(insecure_creds);