This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-crypto.git
The following commit(s) were added to refs/heads/master by this push:
new f0c92d5 [CRYPTO-169][FOLLOWUP] Fix reentrancy of
`CryptoRandomFactory#getCryptoRandom` (#259)
f0c92d5 is described below
commit f0c92d52ab1b5f91a0b6aae1ce6df9c2b9ec2656
Author: YangJie <yangji...@baidu.com>
AuthorDate: Mon Oct 23 19:25:11 2023 +0800
[CRYPTO-169][FOLLOWUP] Fix reentrancy of
`CryptoRandomFactory#getCryptoRandom` (#259)
* fix reentrancy of CryptoRandomFactory#getCryptoRandom
* java 8 and 11
* add INIT_ERROR_CLASSES to ReflectionUtils
* revert import format
* fix import
* declare ConcurrentMap
* reuse test
---
.../commons/crypto/random/CryptoRandomFactory.java | 8 --------
.../commons/crypto/utils/ReflectionUtils.java | 21 ++++++++++++++++++++-
.../crypto/random/CryptoRandomFactoryTest.java | 7 +++++--
3 files changed, 25 insertions(+), 11 deletions(-)
diff --git
a/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java
b/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java
index 8239ae0..8acdf9c 100644
--- a/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java
+++ b/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java
@@ -198,14 +198,6 @@ public class CryptoRandomFactory {
} catch (final Exception e) {
lastException = e;
errorMessage.append("CryptoRandom: [" + className + "] failed
with " + e.getMessage());
- } catch (final ExceptionInInitializerError initializerError) {
- Throwable t = initializerError.getCause();
- if (t instanceof Exception) {
- lastException = (Exception) t;
- errorMessage.append("CryptoRandom: [" + className + "]
initialization failed with " + t.getMessage());
- } else {
- throw initializerError;
- }
}
}
diff --git a/src/main/java/org/apache/commons/crypto/utils/ReflectionUtils.java
b/src/main/java/org/apache/commons/crypto/utils/ReflectionUtils.java
index f5fa8ca..e941dee 100644
--- a/src/main/java/org/apache/commons/crypto/utils/ReflectionUtils.java
+++ b/src/main/java/org/apache/commons/crypto/utils/ReflectionUtils.java
@@ -21,8 +21,12 @@ import java.lang.ref.WeakReference;
import java.lang.reflect.Constructor;
import java.util.Arrays;
import java.util.Collections;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
import org.apache.commons.crypto.cipher.CryptoCipher;
@@ -40,6 +44,7 @@ public final class ReflectionUtils {
}
private static final Map<ClassLoader, Map<String,
WeakReference<Class<?>>>> CACHE_CLASSES = new WeakHashMap<>();
+ private static final ConcurrentMap<ClassLoader, Set<String>>
INIT_ERROR_CLASSES = new ConcurrentHashMap<>();
private static final ClassLoader CLASSLOADER;
@@ -63,6 +68,9 @@ public final class ReflectionUtils {
public static Class<?> getClassByName(final String name) throws
ClassNotFoundException {
final Class<?> ret = getClassByNameOrNull(name);
if (ret == null) {
+ if (INIT_ERROR_CLASSES.get(CLASSLOADER).contains(name)) {
+ throw new IllegalStateException("Class " + name + "
initialization error");
+ }
throw new ClassNotFoundException("Class " + name + " not found");
}
return ret;
@@ -73,9 +81,16 @@ public final class ReflectionUtils {
* couldn't be loaded. This is to avoid the overhead of creating an
exception.
*
* @param name the class name.
- * @return the class object, or {@code null} if it could not be found.
+ * @return the class object, or {@code null} if it could not be found or
initialization failed.
*/
private static Class<?> getClassByNameOrNull(final String name) {
+ final Set<String> set =
+ INIT_ERROR_CLASSES.computeIfAbsent(CLASSLOADER, k ->
Collections.synchronizedSet(new HashSet<>()));
+
+ if (set.contains(name)) {
+ return null;
+ }
+
final Map<String, WeakReference<Class<?>>> map;
synchronized (CACHE_CLASSES) {
@@ -95,6 +110,10 @@ public final class ReflectionUtils {
// Leave a marker that the class isn't found
map.put(name, new WeakReference<>(NEGATIVE_CACHE_SENTINEL));
return null;
+ } catch (final ExceptionInInitializerError error) {
+ // Leave a marker that the class initialization failed
+ set.add(name);
+ return null;
}
// two putters can race here, but they'll put the same class
map.put(name, new WeakReference<>(clazz));
diff --git
a/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java
b/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java
index 4e20502..1119483 100644
---
a/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java
+++
b/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java
@@ -82,8 +82,11 @@ public class CryptoRandomFactoryTest {
String classes =
ExceptionInInitializerErrorRandom.class.getName().concat(",")
.concat(CryptoRandomFactory.RandomProvider.JAVA.getClassName());
properties.setProperty(CryptoRandomFactory.CLASSES_KEY, classes);
- try (final CryptoRandom random =
CryptoRandomFactory.getCryptoRandom(properties)) {
- assertEquals(JavaCryptoRandom.class.getName(),
random.getClass().getName());
+ // Invoke 3 times to test the reentrancy of the method in the scenario
of class initialization failure.
+ for (int i = 0; i < 3; i++) {
+ try (final CryptoRandom random =
CryptoRandomFactory.getCryptoRandom(properties)) {
+ assertEquals(JavaCryptoRandom.class.getName(),
random.getClass().getName());
+ }
}
}
