Zhongxin Yan created LANG-1799:
----------------------------------
Summary: Non-final Static Singleton Variables Break Thread Safety
and Singleton Design in RandomStringUtils
Key: LANG-1799
URL: https://issues.apache.org/jira/browse/LANG-1799
Project: Commons Lang
Issue Type: Bug
Components: lang.*
Affects Versions: 3.20.0
Reporter: Zhongxin Yan
The RandomStringUtils class exposes three primary singleton instances
(INSECURE, SECURE, SECURE_STRONG) through static fields that are currently
missing the final keyword.
{code:java}
private static RandomStringUtils INSECURE = new
RandomStringUtils(RandomUtils::insecure);
private static RandomStringUtils SECURE = new
RandomStringUtils(SECURE_SUPPLIER);
private static RandomStringUtils SECURE_STRONG = new
RandomStringUtils(RandomUtils::secureStrong);
{code}
h3. Key Issues Caused by Non-final Modifier:
* {*}Unintended Reassignment{*}: The variables can be modified at runtime
(e.g., via reflection, accidental code changes, or subclassing), breaking the
singleton contract and leading to inconsistent random string generation
behavior across the application.
* {*}Violation of Documentation{*}: The class Javadoc explicitly labels it
{{{}#ThreadSafe#{}}}, but non-final static state undermines this promise.
{code:java}
// Test
/**
* Test case to demonstrate the security and robustness risk introduced
* by the non-final static singleton fields in RandomStringUtils.
* * NOTE: This test should fail when the code is fixed (i.e., when fields are
made final).
*/
public class RandomStringUtilsNonFinalSingletonTest {
private static final String FIELD_NAME = "SECURE";
/**
* Attempts to corrupt the static singleton reference via reflection.
*/
@Test
void testNonFinalStaticSingletonCorruption() throws Exception {
// 1. Get the non-final static field
Field secureField =
RandomStringUtils.class.getDeclaredField(FIELD_NAME);
// Ensure the field is NOT final (pre-condition for the bug)
assertFalse(java.lang.reflect.Modifier.isFinal(secureField.getModifiers()),
"Pre-condition failed: Field should not be final to demonstrate
the corruption risk.");
// 2. Override accessibility for private field access
secureField.setAccessible(true);
// 3. Store the original instance for cleanup
RandomStringUtils originalInstance = (RandomStringUtils)
secureField.get(null);
assertNotNull(originalInstance, "Original singleton must be
initialized.");
try {
// 4. CORRUPT THE SINGLETON REFERENCE ***
// Setting a static field: pass null as the first argument
secureField.set(null, null);
// 5. Verify corruption succeeded
assertNull(secureField.get(null), "Non-final static field was
successfully set to null.");
// 6. Assert that subsequent application logic fails with NPE
// This proves the integrity of the singleton has been compromised.
assertThrows(NullPointerException.class, () -> {
// The call to RandomStringUtils.secure() returns null, and
.next(1) fails.
RandomStringUtils.secure().next(1);
}, "Subsequent calls to the corrupted singleton must throw NPE.");
} finally {
// Cleanup: Restore the original instance to avoid breaking other
tests
if (secureField.get(null) == null) {
secureField.set(null, originalInstance);
}
}
}
}{code}
[github pr|https://github.com/apache/commons-lang/pull/1513]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
