From be4d2a6d8b3fa79839bbe5bc43788da046840d05 Mon Sep 17 00:00:00 2001
From: AJ Heller <hork@google.com>
Date: Thu, 30 Nov 2023 13:16:10 -0800
Subject: [PATCH] [core] Ensure ChannelArgs::SetObject only allows conforming
 shared_ptr<T> classes (#35008)

ChannelArgs shared_ptr only supports types that extend `enable_shared_from_this`. `args.SetObject<shared_ptr<X>>(x)` with a non-comforming type X will now fail with something like:

```
./src/core/lib/channel/channel_args.h:453:12: error: no matching member function for call to 'Set'
    return Set(ChannelArgNameTraits<T>::ChannelArgName(), std::move(p));
           ^~~
test/core/channel/channel_args_test.cc:352:32: note: in instantiation of function template specialization 'grpc_core::ChannelArgs::SetObject<X>' requested here
  grpc_core::ChannelArgs b = a.SetObject(x);
                               ^
..
```

Closes #35008

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/35008 from drfloob:channel-args-cant-set-unsupported-shared-ptr-type dc93f27ac728fbed233c48aa07fdb839c57ebf0a
PiperOrigin-RevId: 586766674
---
 src/core/lib/channel/channel_args.h           | 26 ++++++++++++-------
 .../fuzzing_event_engine.h                    |  4 ++-
 .../resolver_fuzzer.cc                        |  3 ++-
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/src/core/lib/channel/channel_args.h b/src/core/lib/channel/channel_args.h
index 38bb070213c..78848750294 100644
--- a/src/core/lib/channel/channel_args.h
+++ b/src/core/lib/channel/channel_args.h
@@ -125,6 +125,16 @@ struct ChannelArgTypeTraits<
   };
 };
 
+// Define a check for shared_ptr supported types, which must extend
+// enable_shared_from_this.
+template <typename T>
+struct SupportedSharedPtrType
+    : std::integral_constant<
+          bool, std::is_base_of<std::enable_shared_from_this<T>, T>::value> {};
+template <>
+struct SupportedSharedPtrType<grpc_event_engine::experimental::EventEngine>
+    : std::true_type {};
+
 // Specialization for shared_ptr
 // Incurs an allocation because shared_ptr.release is not a thing.
 template <typename T>
@@ -174,18 +184,12 @@ struct ChannelArgTypeTraits<T,
 };
 
 // GetObject support for shared_ptr and RefCountedPtr
-template <typename T>
-struct WrapInSharedPtr
-    : std::integral_constant<
-          bool, std::is_base_of<std::enable_shared_from_this<T>, T>::value> {};
-template <>
-struct WrapInSharedPtr<grpc_event_engine::experimental::EventEngine>
-    : std::true_type {};
 template <typename T, typename Ignored = void /* for SFINAE */>
 struct GetObjectImpl;
 // std::shared_ptr implementation
 template <typename T>
-struct GetObjectImpl<T, absl::enable_if_t<WrapInSharedPtr<T>::value, void>> {
+struct GetObjectImpl<
+    T, absl::enable_if_t<SupportedSharedPtrType<T>::value, void>> {
   using Result = T*;
   using ReffedResult = std::shared_ptr<T>;
   using StoredType = std::shared_ptr<T>*;
@@ -205,7 +209,8 @@ struct GetObjectImpl<T, absl::enable_if_t<WrapInSharedPtr<T>::value, void>> {
 };
 // RefCountedPtr
 template <typename T>
-struct GetObjectImpl<T, absl::enable_if_t<!WrapInSharedPtr<T>::value, void>> {
+struct GetObjectImpl<
+    T, absl::enable_if_t<!SupportedSharedPtrType<T>::value, void>> {
   using Result = T*;
   using ReffedResult = RefCountedPtr<T>;
   using StoredType = Result;
@@ -391,6 +396,9 @@ class ChannelArgs {
           decltype(ChannelArgTypeTraits<std::shared_ptr<T>>::VTable())>::value,
       ChannelArgs>
   Set(absl::string_view name, std::shared_ptr<T> value) const {
+    static_assert(SupportedSharedPtrType<T>::value,
+                  "Type T must extend std::enable_shared_from_this to be added "
+                  "into ChannelArgs as a shared_ptr<T>");
     auto* store_value = new std::shared_ptr<T>(value);
     return Set(
         name,
diff --git a/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h b/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h
index 5f83f44c9ea..78e36a65d2a 100644
--- a/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h
+++ b/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h
@@ -48,7 +48,9 @@ namespace experimental {
 
 // EventEngine implementation to be used by fuzzers.
 // It's only allowed to have one FuzzingEventEngine instantiated at a time.
-class FuzzingEventEngine : public EventEngine {
+class FuzzingEventEngine
+    : public EventEngine,
+      public std::enable_shared_from_this<FuzzingEventEngine> {
  public:
   struct Options {
     Duration max_delay_run_after = std::chrono::seconds(30);
diff --git a/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc b/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc
index 483ad6b0e4a..97ed6eaa71c 100644
--- a/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc
+++ b/test/core/ext/filters/event_engine_client_channel_resolver/resolver_fuzzer.cc
@@ -71,7 +71,8 @@ absl::Status ErrorToAbslStatus(
 }
 
 class FuzzingResolverEventEngine
-    : public grpc_event_engine::experimental::AbortingEventEngine {
+    : public grpc_event_engine::experimental::AbortingEventEngine,
+      public std::enable_shared_from_this<FuzzingResolverEventEngine> {
  public:
   explicit FuzzingResolverEventEngine(
       const event_engine_client_channel_resolver::Msg& msg,