This is an automated email from the ASF dual-hosted git repository.
jungm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomee.git
The following commit(s) were added to refs/heads/main by this push:
new ec2cdf0960 TOMEE-4708 - fix race condition in shared EL processor
(#2946)
ec2cdf0960 is described below
commit ec2cdf096011ee72caaa6f0cd029bbdd144f73bf
Author: Markus Jung <[email protected]>
AuthorDate: Wed Sep 23 20:19:57 2026 +0200
TOMEE-4708 - fix race condition in shared EL processor (#2946)
---
.../tomee/security/TomEEELInvocationHandler.java | 28 ++++++--
.../tomee/security/cdi/TomEESecurityExtension.java | 3 +-
...dAuthenticationMechanismDefinitionDelegate.java | 2 +-
.../security/TomEEELInvocationHandlerTest.java | 75 ++++++++++++++++++++++
4 files changed, 98 insertions(+), 10 deletions(-)
diff --git
a/tomee/tomee-security/src/main/java/org/apache/tomee/security/TomEEELInvocationHandler.java
b/tomee/tomee-security/src/main/java/org/apache/tomee/security/TomEEELInvocationHandler.java
index 31fb01de5c..048856702c 100644
---
a/tomee/tomee-security/src/main/java/org/apache/tomee/security/TomEEELInvocationHandler.java
+++
b/tomee/tomee-security/src/main/java/org/apache/tomee/security/TomEEELInvocationHandler.java
@@ -26,6 +26,7 @@ import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.HashSet;
import java.util.Set;
+import java.util.function.Supplier;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -34,15 +35,22 @@ public class TomEEELInvocationHandler implements
InvocationHandler {
private static final Pattern EL_EXPRESSION_PATTERN =
Pattern.compile("[#$]\\{([^{}]+)}");
private final Annotation annotation;
- private final ELProcessor processor;
+
+ // ElProcessor isn't threadsafe, so use a Supplier
+ private final Supplier<ELProcessor> processors;
public TomEEELInvocationHandler(final Annotation annotation, final
ELProcessor processor) {
+ this(annotation, () -> processor);
+ }
+
+ private TomEEELInvocationHandler(final Annotation annotation, final
Supplier<ELProcessor> processors) {
this.annotation = annotation;
- this.processor = processor;
+ this.processors = processors;
}
@Override
public Object invoke(final Object proxy, final Method method, final
Object[] args) throws Throwable {
+ final ELProcessor processor = processors.get();
// todo optimize and cache methods
@@ -56,7 +64,7 @@ public class TomEEELInvocationHandler implements
InvocationHandler {
// Nested annotation with possible EL attributes (e.g.
OpenIdAuthenticationMechanismDefinition -> LogoutDefinition)
if (method.getReturnType().isAnnotation())
{
- return of(((Class<Annotation>) method.getReturnType()),
(Annotation) method.invoke(annotation, args), processor);
+ return of(((Class<Annotation>) method.getReturnType()),
(Annotation) method.invoke(annotation, args), processors);
}
// If return value is not a String or an array of string, there is
another method with "Expression" at the end and a return type String
@@ -200,15 +208,21 @@ public class TomEEELInvocationHandler implements
InvocationHandler {
}
public static <T extends Annotation> T of(final Class<T> annotationClass,
final T annotation, final BeanManager beanManager) {
- final ELProcessor elProcessor = new ELProcessor();
- elProcessor.getELManager().addELResolver(beanManager.getELResolver());
- return of(annotationClass, annotation, elProcessor);
+ return of(annotationClass, annotation, () -> {
+ final ELProcessor elProcessor = new ELProcessor();
+
elProcessor.getELManager().addELResolver(beanManager.getELResolver());
+ return elProcessor;
+ });
}
public static <T extends Annotation> T of(final Class<T> annotationClass,
final T annotation, final ELProcessor elProcessor) {
+ return of(annotationClass, annotation, () -> elProcessor);
+ }
+
+ private static <T extends Annotation> T of(final Class<T> annotationClass,
final T annotation, final Supplier<ELProcessor> processors) {
return (T)
Proxy.newProxyInstance(annotation.getClass().getClassLoader(),
new Class[]{annotationClass},
- new
TomEEELInvocationHandler(annotation, elProcessor));
+ new
TomEEELInvocationHandler(annotation, processors));
}
}
diff --git
a/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/TomEESecurityExtension.java
b/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/TomEESecurityExtension.java
index ab63f96e90..fca2f660b8 100644
---
a/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/TomEESecurityExtension.java
+++
b/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/TomEESecurityExtension.java
@@ -16,7 +16,6 @@
*/
package org.apache.tomee.security.cdi;
-import jakarta.enterprise.context.RequestScoped;
import jakarta.enterprise.inject.spi.Bean;
import
jakarta.security.enterprise.authentication.mechanism.http.OpenIdAuthenticationMechanismDefinition;
import org.apache.tomee.security.TomEEELInvocationHandler;
@@ -403,7 +402,7 @@ public class TomEESecurityExtension implements Extension {
.beanClass(OpenIdAuthenticationMechanismDefinition.class)
.types(Object.class,
OpenIdAuthenticationMechanismDefinition.class)
.qualifiers(Default.Literal.INSTANCE, Any.Literal.INSTANCE)
- .scope(RequestScoped.class)
+ .scope(ApplicationScoped.class)
.createWith(creationalContext ->
createOpenIdAuthenticationMechanismDefinition(defaultOidcDefinition,
beanManager));
afterBeanDiscovery.addBean()
diff --git
a/tomee/tomee-security/src/main/java/org/apache/tomee/security/http/openid/OpenIdAuthenticationMechanismDefinitionDelegate.java
b/tomee/tomee-security/src/main/java/org/apache/tomee/security/http/openid/OpenIdAuthenticationMechanismDefinitionDelegate.java
index c4d9bc7203..5fabe7f164 100644
---
a/tomee/tomee-security/src/main/java/org/apache/tomee/security/http/openid/OpenIdAuthenticationMechanismDefinitionDelegate.java
+++
b/tomee/tomee-security/src/main/java/org/apache/tomee/security/http/openid/OpenIdAuthenticationMechanismDefinitionDelegate.java
@@ -210,7 +210,7 @@ public class
OpenIdAuthenticationMechanismDefinitionDelegate implements OpenIdAu
private static final String WELL_KNOWN_CONFIGURATION_PATH =
"/.well-known/openid-configuration";
- private OpenIdProviderMetadata cached = null;
+ private volatile OpenIdProviderMetadata cached = null;
public
AutoResolvingProviderMetadata(OpenIdAuthenticationMechanismDefinition delegate)
{
super(delegate);
diff --git
a/tomee/tomee-security/src/test/java/org/apache/tomee/security/TomEEELInvocationHandlerTest.java
b/tomee/tomee-security/src/test/java/org/apache/tomee/security/TomEEELInvocationHandlerTest.java
index f4948a0819..cc5f3c10eb 100644
---
a/tomee/tomee-security/src/test/java/org/apache/tomee/security/TomEEELInvocationHandlerTest.java
+++
b/tomee/tomee-security/src/test/java/org/apache/tomee/security/TomEEELInvocationHandlerTest.java
@@ -18,6 +18,7 @@ package org.apache.tomee.security;
import jakarta.el.ELProcessor;
import jakarta.el.ELResolver;
+import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.inject.Vetoed;
import jakarta.enterprise.inject.spi.BeanManager;
import jakarta.enterprise.inject.spi.CDI;
@@ -30,8 +31,15 @@ import
jakarta.security.enterprise.identitystore.PasswordHash;
import org.junit.Assert;
import org.junit.Test;
+import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
import static java.util.Arrays.stream;
import static java.util.stream.Collectors.toMap;
@@ -150,6 +158,51 @@ public class TomEEELInvocationHandlerTest extends
AbstractTomEESecurityTest {
Assert.assertThrows(IllegalArgumentException.class, () ->
proxiedAnnotation.clientSecret());
}
+ // See TOMEE-4708
+ @Test
+ public void sharedProxyIsSafeUnderConcurrentEvaluation() throws Exception {
+ final OpenIdAuthenticationMechanismDefinition annotation =
+
ConfigDrivenDefinition.class.getAnnotation(OpenIdAuthenticationMechanismDefinition.class);
+
+ // the BeanManager variant is what TomEESecurityExtension uses for
application-wide definitions
+ final OpenIdAuthenticationMechanismDefinition proxiedAnnotation =
TomEEELInvocationHandler.of(
+ OpenIdAuthenticationMechanismDefinition.class, annotation,
bm());
+
+ final int threads = 16;
+ final int iterations = 2000;
+ final CyclicBarrier barrier = new CyclicBarrier(threads);
+ final List<Throwable> failures = new CopyOnWriteArrayList<>();
+ final ExecutorService executor = Executors.newFixedThreadPool(threads);
+ try {
+ final List<Future<?>> futures = new java.util.ArrayList<>();
+ for (int t = 0; t < threads; t++) {
+ futures.add(executor.submit(() -> {
+ try {
+ barrier.await();
+ for (int i = 0; i < iterations; i++) {
+
Assert.assertFalse(proxiedAnnotation.tokenAutoRefresh());
+ Assert.assertEquals(10000,
proxiedAnnotation.tokenMinValidity());
+ }
+ } catch (final Throwable e) {
+ failures.add(e);
+ }
+ }));
+ }
+ for (final Future<?> future : futures) {
+ future.get(2, TimeUnit.MINUTES);
+ }
+ } finally {
+ executor.shutdownNow();
+ }
+
+ if (!failures.isEmpty()) {
+ final AssertionError error = new AssertionError(failures.size() +
" of " + threads
+ + " threads failed, first: " + failures.get(0));
+ failures.forEach(error::addSuppressed);
+ throw error;
+ }
+ }
+
private BeanManager bm() {
return CDI.current().getBeanManager();
}
@@ -227,4 +280,26 @@ public class TomEEELInvocationHandlerTest extends
AbstractTomEESecurityTest {
}
}
+
+ @OpenIdAuthenticationMechanismDefinition(
+ providerURI = "https://server.example.com";,
+ clientId = "client",
+ tokenAutoRefreshExpression =
"#{elHandlerTestConfig.isTrue('tokenAutoRefresh', false)}",
+ tokenMinValidityExpression =
"#{elHandlerTestConfig.getInt('tokenMinValidity', 10000)}")
+ @Vetoed // keep the extension from registering this as a real OpenID
mechanism
+ public static class ConfigDrivenDefinition {
+ }
+
+ // discovered by CDI so the BeanManager's ELResolver can resolve it, like
an application's config bean
+ @Named("elHandlerTestConfig")
+ @ApplicationScoped
+ public static class Config {
+ public boolean isTrue(final String key, final boolean defaultValue) {
+ return defaultValue;
+ }
+
+ public int getInt(final String key, final int defaultValue) {
+ return defaultValue;
+ }
+ }
}
