From 76d0ec4a73ffa662312732bc6514430acc207feb Mon Sep 17 00:00:00 2001
From: "Mark D. Roth" <roth@google.com>
Date: Thu, 26 Oct 2017 11:08:14 -0700
Subject: Fix a bunch of dumb service config parsing bugs.

---
 src/core/lib/transport/service_config.cc | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

(limited to 'src/core/lib/transport/service_config.cc')

diff --git a/src/core/lib/transport/service_config.cc b/src/core/lib/transport/service_config.cc
index 070a13a2b4..05907de7d7 100644
--- a/src/core/lib/transport/service_config.cc
+++ b/src/core/lib/transport/service_config.cc
@@ -111,7 +111,13 @@ const char* grpc_service_config_get_lb_policy_name(
 static size_t count_names_in_method_config_json(grpc_json* json) {
   size_t num_names = 0;
   for (grpc_json* field = json->child; field != NULL; field = field->next) {
-    if (field->key != NULL && strcmp(field->key, "name") == 0) ++num_names;
+    if (field->key != NULL && strcmp(field->key, "name") == 0) {
+      if (field->type != GRPC_JSON_ARRAY) return -1;
+      for (grpc_json* name = field->child; name != NULL; name = name->next) {
+        if (name->type != GRPC_JSON_OBJECT) return -1;
+        ++num_names;
+      }
+    }
   }
   return num_names;
 }
@@ -148,6 +154,8 @@ static char* parse_json_method_name(grpc_json* json) {
 static bool parse_json_method_config(
     grpc_exec_ctx* exec_ctx, grpc_json* json,
     void* (*create_value)(const grpc_json* method_config_json),
+    void* (*ref_value)(void* value),
+    void (*unref_value)(grpc_exec_ctx* exec_ctx, void* value),
     grpc_slice_hash_table_entry* entries, size_t* idx) {
   // Construct value.
   void* method_config = create_value(json);
@@ -162,6 +170,7 @@ static bool parse_json_method_config(
       if (child->type != GRPC_JSON_ARRAY) goto done;
       for (grpc_json* name = child->child; name != NULL; name = name->next) {
         char* path = parse_json_method_name(name);
+        if (path == NULL) goto done;
         gpr_strvec_add(&paths, path);
       }
     }
@@ -170,11 +179,12 @@ static bool parse_json_method_config(
   // Add entry for each path.
   for (size_t i = 0; i < paths.count; ++i) {
     entries[*idx].key = grpc_slice_from_copied_string(paths.strs[i]);
-    entries[*idx].value = method_config;
+    entries[*idx].value = ref_value(method_config);
     ++*idx;
   }
   success = true;
 done:
+  unref_value(exec_ctx, method_config);
   gpr_strvec_destroy(&paths);
   return success;
 }
@@ -182,7 +192,8 @@ done:
 grpc_slice_hash_table* grpc_service_config_create_method_config_table(
     grpc_exec_ctx* exec_ctx, const grpc_service_config* service_config,
     void* (*create_value)(const grpc_json* method_config_json),
-    void (*destroy_value)(grpc_exec_ctx* exec_ctx, void* value)) {
+    void* (*ref_value)(void* value),
+    void (*unref_value)(grpc_exec_ctx* exec_ctx, void* value)) {
   const grpc_json* json = service_config->json_tree;
   // Traverse parsed JSON tree.
   if (json->type != GRPC_JSON_OBJECT || json->key != NULL) return NULL;
@@ -196,7 +207,9 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table(
       // Find number of entries.
       for (grpc_json* method = field->child; method != NULL;
            method = method->next) {
-        num_entries += count_names_in_method_config_json(method);
+        size_t count = count_names_in_method_config_json(method);
+        if (count <= 0) return NULL;
+        num_entries += count;
       }
       // Populate method config table entries.
       entries = (grpc_slice_hash_table_entry*)gpr_malloc(
@@ -204,8 +217,13 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table(
       size_t idx = 0;
       for (grpc_json* method = field->child; method != NULL;
            method = method->next) {
-        if (!parse_json_method_config(exec_ctx, method, create_value, entries,
-                                      &idx)) {
+        if (!parse_json_method_config(exec_ctx, method, create_value, ref_value,
+                                      unref_value, entries, &idx)) {
+          for (size_t i = 0; i < idx; ++i) {
+            grpc_slice_unref_internal(exec_ctx, entries[i].key);
+            unref_value(exec_ctx, entries[i].value);
+          }
+          gpr_free(entries);
           return NULL;
         }
       }
@@ -216,7 +234,7 @@ grpc_slice_hash_table* grpc_service_config_create_method_config_table(
   grpc_slice_hash_table* method_config_table = NULL;
   if (entries != NULL) {
     method_config_table =
-        grpc_slice_hash_table_create(num_entries, entries, destroy_value, NULL);
+        grpc_slice_hash_table_create(num_entries, entries, unref_value, NULL);
     gpr_free(entries);
   }
   return method_config_table;
-- 
cgit v1.2.3