Author: markt
Date: Sat Jun 2 21:18:53 2012
New Revision: 1345580
URL: http://svn.apache.org/viewvc?rev=1345580&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53333
Validate JNDI resource types against injection target types and use target
types when no type is specified for the resource.
Based on a patch by Violeta Georgieva.
Modified:
tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties
tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java
Modified: tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties?rev=1345580&r1=1345579&r2=1345580&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties
(original)
+++ tomcat/trunk/java/org/apache/catalina/deploy/LocalStrings.properties Sat
Jun 2 21:18:53 2012
@@ -49,3 +49,4 @@ namingResources.cleanupNoContext=Failed
namingResources.cleanupNoResource=Failed to retrieve JNDI resource [{0}] for
container [{1}] so no cleanup was performed for that resource
namingResources.mbeanCreateFail=Failed to create MBean for naming resource
[{0}]
namingResources.mbeanDestroyFail=Failed to destroy MBean for naming resource
[{0}]
+namingResources.resourceTypeFail=The JNDI resource named [{0}] is of type
[{1}] but the type is inconsistent with the type(s) of the injection target(s)
configured for that resource
\ No newline at end of file
Modified: tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java?rev=1345580&r1=1345579&r2=1345580&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java (original)
+++ tomcat/trunk/java/org/apache/catalina/deploy/NamingResources.java Sat Jun
2 21:18:53 2012
@@ -19,6 +19,7 @@ package org.apache.catalina.deploy;
import java.beans.PropertyChangeListener;
import java.beans.PropertyChangeSupport;
import java.io.Serializable;
+import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.HashMap;
@@ -35,6 +36,7 @@ import org.apache.catalina.LifecycleExce
import org.apache.catalina.LifecycleState;
import org.apache.catalina.Server;
import org.apache.catalina.mbeans.MBeanUtils;
+import org.apache.catalina.util.Introspection;
import org.apache.catalina.util.LifecycleMBeanBase;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
@@ -248,6 +250,12 @@ public class NamingResources extends Lif
}
}
+ if (!checkResourceType(environment)) {
+ throw new IllegalArgumentException(sm.getString(
+ "namingResources.resourceTypeFail", environment.getName(),
+ environment.getType()));
+ }
+
entries.add(environment.getName());
synchronized (envs) {
@@ -314,6 +322,11 @@ public class NamingResources extends Lif
if (entries.contains(mdr.getName())) {
return;
} else {
+ if (!checkResourceType(mdr)) {
+ throw new IllegalArgumentException(sm.getString(
+ "namingResources.resourceTypeFail", mdr.getName(),
+ mdr.getType()));
+ }
entries.add(mdr.getName());
}
@@ -348,6 +361,11 @@ public class NamingResources extends Lif
if (entries.contains(resource.getName())) {
return;
} else {
+ if (!checkResourceType(resource)) {
+ throw new IllegalArgumentException(sm.getString(
+ "namingResources.resourceTypeFail", resource.getName(),
+ resource.getType()));
+ }
entries.add(resource.getName());
}
@@ -379,7 +397,11 @@ public class NamingResources extends Lif
if (entries.contains(resource.getName())) {
return;
} else {
- entries.add(resource.getName());
+ if (!checkResourceType(resource)) {
+ throw new IllegalArgumentException(sm.getString(
+ "namingResources.resourceTypeFail", resource.getName(),
+ resource.getType()));
+ } entries.add(resource.getName());
}
synchronized (resourceEnvRefs) {
@@ -1082,4 +1104,147 @@ public class NamingResources extends Lif
// Server or just unknown
return "type=NamingResources";
}
+
+ /**
+ * Checks that the configuration of the type for the specified resource is
+ * consistent with any injection targets and if the type is not specified,
+ * tries to configure the type based on the injection targets
+ *
+ * @param resource The resource to check
+ *
+ * @return <code>true</code> if the type for the resource is now valid (if
+ * previously <code>null</code> this means it is now set) or
+ * <code>false</code> if the current resource type is inconsistent
+ * with the injection targets and/or cannot be determined
+ */
+ private boolean checkResourceType(ResourceBase resource) {
+ if (!(container instanceof Context)) {
+ // Only Context's will have injection targets
+ return true;
+ }
+
+ if (resource.getInjectionTargets() == null ||
+ resource.getInjectionTargets().size() == 0) {
+ // No injection targets so use the defined type for the resource
+ return true;
+ }
+
+ Context context = (Context) container;
+
+ String typeName = resource.getType();
+ Class<?> typeClass = null;
+ if (typeName != null) {
+ typeClass = Introspection.loadClass(context, typeName);
+ if (typeClass == null) {
+ // Can't load the type - will trigger a failure later so don't
+ // fail here
+ return true;
+ }
+ }
+
+ Class<?> injectionClass = getInjectionTargetType(context, resource);
+ if (injectionClass == null) {
+ // Indicates that a compatible type could not be identified that
+ // worked for all injection targets
+ return false;
+ }
+
+ if (typeClass == null) {
+ // Only injectionTarget defined - use it
+ resource.setType(injectionClass.getCanonicalName());
+ return true;
+ } else {
+ return injectionClass.isAssignableFrom(typeClass);
+ }
+ }
+
+ private Class<?> getInjectionTargetType(Context context,
+ ResourceBase resource) {
+
+ Class<?> result = null;
+
+ for (InjectionTarget injectionTarget : resource.getInjectionTargets())
{
+ Class<?> clazz = Introspection.loadClass(
+ context, injectionTarget.getTargetClass());
+ if (clazz == null) {
+ // Can't load class - therefore ignore this target
+ continue;
+ }
+
+ // Look for a match
+ String targetName = injectionTarget.getTargetName();
+ // Look for a setter match first
+ Class<?> targetType = getSetterType(clazz, targetName);
+ if (targetType == null) {
+ // Try a field match if no setter match
+ targetType = getFieldType(clazz,targetName);
+ }
+ if (targetType == null) {
+ // No match - ignore this injection target
+ continue;
+ }
+ targetType = convertPrimitiveType(targetType);
+
+ // Figure out the common type - if there is one
+ if (result == null) {
+ result = targetType;
+ } else if (targetType.isAssignableFrom(result)) {
+ // NO-OP - This will work
+ } else if (result.isAssignableFrom(targetType)) {
+ // Need to use more specific type
+ result = targetType;
+ } else {
+ // Incompatible types
+ return null;
+ }
+ }
+ return result;
+ }
+
+ private Class<?> getSetterType(Class<?> clazz, String name) {
+ Method[] methods = Introspection.getDeclaredMethods(clazz);
+ if (methods != null && methods.length > 0) {
+ for (Method method : methods) {
+ if (Introspection.isValidSetter(method) &&
+ Introspection.getName(method).equals(name)) {
+ return method.getParameterTypes()[0];
+ }
+ }
+ }
+ return null;
+ }
+
+ private Class<?> getFieldType(Class<?> clazz, String name) {
+ Field[] fields = Introspection.getDeclaredFields(clazz);
+ if (fields != null && fields.length > 0) {
+ for (Field field : fields) {
+ if (field.getName().equals(name)) {
+ return field.getType();
+ }
+ }
+ }
+ return null;
+ }
+
+ private Class<?> convertPrimitiveType(Class<?> clazz) {
+ if (clazz.equals(char.class)) {
+ return Character.class;
+ } else if (clazz.equals(int.class)) {
+ return Integer.class;
+ } else if (clazz.equals(boolean.class)) {
+ return Boolean.class;
+ } else if (clazz.equals(double.class)) {
+ return Double.class;
+ } else if (clazz.equals(byte.class)) {
+ return Byte.class;
+ } else if (clazz.equals(short.class)) {
+ return Short.class;
+ } else if (clazz.equals(long.class)) {
+ return Long.class;
+ } else if (clazz.equals(float.class)) {
+ return Float.class;
+ } else {
+ return clazz;
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]