(logging-flume) branch fix/map-resolver created (now 3053b7442)

[pk...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ppkarwasz pushed a change to branch fix/map-resolver
in repository https://gitbox.apache.org/repos/asf/logging-flume.git


      at 3053b7442 Fix order-dependent resolution of cross-referencing properties

This branch includes the following new commits:

     new 3053b7442 Fix order-dependent resolution of cross-referencing properties

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

(logging-flume) 01/01: Fix order-dependent resolution of cross-referencing properties

[pk...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ppkarwasz pushed a commit to branch fix/map-resolver
in repository https://gitbox.apache.org/repos/asf/logging-flume.git

commit 3053b74423ff2bb874e4ed7640eb561a22bdc8ea
Author: Piotr P. Karwasz <pk...@apache.org>
AuthorDate: Fri May 29 22:14:20 2026 +0200

    Fix order-dependent resolution of cross-referencing properties
    
    When properties are defined in terms of each other, e.g. `a = ${b}` with `b = OK`, `MapResolver#resolveProperties` filled the result map while using that same map as the lookup source, so a reference only resolved when the referenced property had already been evaluated. As the iteration order of `Properties#stringPropertyNames` is unspecified, properties like `a` were sometimes left unresolved.
    
    Pre-seed the map with the raw values so that every key is visible to the lookup regardless of evaluation order.
    
    `TestMapResolver` covers both reference orderings, and documents that self-referencing and cyclic definitions are left to Commons Text's recursion guard, which either returns the literal or throws `IllegalStateException`.
---
 .../java/org/apache/flume/node/MapResolver.java    |  4 +++
 .../org/apache/flume/node/TestMapResolver.java     | 40 ++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/flume-ng-node/src/main/java/org/apache/flume/node/MapResolver.java b/flume-ng-node/src/main/java/org/apache/flume/node/MapResolver.java
index ea933ec0d..2af9bfc67 100644
--- a/flume-ng-node/src/main/java/org/apache/flume/node/MapResolver.java
+++ b/flume-ng-node/src/main/java/org/apache/flume/node/MapResolver.java
@@ -56,6 +56,9 @@ final class MapResolver {
 
   public static Map<String, String> resolveProperties(Properties properties) {
     Map<String, String> map = new HashMap<>();
+    // Pre-seed the map, so that evaluation doesn't depend on the order of Properties#stringPropertyNames
+    properties.forEach((key, value) -> map.put(key.toString(), value.toString()));
+
     boolean useEnvVars = ENV_VAR_PROPERTY.equals(System.getProperty(PROPS_IMPL_KEY));
     StringLookup defaultLookup = useEnvVars ? new DefaultLookup(map) :
         StringLookupFactory.INSTANCE.mapStringLookup(map);
@@ -63,6 +66,7 @@ final class MapResolver {
         defaultLookup, false);
     StringSubstitutor substitutor = new StringSubstitutor(lookup);
     substitutor.setEnableSubstitutionInVariables(true);
+
     properties.stringPropertyNames().forEach((k) -> map.put(k,
         substitutor.replace(properties.getProperty(k))));
     return map;
diff --git a/flume-ng-node/src/test/java/org/apache/flume/node/TestMapResolver.java b/flume-ng-node/src/test/java/org/apache/flume/node/TestMapResolver.java
index 9ba57ed8d..b8b9d753b 100644
--- a/flume-ng-node/src/test/java/org/apache/flume/node/TestMapResolver.java
+++ b/flume-ng-node/src/test/java/org/apache/flume/node/TestMapResolver.java
@@ -80,4 +80,44 @@ public class TestMapResolver {
     assertEquals("Test lookup was not resolved", "Value", test);
   }
 
+  @Test
+  public void testOrderOfEvaluation() {
+    // Tests that the evaluation order does not depend
+    // on the order of Properties#propertyNames
+    Properties properties = new Properties();
+    properties.setProperty("a", "${b}");
+    properties.setProperty("b", "OK");
+    Map<String, String> resolveProperties = MapResolver.resolveProperties(properties);
+    assertEquals("Incorrect order of evaluation", "OK", resolveProperties.get("a"));
+
+    properties = new Properties();
+    properties.setProperty("b", "${a}");
+    properties.setProperty("a", "OK");
+    resolveProperties = MapResolver.resolveProperties(properties);
+    assertEquals("Incorrect order of evaluation", "OK", resolveProperties.get("b"));
+  }
+
+  @Test
+  public void testDoesNotResolveRecursiveLookup() {
+    // Commons Text has a self-recursion guard
+    Properties props = new Properties();
+    props.setProperty("a", "${a}");
+    try {
+        // If it does throw, it should return the definition
+        assertEquals("${a}", MapResolver.resolveProperties(props).get("a"));
+    } catch (IllegalStateException ignored) {
+        // or it can throw
+    }
+
+    props = new Properties();
+    props.setProperty("a", "${b}");
+    props.setProperty("b", "${a}");
+      try {
+          // If it does throw, it should return the definition
+          assertEquals("${b}", MapResolver.resolveProperties(props).get("a"));
+      } catch (IllegalStateException ignored) {
+          // or it can throw
+      }
+  }
+
 }