config: error configure_plugins_type on invalid properties

After this change, when a plugin configuration contains an invalid property,
`configure_plugins_type` returns an error which ultimately crashes fluentbit,
instead of only printing an error log and continuing on. The change also fixes
a small issue where `name` was printed by `flb_error` after it was freed. To
properly test this change and get the test to pass with `-DSANITIZE_ADDRESS=On`
on all platforms, I had to update `configure_plugins_type` to free objects that
it fails to instantiate.

Signed-off-by: Bradley Laney <[email protected]>

Author: stoksc

src/flb_config.c

@@ -737,7 +737,7 @@ static int configure_plugins_type(struct flb_config *config, struct flb_cf *cf,
 {
     int ret;
     char *tmp;
-    char *name;
+    char *name = NULL;
     char *s_type;
     struct mk_list *list;
     struct mk_list *head;
@@ -747,7 +747,7 @@ static int configure_plugins_type(struct flb_config *config, struct flb_cf *cf,
     struct flb_cf_section *s;
     struct flb_cf_group *processors = NULL;
     int i;
-    void *ins;
+    void *ins = NULL;
 
     if (type == FLB_CF_CUSTOM) {
         s_type = "custom";
@@ -766,7 +766,7 @@ static int configure_plugins_type(struct flb_config *config, struct flb_cf *cf,
         list = &cf->outputs;
     }
     else {
-        return -1;
+        goto error;
     }
 
     mk_list_foreach(head, list) {
@@ -775,7 +775,7 @@ static int configure_plugins_type(struct flb_config *config, struct flb_cf *cf,
         if (!name) {
             flb_error("[config] section '%s' is missing the 'name' property",
                       s_type);
-            return -1;
+            goto error;
         }
 
         /* translate the variable */
@@ -802,9 +802,8 @@ static int configure_plugins_type(struct flb_config *config, struct flb_cf *cf,
             flb_error("[config] section '%s' tried to instance a plugin name "
                       "that doesn't exist", name);
             flb_sds_destroy(name);
-            return -1;
+            goto error;
         }
-        flb_sds_destroy(name);
 
         /*
          * iterate section properties and populate instance by using specific
@@ -866,6 +865,7 @@ static int configure_plugins_type(struct flb_config *config, struct flb_cf *cf,
                 flb_error("[config] could not configure property '%s' on "
                           "%s plugin with section name '%s'",
                           kv->key, s_type, name);
+                goto error;
             }
         }
 
@@ -875,22 +875,44 @@ static int configure_plugins_type(struct flb_config *config, struct flb_cf *cf,
             if (type == FLB_CF_INPUT) {
                 ret = flb_processors_load_from_config_format_group(((struct flb_input_instance *) ins)->processor, processors);
                 if (ret == -1) {
-                    return -1;
+                    goto error;
                 }
             }
             else if (type == FLB_CF_OUTPUT) {
                 ret = flb_processors_load_from_config_format_group(((struct flb_output_instance *) ins)->processor, processors);
                 if (ret == -1) {
-                    return -1; 
+                    goto error;
                 }
             }
             else {
                 flb_error("[config] section '%s' does not support processors", s_type);
             }
         }
+
+        flb_sds_destroy(name);
     }
 
     return 0;
+
+error:
+    if (name != NULL) {
+        flb_sds_destroy(name);
+    }
+    if (ins != NULL) {
+        if (type == FLB_CF_CUSTOM) {
+            flb_custom_instance_destroy(ins);
+        }
+        else if (type == FLB_CF_INPUT) {
+            flb_input_instance_destroy(ins);
+        }
+        else if (type == FLB_CF_FILTER) {
+            flb_filter_instance_destroy(ins);
+        }
+        else if (type == FLB_CF_OUTPUT) {
+            flb_output_instance_destroy(ins);
+        }
+    }
+    return -1;
 }
 /* Load a struct flb_config_format context into a flb_config instance */
 int flb_config_load_config_format(struct flb_config *config, struct flb_cf *cf)

tests/internal/config_format_yaml.c

@@ -1,6 +1,8 @@
 /* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 
 #include <fluent-bit/flb_info.h>
+#include <fluent-bit/flb_input.h>
+#include <fluent-bit/flb_output.h>
 #include <fluent-bit/flb_mem.h>
 #include <fluent-bit/flb_kv.h>
 #include <fluent-bit/flb_str.h>
@@ -833,6 +835,34 @@ static void test_upstream_servers()
     flb_cf_destroy(cf);
 }
 
+static void test_invalid_property()
+{
+    char* test_cases[] = {
+        FLB_TESTS_CONF_PATH "/invalid_input_property.yaml",
+        FLB_TESTS_CONF_PATH "/invalid_output_property.yaml",
+        NULL,
+    };
+
+    struct flb_cf *cf;
+    struct flb_config *config;
+    int ret;
+    int i;
+
+    for (i = 0; test_cases[i] != NULL; i++) {
+        cf = flb_cf_yaml_create(NULL, test_cases[i], NULL, 0);
+        TEST_ASSERT(cf != NULL);
+    
+        config = flb_config_init();
+        TEST_ASSERT(config != NULL);
+        
+        ret = flb_config_load_config_format(config, cf);
+        TEST_ASSERT_(ret == -1, "expected invalid property to return an error in file %s", test_cases[i]);
+
+        flb_config_exit(config);
+        flb_cf_destroy(cf);
+    }
+}
+
 TEST_LIST = {
     { "basic"    , test_basic},
     { "customs section", test_customs_section},
@@ -846,5 +876,6 @@ TEST_LIST = {
     { "stream_processor", test_stream_processor},
     { "plugins", test_plugins},
     { "upstream_servers", test_upstream_servers},
+    { "invalid_input_property", test_invalid_property},
     { 0 }
 };

tests/internal/data/config_format/yaml/invalid_input_property.yaml

@@ -0,0 +1,4 @@
+pipeline:
+  inputs:
+    - name: dummy
+      log_level: thisdoesnotexist

tests/internal/data/config_format/yaml/invalid_output_property.yaml

@@ -0,0 +1,5 @@
+pipeline:
+  outputs:
+    - name: stdout
+      match: '*'
+      log_level: thisdoesnotexist