From b92f17f78d69557cf9057bb1dcab8a4c839c6f3d Mon Sep 17 00:00:00 2001
From: Vijay Pai <vpai@google.com>
Date: Wed, 12 Aug 2020 01:18:24 -0700
Subject: [PATCH] Fix issues around lifetime of c_str() results

---
 src/core/lib/surface/channel.cc | 48 +++++++++++++++++++--------------
 src/core/lib/surface/channel.h  | 11 ++++++--
 2 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc
index bcfa5e6207d..9a864c07c22 100644
--- a/src/core/lib/surface/channel.cc
+++ b/src/core/lib/surface/channel.cc
@@ -415,26 +415,33 @@ grpc_call* grpc_channel_create_pollset_set_call(
 
 namespace grpc_core {
 
-RegisteredCall::RegisteredCall(const char* method, const char* host) {
-  path = grpc_mdelem_from_slices(GRPC_MDSTR_PATH,
-                                 grpc_core::ExternallyManagedSlice(method));
-  authority =
-      host ? grpc_mdelem_from_slices(GRPC_MDSTR_AUTHORITY,
-                                     grpc_core::ExternallyManagedSlice(host))
-           : GRPC_MDNULL;
-}
+RegisteredCall::RegisteredCall(const char* method_arg, const char* host_arg)
+    : method(method_arg != nullptr ? method_arg : ""),
+      host(host_arg != nullptr ? host_arg : ""),
+      path(grpc_mdelem_from_slices(
+          GRPC_MDSTR_PATH, grpc_core::ExternallyManagedSlice(method.c_str()))),
+      authority(!host.empty()
+                    ? grpc_mdelem_from_slices(
+                          GRPC_MDSTR_AUTHORITY,
+                          grpc_core::ExternallyManagedSlice(host.c_str()))
+                    : GRPC_MDNULL) {}
 
 // TODO(vjpai): Delete copy-constructor when allowed by all supported compilers.
-RegisteredCall::RegisteredCall(const RegisteredCall& other) {
-  path = other.path;
-  authority = other.authority;
-  GRPC_MDELEM_REF(path);
-  GRPC_MDELEM_REF(authority);
-}
-
-RegisteredCall::RegisteredCall(RegisteredCall&& other) noexcept {
-  path = other.path;
-  authority = other.authority;
+RegisteredCall::RegisteredCall(const RegisteredCall& other)
+    : RegisteredCall(other.method.c_str(), other.host.c_str()) {}
+
+RegisteredCall::RegisteredCall(RegisteredCall&& other) noexcept
+    : method(std::move(other.method)),
+      host(std::move(other.host)),
+      path(grpc_mdelem_from_slices(
+          GRPC_MDSTR_PATH, grpc_core::ExternallyManagedSlice(method.c_str()))),
+      authority(!host.empty()
+                    ? grpc_mdelem_from_slices(
+                          GRPC_MDSTR_AUTHORITY,
+                          grpc_core::ExternallyManagedSlice(host.c_str()))
+                    : GRPC_MDNULL) {
+  GRPC_MDELEM_UNREF(other.path);
+  GRPC_MDELEM_UNREF(other.authority);
   other.path = GRPC_MDNULL;
   other.authority = GRPC_MDNULL;
 }
@@ -457,13 +464,14 @@ void* grpc_channel_register_call(grpc_channel* channel, const char* method,
 
   grpc_core::MutexLock lock(&channel->registration_table->mu);
   channel->registration_table->method_registration_attempts++;
-  auto key = std::make_pair(host, method);
+  auto key = std::make_pair(std::string(host != nullptr ? host : ""),
+                            std::string(method != nullptr ? method : ""));
   auto rc_posn = channel->registration_table->map.find(key);
   if (rc_posn != channel->registration_table->map.end()) {
     return &rc_posn->second;
   }
   auto insertion_result = channel->registration_table->map.insert(
-      {key, grpc_core::RegisteredCall(method, host)});
+      {std::move(key), grpc_core::RegisteredCall(method, host)});
   return &insertion_result.first->second;
 }
 
diff --git a/src/core/lib/surface/channel.h b/src/core/lib/surface/channel.h
index 000882bb849..2c8ea4f40d1 100644
--- a/src/core/lib/surface/channel.h
+++ b/src/core/lib/surface/channel.h
@@ -69,21 +69,28 @@ void grpc_channel_update_call_size_estimate(grpc_channel* channel, size_t size);
 namespace grpc_core {
 
 struct RegisteredCall {
+  // The method and host are kept as part of this struct just to manage their
+  // lifetime since they must outlive the mdelem contents.
+  std::string method;
+  std::string host;
+
   grpc_mdelem path;
   grpc_mdelem authority;
 
-  explicit RegisteredCall(const char* method, const char* host);
+  explicit RegisteredCall(const char* method_arg, const char* host_arg);
   // TODO(vjpai): delete copy constructor once all supported compilers allow
   //              std::map value_type to be MoveConstructible.
   RegisteredCall(const RegisteredCall& other);
   RegisteredCall(RegisteredCall&& other) noexcept;
+  RegisteredCall& operator=(const RegisteredCall&) = delete;
+  RegisteredCall& operator=(RegisteredCall&&) = delete;
 
   ~RegisteredCall();
 };
 
 struct CallRegistrationTable {
   grpc_core::Mutex mu;
-  std::map<std::pair<const char*, const char*>, RegisteredCall>
+  std::map<std::pair<std::string, std::string>, RegisteredCall>
       map /* GUARDED_BY(mu) */;
   int method_registration_attempts /* GUARDED_BY(mu) */ = 0;
 };