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 <[email protected]> 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 + } + } + }
