This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new c8dc3121a StrSubstitutor (deprecated) recursive expansion has a cycle
check but no fan-out, depth, or size bound.
c8dc3121a is described below
commit c8dc3121a2d97a63c86eab02ae71a05d9f8066c0
Author: Gary Gregory <[email protected]>
AuthorDate: Sat Sep 5 09:08:22 2026 -0400
StrSubstitutor (deprecated) recursive expansion has a cycle check but no
fan-out, depth, or size bound.
Exponential acyclic expansion, StackOverflowError via nested variable
names (opt-in), undeclared IllegalStateException on cyclic maps.
---
src/changes/changes.xml | 1 +
.../apache/commons/lang3/text/StrSubstitutor.java | 62 +++++++++++++++++++++-
.../commons/lang3/text/StrSubstitutorTest.java | 41 ++++++++++++++
3 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 2e64418cf..2eb0909c8 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -262,6 +262,7 @@ java.lang.NullPointerException: Cannot invoke
<action type="fix" dev="ggregory" due-to="Gary
Gregory">FastDateParser.parse throws undeclared IllegalArgumentException,
NullPointerException, and IllegalStateException on crafted date strings
(f010).</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">Memoizer: default caches the first failure forever, has no size bound
or eviction, and runs the user computation inside the ConcurrentHashMap bin
lock (blocking unrelated keys, deadlocking reentrant use) (f011).</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">StringEscapeUtils.escapeEcmaScript() misses backtick/template-literal
(`, ${) and inline-script parser-state sequences (<!--, <script) - claim
'Deals correctly with quotes' is falsified by ES6 (f012).</action>
+ <action type="fix" dev="ggregory" due-to="Gary
Gregory">StrSubstitutor (deprecated) recursive expansion has a cycle check but
no fan-out, depth, or size bound (f013).</action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="Gary
Gregory">Add JavaVersion.JAVA_27.</action>
<action type="add" dev="ggregory" due-to="Gary
Gregory">Add SystemUtils.IS_JAVA_27.</action>
diff --git a/src/main/java/org/apache/commons/lang3/text/StrSubstitutor.java
b/src/main/java/org/apache/commons/lang3/text/StrSubstitutor.java
index e4f3e892e..7ddb9a8f5 100644
--- a/src/main/java/org/apache/commons/lang3/text/StrSubstitutor.java
+++ b/src/main/java/org/apache/commons/lang3/text/StrSubstitutor.java
@@ -170,6 +170,21 @@ public class StrSubstitutor {
*/
public static final StrMatcher DEFAULT_VALUE_DELIMITER =
StrMatcher.stringMatcher(":-");
+ /**
+ * The maximum nesting depth of variable interpolation. The
cyclic-substitution check only rejects a variable
+ * already on the current substitution stack; without a depth bound,
deeply nested (acyclic) references and
+ * nested variable names (when {@link #isEnableSubstitutionInVariables()}
is on) recurse once per level and
+ * can end in {@link StackOverflowError}.
+ */
+ private static final int MAX_SUBSTITUTION_DEPTH = 256;
+
+ /**
+ * The maximum total number of characters that variable replacement may
emit during one top-level substitution.
+ * Bounds exponential acyclic fan-out (each of N references expanding to N
more), which the cyclic-substitution
+ * check cannot see.
+ */
+ private static final int MAX_SUBSTITUTION_LENGTH = 16 * 1024 * 1024;
+
/**
* Replaces all the occurrences of variables in the given source object
with
* their matching values from the map.
@@ -268,6 +283,17 @@ public static String replaceSystemProperties(final Object
source) {
*/
private boolean preserveEscapes;
+ /**
+ * Current recursion depth of {@link #substitute(StrBuilder, int, int,
List)}. Like the rest of this class,
+ * not thread safe.
+ */
+ private int substitutionDepth;
+
+ /**
+ * Total number of characters emitted by variable replacement in the
current top-level substitution.
+ */
+ private long substitutionLength;
+
/**
* Creates a new instance with defaults for variable prefix and suffix
* and the escaping character.
@@ -1110,8 +1136,37 @@ protected boolean substitute(final StrBuilder buf, final
int offset, final int l
* @param priorVariables The stack keeping track of the replaced
variables, may be null.
* @return The length change that occurs, unless priorVariables is null
when the int
* represents a boolean flag as to whether any change occurred.
+ * @throws IllegalStateException if the interpolation exceeds {@value
#MAX_SUBSTITUTION_DEPTH} nesting levels or
+ * emits more than {@value #MAX_SUBSTITUTION_LENGTH} characters. These
budgets bound recursive expansion that the
+ * cyclic-substitution check cannot detect (acyclic fan-out, deep
nesting). This class is deprecated; the
+ * Apache Commons Text successor {@code StringSubstitutor} should receive
any richer treatment.
+ */
+ private int substitute(final StrBuilder buf, final int offset, final int
length, final List<String> priorVariables) {
+ if (substitutionDepth == 0) {
+ substitutionLength = 0;
+ }
+ if (substitutionDepth >= MAX_SUBSTITUTION_DEPTH) {
+ throw new IllegalStateException("Maximum interpolation depth (" +
MAX_SUBSTITUTION_DEPTH + ") exceeded in variable substitution");
+ }
+ substitutionDepth++;
+ try {
+ return substituteRecursive(buf, offset, length, priorVariables);
+ } finally {
+ substitutionDepth--;
+ }
+ }
+
+ /**
+ * Implements {@link #substitute(StrBuilder, int, int, List)}; only that
budget-enforcing wrapper may call this.
+ *
+ * @param buf The string builder to substitute into, not null.
+ * @param offset The start offset within the builder, must be valid.
+ * @param length The length within the builder to be processed, must be
valid.
+ * @param priorVariables The stack keeping track of the replaced
variables, may be null.
+ * @return The length change that occurs, unless priorVariables is null
when the int
+ * represents a boolean flag as to whether any change occurred.
*/
- private int substitute(final StrBuilder buf, final int offset, final int
length, List<String> priorVariables) {
+ private int substituteRecursive(final StrBuilder buf, final int offset,
final int length, List<String> priorVariables) {
final StrMatcher pfxMatcher = getVariablePrefixMatcher();
final StrMatcher suffMatcher = getVariableSuffixMatcher();
final char escape = getEscapeChar();
@@ -1201,6 +1256,11 @@ private int substitute(final StrBuilder buf, final int
offset, final int length,
final int varLen = varValue.length();
buf.replace(startPos, endPos, varValue);
altered = true;
+ substitutionLength += varLen;
+ if (substitutionLength >
MAX_SUBSTITUTION_LENGTH) {
+ throw new IllegalStateException("Maximum
interpolation size (" + MAX_SUBSTITUTION_LENGTH
+ + " characters) exceeded in
variable substitution");
+ }
int change = substitute(buf, startPos, varLen,
priorVariables);
change = change + varLen - (endPos - startPos);
pos += change;
diff --git
a/src/test/java/org/apache/commons/lang3/text/StrSubstitutorTest.java
b/src/test/java/org/apache/commons/lang3/text/StrSubstitutorTest.java
index b5a175680..c7e3b407f 100644
--- a/src/test/java/org/apache/commons/lang3/text/StrSubstitutorTest.java
+++ b/src/test/java/org/apache/commons/lang3/text/StrSubstitutorTest.java
@@ -211,6 +211,47 @@ void testCyclicReplacement() {
assertThrows(IllegalStateException.class, () -> sub2.replace("The
${animal} jumps over the ${target}."), "Cyclic replacement was not detected.");
}
+ /**
+ * Tests that deeply nested (acyclic) variable references hit the depth
budget with an
+ * {@link IllegalStateException} instead of recursing towards {@link
StackOverflowError}.
+ */
+ @Test
+ void testDeeplyNestedReplacementThrowsIllegalStateException() {
+ final Map<String, String> map = new HashMap<>();
+ for (int i = 0; i < 400; i++) {
+ map.put("v" + i, "${v" + (i + 1) + "}");
+ }
+ map.put("v400", "x");
+ final StrSubstitutor sub = new StrSubstitutor(map);
+ assertThrows(IllegalStateException.class, () -> sub.replace("${v0}"),
"Depth budget was not enforced.");
+ // shallow nesting still works, including after a budget-exceeded
failure (counters reset per top-level call)
+ assertEquals("x", sub.replace("${v398}"));
+ }
+
+ /**
+ * Tests that exponential acyclic fan-out (each variable expanding to many
more) hits the
+ * total-output-size budget with an {@link IllegalStateException} instead
of consuming
+ * unbounded CPU and memory. The cycle check cannot detect this shape (no
variable repeats
+ * on the substitution stack).
+ */
+ @Test
+ void testExponentialFanOutReplacementThrowsIllegalStateException() {
+ final Map<String, String> map = new HashMap<>();
+ final char[] leafChars = new char[8192];
+ java.util.Arrays.fill(leafChars, 'x');
+ map.put("a6", new String(leafChars));
+ for (int level = 5; level >= 0; level--) {
+ final StringBuilder value = new StringBuilder();
+ for (int i = 0; i < 10; i++) {
+ value.append("${a").append(level + 1).append("}");
+ }
+ map.put("a" + level, value.toString());
+ }
+ // full expansion would be 10^6 leaves * 8 KiB = ~8 GiB
+ final StrSubstitutor sub = new StrSubstitutor(map);
+ assertThrows(IllegalStateException.class, () -> sub.replace("${a0}"),
"Size budget was not enforced.");
+ }
+
@Test
void testDefaultValueDelimiters() {
final Map<String, String> map = new HashMap<>();
