WELD-1201 CDI-245 Better validation of event object types

impl/src/main/java/org/jboss/weld/bootstrap/WeldBootstrap.java

@@ -467,13 +467,14 @@ public Bootstrap endInitialization() {
             // clear the TypeSafeResolvers, so data that is only used at startup
             // is not kept around using up memory
             deploymentManager.getBeanResolver().clear();
-            deploymentManager.getAccessibleObserverNotifier().clear();
-            deploymentManager.getGlobalObserverNotifier().clear();
+            deploymentManager.getAccessibleLenientObserverNotifier().clear();
+            deploymentManager.getGlobalStrictObserverNotifier().clear();
+            deploymentManager.getGlobalLenientObserverNotifier().clear();
             deploymentManager.getDecoratorResolver().clear();
             for (Entry<BeanDeploymentArchive, BeanDeployment> entry : beanDeployments.entrySet()) {
                 BeanManagerImpl beanManager = entry.getValue().getBeanManager();
                 beanManager.getBeanResolver().clear();
-                beanManager.getAccessibleObserverNotifier().clear();
+                beanManager.getAccessibleLenientObserverNotifier().clear();
                 beanManager.getDecoratorResolver().clear();
                 beanManager.getInterceptorMetadataReader().cleanAfterBoot();
                 // clean up beans

impl/src/main/java/org/jboss/weld/bootstrap/events/AbstractContainerEvent.java

@@ -66,7 +66,7 @@ protected BeanManagerImpl getBeanManager() {
     public void fire() {
         Type eventType = new ParameterizedTypeImpl(getRawType(), getActualTypeArguments(), null);
         try {
-            beanManager.getGlobalObserverNotifier().fireEvent(eventType, this);
+            beanManager.getGlobalLenientObserverNotifier().fireEvent(eventType, this);
         } catch (Exception e) {
             getErrors().add(e);
         }

impl/src/main/java/org/jboss/weld/event/EventImpl.java

@@ -16,6 +16,8 @@
  */
 package org.jboss.weld.event;
 
+import static org.jboss.weld.logging.messages.EventMessage.PROXY_REQUIRED;
+
 import java.io.ObjectInputStream;
 import java.io.ObjectStreamException;
 import java.io.Serializable;
@@ -30,11 +32,8 @@
 import org.jboss.weld.bean.builtin.FacadeInjectionPoint;
 import org.jboss.weld.exceptions.InvalidObjectException;
 import org.jboss.weld.manager.BeanManagerImpl;
-import org.jboss.weld.util.Observers;
 import org.jboss.weld.util.reflection.Formats;
 
-import static org.jboss.weld.logging.messages.EventMessage.PROXY_REQUIRED;
-
 /**
  * Implementation of the Event interface
  *
@@ -66,7 +65,7 @@ public String toString() {
     }
 
     public void fire(T event) {
-        getBeanManager().getGlobalObserverNotifier().fireEvent(getType(), event, getQualifiers());
+        getBeanManager().getGlobalStrictObserverNotifier().fireEvent(getType(), event, getQualifiers());
     }
 
     public Event<T> select(Annotation... qualifiers) {
@@ -82,7 +81,7 @@ public <U extends T> Event<U> select(TypeLiteral<U> subtype, Annotation... quali
     }
 
     private <U extends T> Event<U> selectEvent(Type subtype, Annotation[] newQualifiers) {
-        Observers.checkEventObjectType(getBeanManager(), subtype);
+        getBeanManager().getGlobalStrictObserverNotifier().checkEventObjectType(subtype);
         return new EventImpl<U>(new FacadeInjectionPoint(getInjectionPoint(), subtype, getQualifiers(), newQualifiers), getBeanManager());
     }
 

impl/src/main/java/org/jboss/weld/event/GlobalObserverNotifierService.java

@@ -50,12 +50,14 @@ public Iterator<ObserverMethod<?>> apply(BeanManagerImpl manager) {
     }
 
     private final Set<BeanManagerImpl> beanManagers;
-    private final ObserverNotifier globalObserverNotifier;
+    private final ObserverNotifier globalLenientObserverNotifier;
+    private final ObserverNotifier globalStrictObserverNotifier;
 
     public GlobalObserverNotifierService(ServiceRegistry services) {
         this.beanManagers = new CopyOnWriteArraySet<BeanManagerImpl>();
         TypeSafeObserverResolver resolver = new TypeSafeObserverResolver(services.get(MetaAnnotationStore.class), createGlobalObserverMethodIterable(beanManagers));
-        this.globalObserverNotifier = ObserverNotifier.of(resolver, services);
+        this.globalLenientObserverNotifier = ObserverNotifier.of(resolver, services, false);
+        this.globalStrictObserverNotifier = ObserverNotifier.of(resolver, services, true);
     }
 
     private static Iterable<ObserverMethod<?>> createGlobalObserverMethodIterable(final Set<BeanManagerImpl> beanManagers) {
@@ -72,13 +74,18 @@ public void registerBeanManager(BeanManagerImpl manager) {
         this.beanManagers.add(manager);
     }
 
-    public ObserverNotifier getGlobalObserverNotifier() {
-        return globalObserverNotifier;
+    public ObserverNotifier getGlobalLenientObserverNotifier() {
+        return globalLenientObserverNotifier;
+    }
+
+    public ObserverNotifier getGlobalStrictObserverNotifier() {
+        return globalStrictObserverNotifier;
     }
 
     @Override
     public void cleanup() {
         this.beanManagers.clear();
-        this.globalObserverNotifier.clear();
+        this.globalStrictObserverNotifier.clear();
+        this.globalLenientObserverNotifier.clear();
     }
 }

impl/src/main/java/org/jboss/weld/event/ObserverNotifier.java

@@ -16,10 +16,14 @@
  */
 package org.jboss.weld.event;
 
+import static org.jboss.weld.logging.messages.UtilMessage.EVENT_TYPE_NOT_ALLOWED;
+import static org.jboss.weld.logging.messages.UtilMessage.TYPE_PARAMETER_NOT_ALLOWED_IN_EVENT_TYPE;
 import static org.jboss.weld.util.reflection.Reflections.cast;
 
 import java.lang.annotation.Annotation;
+import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
+import java.lang.reflect.TypeVariable;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
@@ -28,41 +32,54 @@
 import javax.enterprise.inject.spi.ObserverMethod;
 
 import org.jboss.weld.bootstrap.api.ServiceRegistry;
+import org.jboss.weld.exceptions.IllegalArgumentException;
 import org.jboss.weld.literal.AnyLiteral;
 import org.jboss.weld.resolution.Resolvable;
 import org.jboss.weld.resolution.ResolvableBuilder;
 import org.jboss.weld.resolution.TypeSafeObserverResolver;
 import org.jboss.weld.resources.SharedObjectCache;
 import org.jboss.weld.transaction.spi.TransactionServices;
 import org.jboss.weld.util.Observers;
+import org.jboss.weld.util.reflection.Reflections;
 
 /**
  * Provides event-related operations such sa observer method resolution and event delivery.
  *
+ *
+ *
  * @author Jozef Hartinger
  * @author David Allen
  *
  */
 public class ObserverNotifier {
 
-    public static ObserverNotifier of(TypeSafeObserverResolver resolver, ServiceRegistry services) {
+    /**
+     *
+     * @param resolver
+     * @param services
+     * @param strict indicates whether event type should be performed or not
+     * @return ObserverNotifier instance
+     */
+    public static ObserverNotifier of(TypeSafeObserverResolver resolver, ServiceRegistry services, boolean strict) {
         if (services.contains(TransactionServices.class)) {
-            return new TransactionalObserverNotifier(resolver, services);
+            return new TransactionalObserverNotifier(resolver, services, strict);
         } else {
-            return new ObserverNotifier(resolver, services);
+            return new ObserverNotifier(resolver, services, strict);
         }
     }
 
     private final TypeSafeObserverResolver resolver;
     private final SharedObjectCache sharedObjectCache;
+    private final boolean strict;
 
-    protected ObserverNotifier(TypeSafeObserverResolver resolver, ServiceRegistry services) {
+    protected ObserverNotifier(TypeSafeObserverResolver resolver, ServiceRegistry services, boolean strict) {
         this.resolver = resolver;
         this.sharedObjectCache = services.get(SharedObjectCache.class);
+        this.strict = strict;
     }
 
     public <T> Set<ObserverMethod<? super T>> resolveObserverMethods(T event, Annotation... bindings) {
-        Observers.checkEventObjectType(sharedObjectCache, event);
+        checkEventObjectType(event);
         return this.<T>resolveObserverMethods(event.getClass(), bindings);
     }
 
@@ -71,14 +88,14 @@ public void fireEvent(Object event, Annotation... qualifiers) {
     }
 
     public void fireEvent(Type eventType, Object event, Annotation... qualifiers) {
-        Observers.checkEventObjectType(sharedObjectCache, event);
+        checkEventObjectType(event);
         Set<Annotation> qualifierSet = new HashSet<Annotation>(Arrays.asList(qualifiers));
         // we use the array of qualifiers for resolution so that we can catch duplicate qualifiers
         notifyObservers(event, qualifierSet, resolveObserverMethods(eventType, qualifiers));
     }
 
     public void fireEvent(Type eventType, Object event, Set<Annotation> qualifiers) {
-        Observers.checkEventObjectType(sharedObjectCache, event);
+        checkEventObjectType(event);
         notifyObservers(event, qualifiers, resolveObserverMethods(eventType, qualifiers));
     }
 
@@ -128,4 +145,41 @@ public void clear() {
     protected <T> void notifyObserver(final T event, Set<Annotation> qualifiers, final ObserverMethod<? super T> observer) {
         observer.notify(event, qualifiers);
     }
+
+    public void checkEventObjectType(Object event) {
+        checkEventObjectType(event.getClass());
+    }
+
+    public void checkEventObjectType(Type eventType) {
+        if (!strict) {
+            return;
+        }
+        Type[] typeParameters;
+        final Type resolvedType = sharedObjectCache.getResolvedType(eventType);
+        if (resolvedType instanceof Class<?>) {
+            typeParameters = new Type[0];
+        } else if (resolvedType instanceof ParameterizedType) {
+            typeParameters = ((ParameterizedType) resolvedType).getActualTypeArguments();
+        } else {
+            throw new IllegalArgumentException(EVENT_TYPE_NOT_ALLOWED, resolvedType);
+        }
+        /*
+         * If the runtime type of the event object contains a type variable, the container must throw an IllegalArgumentException.
+         */
+        for (Type type : typeParameters) {
+            if (type instanceof TypeVariable<?>) {
+                throw new IllegalArgumentException(TYPE_PARAMETER_NOT_ALLOWED_IN_EVENT_TYPE, resolvedType);
+            }
+        }
+        /*
+         * If the runtime type of the event object is assignable to the type of a container lifecycle event, IllegalArgumentException
+         * is thrown.
+         */
+        Class<?> resolvedClass = Reflections.getRawType(resolvedType);
+        for (Class<?> containerEventType : Observers.CONTAINER_LIFECYCLE_EVENT_CANONICAL_SUPERTYPES) {
+            if (containerEventType.isAssignableFrom(resolvedClass)) {
+                throw new IllegalArgumentException(EVENT_TYPE_NOT_ALLOWED, resolvedType);
+            }
+        }
+    }
 }

impl/src/main/java/org/jboss/weld/event/TransactionalObserverNotifier.java

@@ -36,8 +36,8 @@ public class TransactionalObserverNotifier extends ObserverNotifier {
 
     private final TransactionServices transactionServices;
 
-    protected TransactionalObserverNotifier(TypeSafeObserverResolver resolver, ServiceRegistry services) {
-        super(resolver, services);
+    protected TransactionalObserverNotifier(TypeSafeObserverResolver resolver, ServiceRegistry services, boolean strict) {
+        super(resolver, services, strict);
         this.transactionServices = services.get(TransactionServices.class);
     }
 

impl/src/main/java/org/jboss/weld/manager/BeanManagerImpl.java

@@ -240,8 +240,13 @@ public class BeanManagerImpl implements WeldManager, Serializable {
     private final transient ELResolver weldELResolver;
     private transient Namespace rootNamespace;
 
-    private final transient ObserverNotifier accessibleObserverNotifier;
-    private final transient ObserverNotifier globalObserverNotifier;
+    /*
+     * Lenient instances do not perform event type checking - this is required for firing container lifecycle events.
+     * Strict instances do performe event type checking and are used for firing application an extension events.
+     */
+    private final transient ObserverNotifier accessibleLenientObserverNotifier;
+    private final transient ObserverNotifier globalLenientObserverNotifier;
+    private final transient ObserverNotifier globalStrictObserverNotifier;
 
     /*
     * Activity scoped data structures
@@ -408,9 +413,10 @@ private BeanManagerImpl(
         this.childActivities = new CopyOnWriteArraySet<BeanManagerImpl>();
 
         TypeSafeObserverResolver accessibleObserverResolver = new TypeSafeObserverResolver(getServices().get(MetaAnnotationStore.class), createDynamicAccessibleIterable(ObserverMethodTransform.INSTANCE));
-        this.accessibleObserverNotifier = ObserverNotifier.of(accessibleObserverResolver, getServices());
+        this.accessibleLenientObserverNotifier = ObserverNotifier.of(accessibleObserverResolver, getServices(), false);
         GlobalObserverNotifierService globalObserverNotifierService = services.get(GlobalObserverNotifierService.class);
-        this.globalObserverNotifier = globalObserverNotifierService.getGlobalObserverNotifier();
+        this.globalLenientObserverNotifier = globalObserverNotifierService.getGlobalLenientObserverNotifier();
+        this.globalStrictObserverNotifier = globalObserverNotifierService.getGlobalStrictObserverNotifier();
         globalObserverNotifierService.registerBeanManager(this);
     }
 
@@ -431,7 +437,7 @@ public void addAccessibleBeanManager(BeanManagerImpl accessibleBeanManager) {
         beanResolver.clear();
         interceptorResolver.clear();
         decoratorResolver.clear();
-        accessibleObserverNotifier.clear();
+        accessibleLenientObserverNotifier.clear();
     }
 
     public HashSet<BeanManagerImpl> getAccessibleManagers() {
@@ -487,7 +493,7 @@ public void addDecorator(Decorator<?> bean) {
 
     @Override
     public <T> Set<ObserverMethod<? super T>> resolveObserverMethods(T event, Annotation... bindings) {
-        return globalObserverNotifier.resolveObserverMethods(event, bindings);
+        return globalStrictObserverNotifier.resolveObserverMethods(event, bindings);
     }
 
     public void addInterceptor(Interceptor<?> bean) {
@@ -622,7 +628,7 @@ public void addObserver(ObserverMethod<?> observer) {
      */
     @Override
     public void fireEvent(Object event, Annotation... qualifiers) {
-        globalObserverNotifier.fireEvent(event, qualifiers);
+        globalStrictObserverNotifier.fireEvent(event, qualifiers);
     }
 
     /**
@@ -879,21 +885,30 @@ public TypeSafeResolver<Resolvable, Decorator<?>> getDecoratorResolver() {
     }
 
     /**
-     * Get the observer notifier for accessible observer methods. For internal use
+     * Get the lenient observer notifier for accessible observer methods. Should never be exposed to an application.
+     *
+     * @return The {@link ObserverNotifier}
+     */
+    public ObserverNotifier getAccessibleLenientObserverNotifier() {
+        return accessibleLenientObserverNotifier;
+    }
+
+    /**
+     * Get the lenient global observer notifier. Should never be exposed to an application.
      *
      * @return The {@link ObserverNotifier}
      */
-    public ObserverNotifier getAccessibleObserverNotifier() {
-        return accessibleObserverNotifier;
+    public ObserverNotifier getGlobalLenientObserverNotifier() {
+        return globalLenientObserverNotifier;
     }
 
     /**
-     * Get the global observer notifier. For internal use
+     * Get the Strict global observer notifier. This one should be used for firing application / extension events.
      *
      * @return The {@link ObserverNotifier}
      */
-    public ObserverNotifier getGlobalObserverNotifier() {
-        return globalObserverNotifier;
+    public ObserverNotifier getGlobalStrictObserverNotifier() {
+        return globalStrictObserverNotifier;
     }
 
     /**
@@ -1271,7 +1286,7 @@ public void cleanup() {
         this.interceptors.clear();
         this.nameBasedResolver.clear();
         this.namespaces.clear();
-        this.accessibleObserverNotifier.clear();
+        this.accessibleLenientObserverNotifier.clear();
         this.observers.clear();
     }