Return independently owned value arrays from get_values

storm-core/src/native/worker-launcher/impl/configuration.c

@@ -295,7 +295,31 @@ char * get_value(const char* key) {
  */
 char ** get_values(const char * key) {
   char *value = get_value(key);
-  return extract_values_delim(value, ",");
+  // extract_values_delim tokenizes value in place and returns pointers into it.
+  // Copy the tokens into an independently owned array and release the backing
+  // buffer here, so a leading or trailing delimiter (e.g. ",a") never leaves
+  // free_values with an interior pointer to free, and free_values can release
+  // each element without assuming anything about the original buffer.
+  char **tokens = extract_values_delim(value, ",");
+  if (tokens == NULL) {
+    free(value);
+    return NULL;
+  }
+  int count = 0;
+  while (tokens[count] != NULL) {
+    count++;
+  }
+  char **out = (char **) malloc(sizeof(char *) * (count + 1));
+  if (out != NULL) {
+    int i;
+    for (i = 0; i < count; i++) {
+      out[i] = strdup(tokens[i]);
+    }
+    out[count] = NULL;
+  }
+  free(tokens);
+  free(value);
+  return out;
 }
 
 /**
@@ -326,20 +350,23 @@ char ** extract_values_delim(char *value, const char *delim) {
       tempTok = strtok_r(NULL, delim, &tempstr);
     }
   }
-  if (size > 0) {
+  // Terminate whenever an array was allocated, including the zero-token case
+  // (e.g. a value consisting only of delimiters). Callers walk the array until
+  // the first NULL, so an unterminated array would be read past its contents.
+  if (toPass != NULL) {
     toPass[size] = NULL;
   }
   return toPass;
 }
 
-// free an entry set of values
+// free a NULL-terminated array of values and the array itself
 void free_values(char** values) {
-  if (*values != NULL) {
-    free(*values);
-    *values = NULL;
+  if (values == NULL) {
+    return;
   }
-  if (values != NULL) {
-    free(values);
-    values = NULL;
+  char** v;
+  for (v = values; *v != NULL; v++) {
+    free(*v);
   }
+  free(values);
 }

storm-core/src/native/worker-launcher/test/test-worker-launcher.c

@@ -263,6 +263,63 @@ void test_signal_container_group() {
   }
 }
 
+// get_values must return an independently owned, NULL-terminated array for any
+// value, including ones with leading, trailing, or only delimiters, so that
+// free_values can release it without freeing an interior pointer of the parsed
+// buffer. Run in a child (via run_test_in_child) so the temporary config that
+// read_config installs does not leak into later tests.
+void test_get_values_degenerate() {
+  const char* cfg = TEST_ROOT "/get-values.cfg";
+  FILE* f = fopen(cfg, "w");
+  if (f == NULL) {
+    printf("FAIL: could not write %s\n", cfg);
+    exit(1);
+  }
+  fprintf(f, "test.values.normal=a,b,c\n");
+  fprintf(f, "test.values.leading=,a,b\n");
+  fprintf(f, "test.values.trailing=a,b,\n");
+  fprintf(f, "test.values.only.delims=,,\n");
+  fclose(f);
+  read_config(cfg);
+
+  char** v = get_values("test.values.normal");
+  if (v == NULL || v[0] == NULL || strcmp(v[0], "a") != 0
+      || v[1] == NULL || strcmp(v[1], "b") != 0
+      || v[2] == NULL || strcmp(v[2], "c") != 0 || v[3] != NULL) {
+    printf("FAIL: get_values did not return [a,b,c] for a normal value\n");
+    exit(1);
+  }
+  free_values(v);
+
+  // A leading delimiter makes the first token an interior pointer of the parsed
+  // buffer; get_values must still yield [a,b] and free_values must not choke.
+  v = get_values("test.values.leading");
+  if (v == NULL || v[0] == NULL || strcmp(v[0], "a") != 0
+      || v[1] == NULL || strcmp(v[1], "b") != 0 || v[2] != NULL) {
+    printf("FAIL: get_values did not return [a,b] for a leading-delimiter value\n");
+    exit(1);
+  }
+  free_values(v);
+
+  v = get_values("test.values.trailing");
+  if (v == NULL || v[0] == NULL || strcmp(v[0], "a") != 0
+      || v[1] == NULL || strcmp(v[1], "b") != 0 || v[2] != NULL) {
+    printf("FAIL: get_values did not return [a,b] for a trailing-delimiter value\n");
+    exit(1);
+  }
+  free_values(v);
+
+  // Only delimiters: an empty but NULL-terminated array, not an unterminated one.
+  v = get_values("test.values.only.delims");
+  if (v == NULL || v[0] != NULL) {
+    printf("FAIL: get_values did not return an empty terminated array for a delimiter-only value\n");
+    exit(1);
+  }
+  free_values(v);
+
+  printf("get_values degenerate-value handling OK\n");
+}
+
 int main(int argc, char **argv) {
   LOGFILE = stdout;
   ERRORFILE = stderr;
@@ -314,6 +371,7 @@ int main(int argc, char **argv) {
   // when they change user they don't give up our privs
   run_test_in_child("test_signal_container", test_signal_container);
   run_test_in_child("test_signal_container_group", test_signal_container_group);
+  run_test_in_child("test_get_values_degenerate", test_get_values_degenerate);
 
   seteuid(0);
   run("rm -fr " TEST_ROOT);