updating ChooseLbPolicy to revert to pick_first lb_policy if an unsupported lb_policy is passed through channel_args (#28651)

* updating ChooseLbPolicy to revert to pick_first lb_policy if an unsupported lb_policy is passed through channel_args

* addressing review comments

* updating comment

* adding check to ensure policy_name is not nullptr

* initializing bool variable to avoid ubsan errors

src/core/ext/filters/client_channel/client_channel.cc

@@ -1150,6 +1150,24 @@ RefCountedPtr<LoadBalancingPolicy::Config> ChooseLbPolicy(
   } else {
     policy_name = grpc_channel_args_find_string(resolver_result.args,
                                                 GRPC_ARG_LB_POLICY_NAME);
+    bool requires_config = false;
+    if (policy_name != nullptr &&
+        (!LoadBalancingPolicyRegistry::LoadBalancingPolicyExists(
+             policy_name, &requires_config) ||
+         requires_config)) {
+      if (requires_config) {
+        gpr_log(GPR_ERROR,
+                "LB policy: %s passed through channel_args must not "
+                "require a config. Using pick_first instead.",
+                policy_name);
+      } else {
+        gpr_log(GPR_ERROR,
+                "LB policy: %s passed through channel_args does not exist. "
+                "Using pick_first instead.",
+                policy_name);
+      }
+      policy_name = "pick_first";
+    }
   }
   // Use pick_first if nothing was specified and we didn't select grpclb
   // above.
@@ -1167,12 +1185,9 @@ RefCountedPtr<LoadBalancingPolicy::Config> ChooseLbPolicy(
   //   already verified that the policy does not require a config.
   // - One of the hard-coded values here, all of which are known to not
   //   require a config.
-  // - A channel arg, in which case the application did something that
-  //   is a misuse of our API.
-  // In the first two cases, these assertions will always be true.  In
-  // the last case, this is probably fine for now.
-  // TODO(roth): If the last case becomes a problem, add better error
-  // handling here.
+  // - A channel arg, in which case we check that the specified policy exists
+  //   and accepts an empty config. If not, we revert to using pick_first
+  //   lb_policy
   GPR_ASSERT(lb_policy_config != nullptr);
   GPR_ASSERT(parse_error == GRPC_ERROR_NONE);
   return lb_policy_config;

test/core/end2end/fuzzers/api_fuzzer_corpus/testcase-4572962842083328

@@ -0,0 +1,29 @@
+actions {
+  create_channel {
+    target: "unix::pehttpP~GK467440737046,,\343\236\234,,t49,Sac,,t"
+    channel_args {
+      key: "grpc.lb_policy_name"
+      str: "unix::pehttpP~GK467440737046,,\343\236\234,,t49,Sac,,t"
+    }
+    channel_actions {
+      add_n_bytes_writable: 32769
+      wait_ms: 89
+    }
+  }
+}
+actions {
+  create_call {
+    method {
+      value: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+    }
+    timeout: 858993408
+  }
+}
+actions {
+  queue_batch {
+    operations {
+      send_initial_metadata {
+      }
+    }
+  }
+}