From 8817af07fb72fd78fb753fe5d069d0a65a79742f Mon Sep 17 00:00:00 2001
From: Jude Melton-Houghton <jwmhjwmh@gmail.com>
Date: Thu, 24 Nov 2022 17:56:43 -0500
Subject: [PATCH] Deprecate special handling of `${key}` syntax in metadata
 values (#12970)

---
 builtin/common/misc_helpers.lua |  5 +++--
 doc/breakages.md                |  1 +
 doc/lua_api.txt                 |  5 +++++
 src/metadata.cpp                | 11 ++++++++---
 src/metadata.h                  |  2 +-
 src/util/string.h               |  1 +
 6 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/builtin/common/misc_helpers.lua b/builtin/common/misc_helpers.lua
index 720df3998..1a2b9500a 100644
--- a/builtin/common/misc_helpers.lua
+++ b/builtin/common/misc_helpers.lua
@@ -250,11 +250,12 @@ local formspec_escapes = {
 	["["] = "\\[",
 	["]"] = "\\]",
 	[";"] = "\\;",
-	[","] = "\\,"
+	[","] = "\\,",
+	["$"] = "\\$",
 }
 function core.formspec_escape(text)
 	-- Use explicit character set instead of dot here because it doubles the performance
-	return text and string.gsub(text, "[\\%[%];,]", formspec_escapes)
+	return text and string.gsub(text, "[\\%[%];,$]", formspec_escapes)
 end
 
 
diff --git a/doc/breakages.md b/doc/breakages.md
index 9f59b9705..52fe6b741 100644
--- a/doc/breakages.md
+++ b/doc/breakages.md
@@ -9,3 +9,4 @@ This document contains a list of breaking changes to be made in the next major v
 * remove `depends.txt` / `description.txt` (would simplify ContentDB and Minetest code a little)
 * rotate moon texture by 180°, making it coherent with the sun (see https://github.com/minetest/minetest/pull/11902)
 * remove undocumented `set_physics_override(num, num, num)`
+* remove special handling of `${key}` syntax in metadata values
diff --git a/doc/lua_api.txt b/doc/lua_api.txt
index 1630b06dc..1c8cc1022 100644
--- a/doc/lua_api.txt
+++ b/doc/lua_api.txt
@@ -6861,6 +6861,11 @@ Can be obtained via `item:get_meta()`.
 Base class used by [`StorageRef`], [`NodeMetaRef`], [`ItemStackMetaRef`],
 and [`PlayerMetaRef`].
 
+Note: If a metadata value is in the format `${k}`, an attempt to get the value
+will return the value associated with key `k`. There is a low recursion limit.
+This behavior is **deprecated** and will be removed in a future version. Usage
+of the `${k}` syntax in formspecs is not deprecated.
+
 ### Methods
 
 * `contains(key)`: Returns true if key present, otherwise false.
diff --git a/src/metadata.cpp b/src/metadata.cpp
index d8a41218d..f02495fa4 100644
--- a/src/metadata.cpp
+++ b/src/metadata.cpp
@@ -51,7 +51,7 @@ const std::string &IMetadata::getString(const std::string &name, std::string *pl
 		return empty_string;
 	}
 
-	return resolveString(*raw, place, recursion);
+	return resolveString(*raw, place, recursion, true);
 }
 
 bool IMetadata::getStringToRef(const std::string &name,
@@ -61,16 +61,21 @@ bool IMetadata::getStringToRef(const std::string &name,
 	if (!raw)
 		return false;
 
-	const std::string &resolved = resolveString(*raw, &str, recursion);
+	const std::string &resolved = resolveString(*raw, &str, recursion, true);
 	if (&resolved != &str)
 		str = resolved;
 	return true;
 }
 
 const std::string &IMetadata::resolveString(const std::string &str, std::string *place,
-		u16 recursion) const
+		u16 recursion, bool deprecated) const
 {
 	if (recursion <= 1 && str.substr(0, 2) == "${" && str[str.length() - 1] == '}') {
+		if (deprecated) {
+			warningstream << "Deprecated use of recursive resolution syntax in metadata: ";
+			safe_print_string(warningstream, str);
+			warningstream << std::endl;
+		}
 		// It may be the case that &str == place, but that's fine.
 		return getString(str.substr(2, str.length() - 3), place, recursion + 1);
 	}
diff --git a/src/metadata.h b/src/metadata.h
index 45a3774d2..1a0350bbf 100644
--- a/src/metadata.h
+++ b/src/metadata.h
@@ -65,7 +65,7 @@ public:
 
 	// Add support for variable names in values. Uses place like getString.
 	const std::string &resolveString(const std::string &str, std::string *place,
-			u16 recursion = 0) const;
+			u16 recursion = 0, bool deprecated = false) const;
 
 protected:
 	// Returns nullptr to indicate absence of value. Uses place like getString.
diff --git a/src/util/string.h b/src/util/string.h
index 80eea754c..4c7a4068d 100644
--- a/src/util/string.h
+++ b/src/util/string.h
@@ -459,6 +459,7 @@ inline void str_formspec_escape(std::string &str)
 	str_replace(str, "[", "\\[");
 	str_replace(str, ";", "\\;");
 	str_replace(str, ",", "\\,");
+	str_replace(str, "$", "\\$");
 }
 
 /**