From 10371925ec2b8b9915d52ffa905671f2dabccef4 Mon Sep 17 00:00:00 2001 From: Ivan Kasarov Date: Fri, 2 Aug 2024 14:01:22 +0300 Subject: [PATCH] fixed circular ref false positive during property resolution with mutliple references --- .../multiapps/common/test/Tester.java | 4 +- .../mta/resolvers/PropertiesResolver.java | 18 ++++++- .../mta/resolvers/PropertiesResolverTest.java | 49 ++++++++++--------- .../mta/resolvers/moduleProperties.yaml | 18 ++++++- 4 files changed, 60 insertions(+), 29 deletions(-) diff --git a/multiapps-common-test/src/main/java/org/cloudfoundry/multiapps/common/test/Tester.java b/multiapps-common-test/src/main/java/org/cloudfoundry/multiapps/common/test/Tester.java index 192b2ca3..bb0a6afb 100644 --- a/multiapps-common-test/src/main/java/org/cloudfoundry/multiapps/common/test/Tester.java +++ b/multiapps-common-test/src/main/java/org/cloudfoundry/multiapps/common/test/Tester.java @@ -98,8 +98,8 @@ private void validateFailure(Expectation expectation, Exception e) { } String exceptionMessage = e.getMessage(); assertTrue(exceptionMessage.contains(expectation.getExpectationAsString()), - MessageFormat.format("Exception's message doesn't match up! Expected [{0}] to contain [{1}]!", - expectation.getExpectationAsString(), exceptionMessage)); + MessageFormat.format("Exception's message doesn't match up! Expected [{0}] to contain [{1}]!", exceptionMessage, + expectation.getExpectationAsString())); } private Object loadResourceAsJsonObject(String resource) { diff --git a/multiapps-mta/src/main/java/org/cloudfoundry/multiapps/mta/resolvers/PropertiesResolver.java b/multiapps-mta/src/main/java/org/cloudfoundry/multiapps/mta/resolvers/PropertiesResolver.java index 8e0520d3..88b32046 100644 --- a/multiapps-mta/src/main/java/org/cloudfoundry/multiapps/mta/resolvers/PropertiesResolver.java +++ b/multiapps-mta/src/main/java/org/cloudfoundry/multiapps/mta/resolvers/PropertiesResolver.java @@ -88,7 +88,7 @@ private Object resolveReferences(String key, String value) { private StringBuilder resolveReferenceInPlace(String key, StringBuilder value, Reference reference) { String matchedPattern = reference.getMatchedPattern(); int patternStartIndex = value.indexOf(matchedPattern); - Object resolvedReference = resolveReferenceInContext(key, reference); + Object resolvedReference = resolveReferenceInContext(key, reference, resolutionContext != null); if (resolvedReference != null) { return value.replace(patternStartIndex, patternStartIndex + matchedPattern.length(), resolvedReference.toString()); } @@ -102,14 +102,24 @@ private boolean isSimpleReference(String value, List references) { } protected Object resolveReferenceInContext(String key, Reference reference) { + return resolveReferenceInContext(key, reference, false); + } + + protected Object resolveReferenceInContext(String key, Reference reference, boolean shouldBackupContext) { boolean resolutionContextWasCreated = false; + HashSet contextKeysBackup = null; if (resolutionContext == null) { resolutionContext = new ResolutionContext(NameUtil.getPrefixedName(prefix, key)); resolutionContextWasCreated = true; + } else if (shouldBackupContext) { + // if multiple refs are resolved sequentially in a value - backup and revert context after each ref + contextKeysBackup = new HashSet<>(resolutionContext.referencedKeys); } Object resolvedValue = resolveReference(reference); if (resolutionContextWasCreated) { resolutionContext = null; + } else if (contextKeysBackup != null) { + resolutionContext.setReferencedKeys(contextKeysBackup); } return resolvedValue; } @@ -220,7 +230,7 @@ private List detectReferences(String line) { private static class ResolutionContext { - private String rootKey; + private final String rootKey; private Set referencedKeys = new HashSet<>(); public ResolutionContext(String rootKey) { @@ -234,6 +244,10 @@ public void addToReferencedKeys(String key) { referencedKeys.add(key); } + public void setReferencedKeys(Set keys) { + this.referencedKeys = keys; + } + } } \ No newline at end of file diff --git a/multiapps-mta/src/test/java/org/cloudfoundry/multiapps/mta/resolvers/PropertiesResolverTest.java b/multiapps-mta/src/test/java/org/cloudfoundry/multiapps/mta/resolvers/PropertiesResolverTest.java index f538b777..880edff0 100644 --- a/multiapps-mta/src/test/java/org/cloudfoundry/multiapps/mta/resolvers/PropertiesResolverTest.java +++ b/multiapps-mta/src/test/java/org/cloudfoundry/multiapps/mta/resolvers/PropertiesResolverTest.java @@ -13,41 +13,42 @@ class PropertiesResolverTest { - protected static final PropertiesResolver testResolver = new PropertiesResolver(null, - null, - null, - "test-", - false, - Collections.emptySet()); + protected static final Map replacementValues = TestUtil.getMap("moduleProperties.yaml", PropertiesResolverTest.class); private final Tester tester = Tester.forClass(getClass()); static Stream testResolve() { - return Stream.of(Arguments.of("moduleProperties.yaml", "test-map/a-list/1", new Expectation("second-item")), - Arguments.of("moduleProperties.yaml", "test-list/0/foo", new Expectation("@foo")), - Arguments.of("moduleProperties.yaml", "test-list/0/foo/", new Expectation("@foo")), - Arguments.of("moduleProperties.yaml", "test-list/1", new Expectation("{fizz=@fizz, buzz=@buzz}")), - Arguments.of("moduleProperties.yaml", "test-list/1/buzz", new Expectation("@buzz")), - Arguments.of("moduleProperties.yaml", "test-list/2", new Expectation("a string in list")), - Arguments.of("moduleProperties.yaml", "hosts/0/", new Expectation("some host")), - Arguments.of("moduleProperties.yaml", "hosts/1.0", + return Stream.of(Arguments.of("list-in-map", "${test-map/a-list/1}", new Expectation("second-item")), + Arguments.of("map-in-list", "${test-list/0/foo}", new Expectation("@foo")), + Arguments.of("map-in-list-extra-slash", "${test-list/0/foo/}", new Expectation("@foo")), + Arguments.of("complex-value", "${test-list/1}", new Expectation("{buzz=@buzz, fizz=@fizz}")), + Arguments.of("simple-value", "${test-list/2}", new Expectation("a string in list")), + Arguments.of("first-in-list", "${hosts/0/}", new Expectation("some host")), + Arguments.of("bad-index", "${hosts/1.0}", new Expectation(Expectation.Type.EXCEPTION, "Unable to resolve \"test-#hosts/1.0\"")), - Arguments.of("moduleProperties.yaml", "test-list/10/foo/", + Arguments.of("too-high-index", "${test-list/10/foo/}", new Expectation(Expectation.Type.EXCEPTION, "Unable to resolve \"test-#test-list/10/foo/\"")), - Arguments.of("moduleProperties.yaml", "test-list//foo/", + Arguments.of("missing-index", "${test-list//foo/}", new Expectation(Expectation.Type.EXCEPTION, "Unable to resolve \"test-#test-list//foo/\"")), - Arguments.of("moduleProperties.yaml", "tricky-map/just a key", new Expectation("foo")), - Arguments.of("moduleProperties.yaml", "tricky-map/0/1", new Expectation("baz")), - Arguments.of("moduleProperties.yaml", "tricky-map/one/two/", new Expectation("why")), - Arguments.of("moduleProperties.yaml", "tricky-map/3/4/5", new Expectation("stop"))); + Arguments.of("corner-case-whitespace", "${tricky-map/just a key}", new Expectation("foo")), + Arguments.of("corner-case-map-and-list", "${tricky-map/0/1}", new Expectation("baz")), + Arguments.of("corner-case-slash-1", "${tricky-map/one/two/}", new Expectation("why")), + Arguments.of("corner-case-slash-2", "${tricky-map/3/4/5}", new Expectation("stop")), + Arguments.of("9-qux", "${NO_CIRCULAR_REF}", new Expectation("qux-qux-qux-qux-qux-qux-qux-qux-qux")), + Arguments.of("9-qux-direct", "${foo}-${bar}-${baz}", new Expectation("qux-qux-qux-qux-qux-qux-qux-qux-qux")), + Arguments.of("2-ha", "${NO_CIRCULAR_REF_IN_MAP/a}", new Expectation("ha-ha")), + Arguments.of("circular-ref", "${CIRCULAR_REF}", + new Expectation(Expectation.Type.EXCEPTION, "Circular reference detected in \"test-#circular-ref\"")), + Arguments.of("circular-ref-map", "${CIRCULAR_REF_MAP_IN_MAP/key/key}", new Expectation(Expectation.Type.EXCEPTION, + "Circular reference detected in \"test-#circular-ref-map\""))); } @ParameterizedTest @MethodSource - void testResolve(String modulePropertiesLocation, String parameterExpression, Expectation expectation) { - Map moduleProperties = TestUtil.getMap(modulePropertiesLocation, getClass()); - - tester.test(() -> testResolver.resolveReferenceInDepth(new Reference(null, parameterExpression), moduleProperties), expectation); + void testResolve(String parameterName, String parameterValue, Expectation expectation) { + PropertiesResolver testResolver = new PropertiesResolver(null, irrelevant -> replacementValues, ReferencePattern.PLACEHOLDER, + "test-", false, Collections.emptySet()); + tester.test(() -> testResolver.visit(parameterName, parameterValue), expectation); } } diff --git a/multiapps-mta/src/test/resources/org/cloudfoundry/multiapps/mta/resolvers/moduleProperties.yaml b/multiapps-mta/src/test/resources/org/cloudfoundry/multiapps/mta/resolvers/moduleProperties.yaml index fabc8f8e..dc32530a 100644 --- a/multiapps-mta/src/test/resources/org/cloudfoundry/multiapps/mta/resolvers/moduleProperties.yaml +++ b/multiapps-mta/src/test/resources/org/cloudfoundry/multiapps/mta/resolvers/moduleProperties.yaml @@ -22,4 +22,20 @@ tricky-map: ? "3/4" : "5": stop ? "3/4/5" - : this is actually unreachable \ No newline at end of file + : this is actually unreachable +foo: ${qux}-${baz}-${bar}-${baz}-${qux} +bar: ${baz}-${qux} +baz: ${qux} +qux: "qux" +NO_CIRCULAR_REF: ${foo}-${bar}-${baz} +NO_CIRCULAR_REF_IN_MAP: + a: ${NO_CIRCULAR_REF_IN_MAP/b}-${NO_CIRCULAR_REF_IN_MAP/c} + b: ${NO_CIRCULAR_REF_IN_MAP/c} + c: "ha" +head: ${baz}-${body} +body: ${baz}-${tail} +tail: ${baz}-${head} +CIRCULAR_REF: ${foo}-${head} +CIRCULAR_REF_MAP_IN_MAP: + key: + key: ${head} \ No newline at end of file