fix: regression - partially-shadowed object has nested delayed merge

Author: johanandren

config/src/main/java/com/typesafe/config/impl/ConfigDelayedMerge.java

@@ -6,9 +6,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
 
 import com.typesafe.config.ConfigException;
 import com.typesafe.config.ConfigOrigin;
@@ -134,20 +132,19 @@ else if (end instanceof Unmergeable) {
                 sourceForEnd = source.pushParent(replaceable);
 
                 // Same spec rule as the short-circuit above, applied per-key:
-                // keys in the lower-priority 'end' object that are already
-                // shadowed in 'merged' by a value ignoring fallbacks would be
-                // discarded by the subsequent merge, so don't resolve them.
-                if (merged instanceof AbstractConfigObject && end instanceof AbstractConfigObject) {
-                    AbstractConfigObject prunedEnd = pruneShadowedKeys(
-                            (AbstractConfigObject) end, (AbstractConfigObject) merged);
-                    if (prunedEnd == null) {
-                        if (ConfigImpl.traceSubstitutionsEnabled())
-                            ConfigImpl.trace(newContext.depth(),
-                                    "all keys in end are shadowed by merged, skipping");
-                        count += 1;
-                        continue;
-                    }
-                    end = prunedEnd;
+                // if every key in the lower-priority 'end' object is already
+                // shadowed in 'merged' by a value ignoring fallbacks, the whole
+                // 'end' would be discarded by the subsequent merge. Skip
+                // resolving it. (We only skip the whole entry — substituting a
+                // partial copy of 'end' would change identity and break parent
+                // chain walks during inner substitution resolution.)
+                if (merged instanceof AbstractConfigObject && end instanceof SimpleConfigObject
+                        && allKeysShadowed((SimpleConfigObject) end, (AbstractConfigObject) merged)) {
+                    if (ConfigImpl.traceSubstitutionsEnabled())
+                        ConfigImpl.trace(newContext.depth(),
+                                "all keys in end are shadowed by merged, skipping");
+                    count += 1;
+                    continue;
                 }
             }
 
@@ -181,36 +178,22 @@ else if (end instanceof Unmergeable) {
         return ResolveResult.make(newContext, merged);
     }
 
-    // Returns a copy of 'end' with keys removed when 'merged' already has a
-    // value at that key which ignores fallbacks. Returns null if every key in
-    // 'end' is shadowed. Returns 'end' unchanged if no pruning applies or if
-    // we cannot safely inspect merged (e.g. unresolved CDMO).
-    private static AbstractConfigObject pruneShadowedKeys(AbstractConfigObject end,
-            AbstractConfigObject merged) {
-        if (!(end instanceof SimpleConfigObject))
-            return end;
-        SimpleConfigObject simple = (SimpleConfigObject) end;
-        Map<String, AbstractConfigValue> kept = new LinkedHashMap<String, AbstractConfigValue>();
-        boolean pruned = false;
-        for (String key : simple.keySet()) {
+    // True when every key in 'end' is shadowed by a value in 'merged' that
+    // ignores fallbacks (so the subsequent merge would drop the whole 'end').
+    private static boolean allKeysShadowed(SimpleConfigObject end, AbstractConfigObject merged) {
+        if (end.isEmpty())
+            return false;
+        for (String key : end.keySet()) {
             AbstractConfigValue mergedValue;
             try {
                 mergedValue = merged.attemptPeekWithPartialResolve(key);
             } catch (ConfigException.NotResolved e) {
-                return end;
-            }
-            if (mergedValue != null && mergedValue.ignoresFallbacks()) {
-                pruned = true;
-            } else {
-                kept.put(key, simple.attemptPeekWithPartialResolve(key));
+                return false;
             }
+            if (mergedValue == null || !mergedValue.ignoresFallbacks())
+                return false;
         }
-        if (!pruned)
-            return end;
-        if (kept.isEmpty())
-            return null;
-        return new SimpleConfigObject(end.origin(), kept,
-                ResolveStatus.fromValues(kept.values()), false);
+        return true;
     }
 
     @Override

config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala

@@ -1343,4 +1343,42 @@ class ConfigSubstitutionTest extends TestUtils {
         val resolved = resolve(obj)
         assertEquals(42, resolved.getInt("p"))
     }
+
+    // Regression: when a delayed-merge object's stack contains a SimpleConfigObject
+    // whose keys are partially shadowed by a higher-priority entry, pruning
+    // produces a SimpleConfigObject containing only the kept keys. If a kept key
+    // holds a nested ConfigDelayedMerge with an unmergeable element, resolving
+    // that inner CDM walks up the parent chain and reaches the outer CDMO, which
+    // still has the original (unpruned) entry in its stack — leading to
+    // ConfigException.BugOrBroken("tried to replace ... which is not in [...]").
+    @Test
+    def partiallyShadowedObjectWithInnerDelayedMergeResolves() {
+        // ${overrides} between the two `db:` definitions forces `db` into a
+        // ConfigDelayedMergeObject (the merge cannot happen at parse time).
+        // The lower-priority entry (defined first) has `database`, `user`, and
+        // `ssl`. The higher-priority entry (defined last) shadows `database`
+        // and `user` but does not define `ssl`, so pruning yields {ssl: …}.
+        // `ssl.cert` is a CDM stack of an optional substitution and "default",
+        // whose resolution walks back through the path to the outer CDMO.
+        val obj = parseObject("""
+            db: {
+              database: "fallback"
+              user: "fallback"
+              ssl: {
+                cert: "default"
+                cert: ${?SSL_CERT}
+              }
+            }
+            db: ${overrides}
+            db: {
+              database: "primary"
+              user: "admin"
+            }
+            overrides: {}
+        """)
+        val resolved = resolve(obj)
+        assertEquals("primary", resolved.getString("db.database"))
+        assertEquals("admin", resolved.getString("db.user"))
+        assertEquals("default", resolved.getString("db.ssl.cert"))
+    }
 }