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 4c87656ee [LANG-1837] Prevent null returns from lazily initialized
string caches
4c87656ee is described below
commit 4c87656ee569063e44d9368ee6f7814a0c0a2d43
Author: Gary Gregory <[email protected]>
AuthorDate: Thu Oct 15 17:36:35 2026 -0400
[LANG-1837] Prevent null returns from lazily initialized string caches
Apply the fix from PR #1811 to read each cache once into a local
variable in CharRange.toString(), Range.toString(), Fraction.toString(),
and Fraction.toProperString(). This prevents a second unsynchronized
read from observing null after the first read observed a cached value.
Improve existing tests to verify reuse of the first returned string
across formatting branches. Add inline comments explaining the cache
reuse assertions. These checks strengthen functional coverage but do
not deterministically reproduce the concurrency bug.
---
src/changes/changes.xml | 1 +
.../java/org/apache/commons/lang3/CharRange.java | 9 +++--
src/main/java/org/apache/commons/lang3/Range.java | 9 +++--
.../org/apache/commons/lang3/math/Fraction.java | 28 +++++++++------
.../org/apache/commons/lang3/CharRangeTest.java | 41 +++++++++++++++++-----
.../java/org/apache/commons/lang3/RangeTest.java | 2 ++
.../apache/commons/lang3/math/FractionTest.java | 40 +++++++++++++++------
7 files changed, 95 insertions(+), 35 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index efd0444fb..ba5b90c4f 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -58,6 +58,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="alhuda, Gary
Gregory">Keep RandomUtils's randomDouble() and randomFloat() below the
exclusive upper bound (#1817).</action>
<action type="fix" dev="ggregory" due-to="alhuda, Gary
Gregory">Use standalone month names for month-only patterns in FastDatePrinter
(#1815).</action>
<action type="fix" dev="ggregory" due-to="XenoAmess,
Gary Gregory">Fix TypeUtils.isAssignable rejecting Class<X> to
Class<?> assignments (#1820).</action>
+ <action issue="LANG-1837" type="fix" dev="ggregory" due-to="Aosen Xiong,
Gary Gregory">Prevent null returns from lazily initialized string
caches.</action>
<!-- ADD -->
<!-- UPDATE -->
</release>
diff --git a/src/main/java/org/apache/commons/lang3/CharRange.java
b/src/main/java/org/apache/commons/lang3/CharRange.java
index d9a861ce1..c532d6a2b 100644
--- a/src/main/java/org/apache/commons/lang3/CharRange.java
+++ b/src/main/java/org/apache/commons/lang3/CharRange.java
@@ -424,7 +424,9 @@ private void readObject(final ObjectInputStream in) throws
IOException, ClassNot
*/
@Override
public String toString() {
- if (iToString == null) {
+ // Read the lazily initialized field only once: a second racy read may
see null.
+ String result = iToString;
+ if (result == null) {
final StringBuilder buf = new StringBuilder(4);
if (isNegated()) {
buf.append('^');
@@ -434,8 +436,9 @@ public String toString() {
buf.append('-');
buf.append(end);
}
- iToString = buf.toString();
+ result = buf.toString();
+ iToString = result;
}
- return iToString;
+ return result;
}
}
diff --git a/src/main/java/org/apache/commons/lang3/Range.java
b/src/main/java/org/apache/commons/lang3/Range.java
index b1ca03e71..66e965aaa 100644
--- a/src/main/java/org/apache/commons/lang3/Range.java
+++ b/src/main/java/org/apache/commons/lang3/Range.java
@@ -645,10 +645,13 @@ private void readObject(final ObjectInputStream in)
throws IOException, ClassNot
*/
@Override
public String toString() {
- if (toString == null) {
- toString = "[" + minimum + ".." + maximum + "]";
+ // Read the lazily initialized field only once: a second racy read may
see null.
+ String result = toString;
+ if (result == null) {
+ result = "[" + minimum + ".." + maximum + "]";
+ toString = result;
}
- return toString;
+ return result;
}
/**
diff --git a/src/main/java/org/apache/commons/lang3/math/Fraction.java
b/src/main/java/org/apache/commons/lang3/math/Fraction.java
index 5e5d1d95c..7a8807074 100644
--- a/src/main/java/org/apache/commons/lang3/math/Fraction.java
+++ b/src/main/java/org/apache/commons/lang3/math/Fraction.java
@@ -922,13 +922,15 @@ public Fraction subtract(final Fraction fraction) {
* @return A {@link String} form of the fraction
*/
public String toProperString() {
- if (toProperString == null) {
+ // Read the lazily initialized field only once: a second racy read may
see null.
+ String result = toProperString;
+ if (result == null) {
if (numerator == 0) {
- toProperString = "0";
+ result = "0";
} else if (numerator == denominator) {
- toProperString = "1";
+ result = "1";
} else if (numerator == -1 * denominator) {
- toProperString = "-1";
+ result = "-1";
} else if ((numerator > 0 ? -numerator : numerator) <
-denominator) {
// note that we do the magnitude comparison test above with
// NEGATIVE (not positive) numbers, since negative numbers
@@ -936,15 +938,16 @@ public String toProperString() {
// is handled incorrectly.
final int properNumerator = getProperNumerator();
if (properNumerator == 0) {
- toProperString = Integer.toString(getProperWhole());
+ result = Integer.toString(getProperWhole());
} else {
- toProperString = getProperWhole() + " " + properNumerator
+ "/" + getDenominator();
+ result = getProperWhole() + " " + properNumerator + "/" +
getDenominator();
}
} else {
- toProperString = getNumerator() + "/" + getDenominator();
+ result = getNumerator() + "/" + getDenominator();
}
+ toProperString = result;
}
- return toProperString;
+ return result;
}
/**
@@ -957,9 +960,12 @@ public String toProperString() {
*/
@Override
public String toString() {
- if (toString == null) {
- toString = getNumerator() + "/" + getDenominator();
+ // Read the lazily initialized field only once: a second racy read may
see null.
+ String result = toString;
+ if (result == null) {
+ result = getNumerator() + "/" + getDenominator();
+ toString = result;
}
- return toString;
+ return result;
}
}
diff --git a/src/test/java/org/apache/commons/lang3/CharRangeTest.java
b/src/test/java/org/apache/commons/lang3/CharRangeTest.java
index d7dde92e0..5b564dc6b 100644
--- a/src/test/java/org/apache/commons/lang3/CharRangeTest.java
+++ b/src/test/java/org/apache/commons/lang3/CharRangeTest.java
@@ -23,6 +23,7 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -50,7 +51,10 @@ void testConstructorAccessors_is() {
assertEquals('a', rangea.getStart());
assertEquals('a', rangea.getEnd());
assertFalse(rangea.isNegated());
- assertEquals("a", rangea.toString());
+ final String str = rangea.toString();
+ assertEquals("a", str);
+ // Subsequent calls reuse the string cached by the first call.
+ assertSame(str, rangea.toString());
}
@Test
@@ -59,7 +63,10 @@ void testConstructorAccessors_isIn_Normal() {
assertEquals('a', rangea.getStart());
assertEquals('e', rangea.getEnd());
assertFalse(rangea.isNegated());
- assertEquals("a-e", rangea.toString());
+ final String str = rangea.toString();
+ assertEquals("a-e", str);
+ // Subsequent calls reuse the string cached by the first call.
+ assertSame(str, rangea.toString());
}
@Test
@@ -68,7 +75,10 @@ void testConstructorAccessors_isIn_Reversed() {
assertEquals('a', rangea.getStart());
assertEquals('e', rangea.getEnd());
assertFalse(rangea.isNegated());
- assertEquals("a-e", rangea.toString());
+ final String str = rangea.toString();
+ assertEquals("a-e", str);
+ // Subsequent calls reuse the string cached by the first call.
+ assertSame(str, rangea.toString());
}
@Test
@@ -77,7 +87,10 @@ void testConstructorAccessors_isIn_Same() {
assertEquals('a', rangea.getStart());
assertEquals('a', rangea.getEnd());
assertFalse(rangea.isNegated());
- assertEquals("a", rangea.toString());
+ final String str = rangea.toString();
+ assertEquals("a", str);
+ // Subsequent calls reuse the string cached by the first call.
+ assertSame(str, rangea.toString());
}
@Test
@@ -86,7 +99,10 @@ void testConstructorAccessors_isNot() {
assertEquals('a', rangea.getStart());
assertEquals('a', rangea.getEnd());
assertTrue(rangea.isNegated());
- assertEquals("^a", rangea.toString());
+ final String str = rangea.toString();
+ assertEquals("^a", str);
+ // Subsequent calls reuse the string cached by the first call.
+ assertSame(str, rangea.toString());
}
@Test
@@ -95,7 +111,10 @@ void testConstructorAccessors_isNotIn_Normal() {
assertEquals('a', rangea.getStart());
assertEquals('e', rangea.getEnd());
assertTrue(rangea.isNegated());
- assertEquals("^a-e", rangea.toString());
+ final String str = rangea.toString();
+ assertEquals("^a-e", str);
+ // Subsequent calls reuse the string cached by the first call.
+ assertSame(str, rangea.toString());
}
@Test
@@ -104,7 +123,10 @@ void testConstructorAccessors_isNotIn_Reversed() {
assertEquals('a', rangea.getStart());
assertEquals('e', rangea.getEnd());
assertTrue(rangea.isNegated());
- assertEquals("^a-e", rangea.toString());
+ final String str = rangea.toString();
+ assertEquals("^a-e", str);
+ // Subsequent calls reuse the string cached by the first call.
+ assertSame(str, rangea.toString());
}
@Test
@@ -113,7 +135,10 @@ void testConstructorAccessors_isNotIn_Same() {
assertEquals('a', rangea.getStart());
assertEquals('a', rangea.getEnd());
assertTrue(rangea.isNegated());
- assertEquals("^a", rangea.toString());
+ final String str = rangea.toString();
+ assertEquals("^a", str);
+ // Subsequent calls reuse the string cached by the first call.
+ assertSame(str, rangea.toString());
}
@Test
diff --git a/src/test/java/org/apache/commons/lang3/RangeTest.java
b/src/test/java/org/apache/commons/lang3/RangeTest.java
index 611a89a52..df9f48128 100644
--- a/src/test/java/org/apache/commons/lang3/RangeTest.java
+++ b/src/test/java/org/apache/commons/lang3/RangeTest.java
@@ -419,6 +419,8 @@ void testToString() {
final String str = intRange.toString();
assertEquals("[10..20]", str);
+ // Subsequent calls reuse the string cached by the first call.
+ assertSame(str, intRange.toString());
assertEquals("[-20..-10]", Range.between(-20, -10).toString());
}
diff --git a/src/test/java/org/apache/commons/lang3/math/FractionTest.java
b/src/test/java/org/apache/commons/lang3/math/FractionTest.java
index aac15b782..dca860ea7 100644
--- a/src/test/java/org/apache/commons/lang3/math/FractionTest.java
+++ b/src/test/java/org/apache/commons/lang3/math/FractionTest.java
@@ -1240,35 +1240,55 @@ void testToProperString() {
Fraction f;
f = Fraction.getFraction(3, 5);
- final String str = f.toProperString();
+ String str = f.toProperString();
assertEquals("3/5", str);
+ // Subsequent calls reuse the string cached by the first call.
assertSame(str, f.toProperString());
f = Fraction.getFraction(7, 5);
- assertEquals("1 2/5", f.toProperString());
+ str = f.toProperString();
+ assertEquals("1 2/5", str);
+ assertSame(str, f.toProperString());
f = Fraction.getFraction(14, 10);
- assertEquals("1 4/10", f.toProperString());
+ str = f.toProperString();
+ assertEquals("1 4/10", str);
+ assertSame(str, f.toProperString());
f = Fraction.getFraction(4, 2);
- assertEquals("2", f.toProperString());
+ str = f.toProperString();
+ assertEquals("2", str);
+ assertSame(str, f.toProperString());
f = Fraction.getFraction(0, 2);
- assertEquals("0", f.toProperString());
+ str = f.toProperString();
+ assertEquals("0", str);
+ assertSame(str, f.toProperString());
f = Fraction.getFraction(2, 2);
- assertEquals("1", f.toProperString());
+ str = f.toProperString();
+ assertEquals("1", str);
+ assertSame(str, f.toProperString());
f = Fraction.getFraction(-7, 5);
- assertEquals("-1 2/5", f.toProperString());
+ str = f.toProperString();
+ assertEquals("-1 2/5", str);
+ assertSame(str, f.toProperString());
f = Fraction.getFraction(Integer.MIN_VALUE, 0, 1);
- assertEquals("-2147483648", f.toProperString());
+ str = f.toProperString();
+ assertEquals("-2147483648", str);
+ assertSame(str, f.toProperString());
f = Fraction.getFraction(-1, 1, Integer.MAX_VALUE);
- assertEquals("-1 1/2147483647", f.toProperString());
+ str = f.toProperString();
+ assertEquals("-1 1/2147483647", str);
+ assertSame(str, f.toProperString());
- assertEquals("-1", Fraction.getFraction(-1).toProperString());
+ f = Fraction.getFraction(-1);
+ str = f.toProperString();
+ assertEquals("-1", str);
+ assertSame(str, f.toProperString());
}
@Test
