Adjust algorithm to match marcalff's proposal

Author: jack-berg

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfiguration.java

@@ -29,6 +29,7 @@
 import java.util.regex.MatchResult;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import javax.annotation.Nullable;
 import org.snakeyaml.engine.v2.api.Load;
 import org.snakeyaml.engine.v2.api.LoadSettings;
 import org.snakeyaml.engine.v2.common.ScalarStyle;
@@ -51,7 +52,7 @@ public final class FileConfiguration {
 
   private static final Logger logger = Logger.getLogger(FileConfiguration.class.getName());
   private static final Pattern ENV_VARIABLE_REFERENCE =
-      Pattern.compile("\\${1,2}\\{([a-zA-Z_][a-zA-Z0-9_]*)(:-([^\n}]*))?}");
+      Pattern.compile("\\$\\{([a-zA-Z_][a-zA-Z0-9_]*)(:-([^\n}]*))?}");
   private static final ComponentLoader DEFAULT_COMPONENT_LOADER =
       SpiHelper.serviceComponentLoader(FileConfiguration.class.getClassLoader());
 
@@ -297,6 +298,10 @@ protected Map<Object, Object> constructMapping(MappingNode node) {
       return mapping;
     }
 
+    private static final String ESCAPE_SEQUENCE = "$$";
+    private static final int ESCAPE_SEQUENCE_LENGTH = ESCAPE_SEQUENCE.length();
+    private static final char ESCAPE_SEQUENCE_REPLACEMENT = '$';
+
     private Object constructValueObject(Node node) {
       Object value = constructObject(node);
       if (!(node instanceof ScalarNode)) {
@@ -307,44 +312,69 @@ private Object constructValueObject(Node node) {
       }
 
       String val = (String) value;
+      ScalarStyle scalarStyle = ((ScalarNode) node).getScalarStyle();
+
+      // Iterate through val left to right, search for escape sequence "$$"
+      // For the substring of val between the last escape sequence and the next found, perform
+      // environment variable substitution
+      // Add the escape replacement character '$' in place of each escape sequence found
+
+      int lastEscapeIndexEnd = 0;
+      StringBuilder newVal = null;
+      while (true) {
+        int escapeIndex = val.indexOf(ESCAPE_SEQUENCE, lastEscapeIndexEnd);
+        int substitutionEndIndex = escapeIndex == -1 ? val.length() : escapeIndex;
+        newVal = envVarSubstitution(newVal, val, lastEscapeIndexEnd, substitutionEndIndex);
+        if (escapeIndex == -1) {
+          break;
+        } else {
+          newVal.append(ESCAPE_SEQUENCE_REPLACEMENT);
+        }
+        lastEscapeIndexEnd = escapeIndex + ESCAPE_SEQUENCE_LENGTH;
+        if (lastEscapeIndexEnd >= val.length()) {
+          break;
+        }
+      }
+
+      // If the value was double quoted, retain the double quotes so we don't change a value
+      // intended to be a string to a different type after environment variable substitution
+      if (scalarStyle == ScalarStyle.DOUBLE_QUOTED) {
+        newVal.insert(0, "\"");
+        newVal.append("\"");
+      }
+      return load.loadFromString(newVal.toString());
+    }
+
+    private StringBuilder envVarSubstitution(
+        @Nullable StringBuilder newVal, String source, int startIndex, int endIndex) {
+      String val = source.substring(startIndex, endIndex);
       Matcher matcher = ENV_VARIABLE_REFERENCE.matcher(val);
+
       if (!matcher.find()) {
-        return value;
+        return newVal == null ? new StringBuilder(val) : newVal.append(val);
+      }
+
+      if (newVal == null) {
+        newVal = new StringBuilder();
       }
 
       int offset = 0;
-      StringBuilder newVal = new StringBuilder();
-      ScalarStyle scalarStyle = ((ScalarNode) node).getScalarStyle();
       do {
         MatchResult matchResult = matcher.toMatchResult();
-        newVal.append(val, offset, matchResult.start());
-
-        String ref = val.substring(matchResult.start(), matchResult.end());
-        // $$ indicates that env var reference is escaped. Strip the leading $.
-        if (ref.startsWith("$$")) {
-          newVal.append(ref.substring(1));
-        } else {
-          String envVarKey = matcher.group(1);
-          String defaultValue = matcher.group(3);
-          if (defaultValue == null) {
-            defaultValue = "";
-          }
-          String replacement = environmentVariables.getOrDefault(envVarKey, defaultValue);
-          newVal.append(replacement);
+        String envVarKey = matcher.group(1);
+        String defaultValue = matcher.group(3);
+        if (defaultValue == null) {
+          defaultValue = "";
         }
-
+        String replacement = environmentVariables.getOrDefault(envVarKey, defaultValue);
+        newVal.append(val, offset, matchResult.start()).append(replacement);
         offset = matchResult.end();
       } while (matcher.find());
       if (offset != val.length()) {
         newVal.append(val, offset, val.length());
       }
-      // If the value was double quoted, retain the double quotes so we don't change a value
-      // intended to be a string to a different type after environment variable substitution
-      if (scalarStyle == ScalarStyle.DOUBLE_QUOTED) {
-        newVal.insert(0, "\"");
-        newVal.append("\"");
-      }
-      return load.loadFromString(newVal.toString());
+
+      return newVal;
     }
   }
 }

sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfigurationParseTest.java

@@ -592,6 +592,7 @@ private static java.util.stream.Stream<Arguments> coreSchemaValuesArgs() {
   @MethodSource("envVarSubstitutionArgs")
   void envSubstituteAndLoadYaml(String rawYaml, Object expectedYamlResult) {
     Map<String, String> environmentVariables = new HashMap<>();
+    environmentVariables.put("FOO", "BAR");
     environmentVariables.put("STR_1", "value1");
     environmentVariables.put("STR_2", "value2");
     environmentVariables.put("EMPTY_STR", "");
@@ -658,8 +659,18 @@ private static java.util.stream.Stream<Arguments> envVarSubstitutionArgs() {
         Arguments.of("key1: \"${INT}\"\n", mapOf(entry("key1", "1"))),
         Arguments.of("key1: \"${FLOAT}\"\n", mapOf(entry("key1", "1.1"))),
         // Escaped
+        Arguments.of("key1: ${FOO}\n", mapOf(entry("key1", "BAR"))),
+        Arguments.of("key1: $${FOO}\n", mapOf(entry("key1", "${FOO}"))),
+        Arguments.of("key1: $$${FOO}\n", mapOf(entry("key1", "$BAR"))),
+        Arguments.of("key1: $$$${FOO}\n", mapOf(entry("key1", "$${FOO}"))),
+        Arguments.of("key1: a $$ b\n", mapOf(entry("key1", "a $ b"))),
+        Arguments.of("key1: $$ b\n", mapOf(entry("key1", "$ b"))),
+        Arguments.of("key1: a $$\n", mapOf(entry("key1", "a $"))),
+        Arguments.of("key1: a $ b\n", mapOf(entry("key1", "a $ b"))),
         Arguments.of("key1: $${STR_1}\n", mapOf(entry("key1", "${STR_1}"))),
-        Arguments.of("key1: $$${STR_1}\n", mapOf(entry("key1", "$${STR_1}"))),
+        Arguments.of("key1: $${STR_1}$${STR_1}\n", mapOf(entry("key1", "${STR_1}${STR_1}"))),
+        Arguments.of("key1: $${STR_1}$$\n", mapOf(entry("key1", "${STR_1}$"))),
+        Arguments.of("key1: $$${STR_1}\n", mapOf(entry("key1", "$value1"))),
         Arguments.of("key1: \"$${STR_1}\"\n", mapOf(entry("key1", "${STR_1}"))),
         Arguments.of("key1: $${STR_1} ${STR_2}\n", mapOf(entry("key1", "${STR_1} value2"))),
         Arguments.of("key1: $${STR_1} $${STR_2}\n", mapOf(entry("key1", "${STR_1} ${STR_2}"))),