From efa072204af0f43b7eb75fa10b878b844ff9b0ff Mon Sep 17 00:00:00 2001
From: Phillip Webb <pwebb@vmware.com>
Date: Fri, 16 Jun 2023 16:12:39 -0700
Subject: [PATCH] Don't migrate properties that cause a circular reference

Update `PropertiesMigrationReporter` so that properties are only
migrated automatically when they don't cause a circular reference.

Fixes gh-35919
---
 .../migrator/PropertiesMigrationReporter.java | 57 ++++++++++++++++---
 .../PropertiesMigrationReporterTests.java     |  8 +++
 2 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/spring-boot-project/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java b/spring-boot-project/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java
index 8e495f8eb1..231c51e354 100644
--- a/spring-boot-project/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java
+++ b/spring-boot-project/spring-boot-properties-migrator/src/main/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporter.java
@@ -17,9 +17,11 @@
 package org.springframework.boot.context.properties.migrator;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
@@ -86,14 +88,26 @@ class PropertiesMigrationReporter {
 		if (renamed.isEmpty()) {
 			return null;
 		}
-		String target = "migrate-" + name;
-		Map<String, OriginTrackedValue> content = new LinkedHashMap<>();
-		for (PropertyMigration candidate : renamed) {
-			OriginTrackedValue value = OriginTrackedValue.of(candidate.getProperty().getValue(),
-					candidate.getProperty().getOrigin());
-			content.put(candidate.getNewPropertyName(), value);
+		NameTrackingPropertySource nameTrackingPropertySource = new NameTrackingPropertySource();
+		this.environment.getPropertySources().addFirst(nameTrackingPropertySource);
+		try {
+			String target = "migrate-" + name;
+			Map<String, OriginTrackedValue> content = new LinkedHashMap<>();
+			for (PropertyMigration candidate : renamed) {
+				String newPropertyName = candidate.getNewPropertyName();
+				Object value = candidate.getProperty().getValue();
+				if (nameTrackingPropertySource.isPlaceholderThatAccessesName(value, newPropertyName)) {
+					continue;
+				}
+				OriginTrackedValue originTrackedValue = OriginTrackedValue.of(value,
+						candidate.getProperty().getOrigin());
+				content.put(newPropertyName, originTrackedValue);
+			}
+			return new OriginTrackedMapPropertySource(target, content);
+		}
+		finally {
+			this.environment.getPropertySources().remove(nameTrackingPropertySource.getName());
 		}
-		return new OriginTrackedMapPropertySource(target, content);
 	}
 
 	private boolean isMapType(ConfigurationMetadataProperty property) {
@@ -172,4 +186,33 @@ class PropertiesMigrationReporter {
 		return source.getUnderlyingSource().toString();
 	}
 
+	/**
+	 * {@link PropertySource} used to track accessed properties to protect against
+	 * circular references.
+	 */
+	private class NameTrackingPropertySource extends PropertySource<Object> {
+
+		private final Set<String> accessedNames = new HashSet<>();
+
+		NameTrackingPropertySource() {
+			super(NameTrackingPropertySource.class.getName());
+		}
+
+		boolean isPlaceholderThatAccessesName(Object value, String name) {
+			if (value instanceof String) {
+				this.accessedNames.clear();
+				PropertiesMigrationReporter.this.environment.resolvePlaceholders((String) value);
+				return this.accessedNames.contains(name);
+			}
+			return false;
+		}
+
+		@Override
+		public Object getProperty(String name) {
+			this.accessedNames.add(name);
+			return null;
+		}
+
+	}
+
 }
diff --git a/spring-boot-project/spring-boot-properties-migrator/src/test/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporterTests.java b/spring-boot-project/spring-boot-properties-migrator/src/test/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporterTests.java
index 815f9f7f4b..3de49fe366 100644
--- a/spring-boot-project/spring-boot-properties-migrator/src/test/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporterTests.java
+++ b/spring-boot-project/spring-boot-properties-migrator/src/test/java/org/springframework/boot/context/properties/migrator/PropertiesMigrationReporterTests.java
@@ -205,6 +205,14 @@ class PropertiesMigrationReporterTests {
 			.contains("Replacement: custom.the-map-replacement.key");
 	}
 
+	@Test // gh-35919
+	void directCircularReference() {
+		this.environment.getPropertySources()
+			.addFirst(new MapPropertySource("backcompat", Collections.singletonMap("wrong.two", "${test.two}")));
+		createAnalyzer(loadRepository("metadata/sample-metadata.json")).getReport();
+		assertThat(this.environment.getProperty("test.two")).isNull();
+	}
+
 	private List<String> mapToNames(PropertySources sources) {
 		List<String> names = new ArrayList<>();
 		for (PropertySource<?> source : sources) {