re-work logic to allow mixed quoted/unquoted list elements

open-telemetry · Dec 8, 2023 · 6390294 · 6390294
1 parent 719ff40
commit 6390294
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 31 deletions.
diff --git a/...rc/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/DefaultConfigProperties.java b/...rc/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/DefaultConfigProperties.java
@@ -14,6 +14,7 @@
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
 import java.time.Duration;
 import java.util.AbstractMap;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -183,17 +184,31 @@ public List<String> getList(String name) {
     }
 
     // Support list members containing commas per RFC9110 5.5 suggestion
-    String[] listMembers;
-    // check if list members are double-quoted
-    if (value.startsWith("\"") && value.endsWith("\"")) {
-      // remove first and last quote and split on '","'
-      value = value.substring(1, value.length() - 1);
-      listMembers = value.split("\"\\s?,\\s?\"");
-    } else if (value.contains(",") && value.contains("\"")) {
-      throw new ConfigurationException("Invalid list property: " + name + "=" + config.get(name));
-    } else {
-      listMembers = value.split(",");
+    List<String> listMembers = new ArrayList<>();
+    StringBuilder curValue = new StringBuilder();
+    boolean openedQuote = false;
+    for (int i = 0; i < value.length(); i++) {
+      char c = value.charAt(i);
+      if (c == '"') {
+        if (openedQuote) {
+          openedQuote = false;
+          listMembers.add(curValue.toString());
+          curValue = new StringBuilder();
+        } else {
+          openedQuote = true;
+        }
+      } else if (c == ',') {
+        if (openedQuote) {
+          curValue.append(c);
+        } else {
+          listMembers.add(curValue.toString());
+          curValue = new StringBuilder();
+        }
+      } else {
+        curValue.append(c);
+      }
     }
+    listMembers.add(curValue.toString());
 
     return filterBlanksAndNulls(listMembers);
   }
@@ -226,8 +241,8 @@ public Map<String, String> getMap(String name) {
     return getList(ConfigUtil.normalizePropertyKey(name)).stream()
         .map(
             entry -> {
-              String[] split = entry.split("=", 2);
-              if (split.length != 2 || StringUtils.isNullOrEmpty(split[0])) {
+              List<String> split = Arrays.asList(entry.split("=", 2));
+              if (split.size() != 2 || StringUtils.isNullOrEmpty(split.get(0))) {
                 throw new ConfigurationException(
                     "Invalid map property: " + name + "=" + config.get(name));
               }
@@ -260,11 +275,8 @@ private static ConfigurationException newInvalidPropertyException(
         "Invalid value for property " + name + "=" + value + ". Must be a " + type + ".");
   }
 
-  private static List<String> filterBlanksAndNulls(String[] values) {
-    return Arrays.stream(values)
-        .map(String::trim)
-        .filter(s -> !s.isEmpty())
-        .collect(Collectors.toList());
+  private static List<String> filterBlanksAndNulls(List<String> values) {
+    return values.stream().map(String::trim).filter(s -> !s.isEmpty()).collect(Collectors.toList());
   }
 
   /** Returns the TimeUnit associated with a unit string. Defaults to milliseconds. */

diff --git a/...i/src/test/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigPropertiesTest.java b/...i/src/test/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigPropertiesTest.java
@@ -36,7 +36,7 @@ void allValid() {
         .containsExactly(entry("cat", "meow"), entry("dog", "bark"), entry("bear", "growl"));
     assertThat(config.getMap("test.map.commas"))
         .containsExactly(
-            entry("cat", "meow,hiss"), entry("dog", "bark,growl"), entry("bear", "growl,roar"));
+            entry("cat", "meow,hiss"), entry("dog", "bark,growl"), entry("bear", "roar"));
     assertThat(config.getDuration("test.duration")).isEqualTo(Duration.ofSeconds(1));
   }
 
@@ -54,7 +54,7 @@ void allValidUsingHyphens() {
         .containsExactly(entry("cat", "meow"), entry("dog", "bark"), entry("bear", "growl"));
     assertThat(config.getMap("test-map-commas"))
         .containsExactly(
-            entry("cat", "meow,hiss"), entry("dog", "bark,growl"), entry("bear", "growl,roar"));
+            entry("cat", "meow,hiss"), entry("dog", "bark,growl"), entry("bear", "roar"));
     assertThat(config.getDuration("test-duration")).isEqualTo(Duration.ofSeconds(1));
   }
 
@@ -178,17 +178,6 @@ void invalidMap() {
         .hasMessage("Invalid map property: map=a=1,=b");
   }
 
-  @Test
-  void invalidList() {
-    DefaultConfigProperties config =
-        DefaultConfigProperties.createFromMap(
-            Collections.singletonMap(
-                "invalid", "\"cat=meow,hiss\",\"dog=bark,growl\", bear=growl"));
-    assertThatThrownBy(() -> config.getList("invalid"))
-        .isInstanceOf(ConfigurationException.class)
-        .hasMessageContaining("Invalid list property");
-  }
-
   @Test
   void invalidDuration() {
     assertThatThrownBy(
@@ -297,7 +286,7 @@ private static Map<String, String> makeTestProps() {
     properties.put("test.list", "cat,dog,bear");
     properties.put("test.map", "cat=meow,dog=bark,bear=growl,bird=");
     properties.put("test.duration", "1s");
-    properties.put("test.map.commas", "\"cat=meow,hiss\",\"dog=bark,growl\", \"bear=growl,roar\"");
+    properties.put("test.map.commas", "cat=\"meow,hiss\",dog=\"bark,growl\", bear=roar");
     return properties;
   }
 }