[jboss-jira] [JBoss JIRA] (WFLY-4405) NPE in AstValue.setValue()
Christer Bernérus (JIRA)
issues at jboss.org
Mon Mar 9 06:44:19 EDT 2015
[ https://issues.jboss.org/browse/WFLY-4405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13047315#comment-13047315 ]
Christer Bernérus commented on WFLY-4405:
-----------------------------------------
Last week i dug deeper into this, so I'm pretty sure about what has triggered this behaviour.
I have found a solution for myself working around this, by creating a BeanInfo class, but my analysis might be give some input for thoughts for correcting some possibly very old code.
In my code I have wrapped the properties in the beans using a generic class that acts like a lazily evaluating cache. So every access to the actual property values goes through the
getValue() and setValue() methods of that class.
The problem is that the return type of getValue() is T or essentially "object", and that setValue() is a multiply overloaded method that takes String, Boolean, Integer, DateTime or whatever.
(I'm aware that this, most likely isn't the kosher way of baking beans, but it has worked fine until now :)
The elResolver.getType(ctx, t.base, property) in the code above call is used by AstValue to determine the type of the property, and a few levels down, via ValueExpressionAnalyzer$InterceptingElResolver and DemuxCompositElResolver, it lands in the method BeanELResolver.getType() which after fetching a BeanProperty regards the EL Context as resolved, and then goes on returning whatever the BeanProperty.getPropertyType() returns.
{code: title=BeanELResolver.java|borderStyle=dashed|borderColor=#ccc|titleBGColor=#F7D6C1|bgColor=#FFFFCE}
/**
* If the base object is not <code>null</code>, returns the most
* general acceptable type that can be set on this bean property.
*
* <p>If the base is not <code>null</code>, the
* <code>propertyResolved</code> property of the <code>ELContext</code>
* object must be set to <code>true</code> by this resolver, before
* returning. If this property is not <code>true</code> after this
* method is called, the caller should ignore the return value.</p>
*
* <p>The provided property will first be coerced to a <code>String</code>.
* If there is a <code>BeanInfoProperty</code> for this property and
* there were no errors retrieving it, the <code>propertyType</code> of
* the <code>propertyDescriptor</code> is returned. Otherwise, a
* <code>PropertyNotFoundException</code> is thrown.</p>
*
* @param context The context of this evaluation.
* @param base The bean to analyze.
* @param property The name of the property to analyze. Will be coerced to
* a <code>String</code>.
* @return If the <code>propertyResolved</code> property of
* <code>ELContext</code> was set to <code>true</code>, then
* the most general acceptable type; otherwise undefined.
* @throws NullPointerException if context is <code>null</code>
* @throws PropertyNotFoundException if <code>base</code> is not
* <code>null</code> and the specified property does not exist
* or is not readable.
* @throws ELException if an exception was thrown while performing
* the property or variable resolution. The thrown exception
* must be included as the cause property of this exception, if
* available.
*/
public Class<?> getType(ELContext context,
Object base,
Object property) {
if (context == null) {
throw new NullPointerException();
}
if (base == null || property == null){
return null;
}
BeanProperty bp = getBeanProperty(context, base, property);
context.setPropertyResolved(true);
return bp.getPropertyType();
}
{code}
Going further down looking at BeanProperty.getPropertyType() it calls the getPropertyType() of its descriptor which in my case goes to PropertyDescriptor.getPropertyType()
{code:title=java.beans.PropertyDescriptor.java|borderStyle=dashed|borderColor=#ccc|titleBGColor=#F7D6C1|bgColor=#FFFFCE}
/**
* Returns the Java type info for the property.
* Note that the {@code Class} object may describe
* primitive Java types such as {@code int}.
* This type is returned by the read method
* or is used as the parameter type of the write method.
* Returns {@code null} if the type is an indexed property
* that does not support non-indexed access.
*
* @return the {@code Class} object that represents the Java type info,
* or {@code null} if the type cannot be determined
*/
public synchronized Class<?> getPropertyType() {
Class<?> type = getPropertyType0();
if (type == null) {
try {
type = findPropertyType(getReadMethod(), getWriteMethod());
setPropertyType(type);
} catch (IntrospectionException ex) {
// Fall
}
}
return type;
}
{code}
In my case, there is no type reference to the property (this is where I assume my BeanInfo solution kicks in), so getPropertyType0() returns null, and the method goes into introspection via the call to findPropertyType(). Here's the code:
{code:title=java.beans.PropertyDescriptor.java|borderStyle=dashed|borderColor=#ccc|titleBGColor=#F7D6C1|bgColor=#FFFFCE}
/**
* Returns the property type that corresponds to the read and write method.
* The type precedence is given to the readMethod.
*
* @return the type of the property descriptor or null if both
* read and write methods are null.
* @throws IntrospectionException if the read or write method is invalid
*/
private Class<?> findPropertyType(Method readMethod, Method writeMethod)
throws IntrospectionException {
Class<?> propertyType = null;
try {
if (readMethod != null) {
Class<?>[] params = getParameterTypes(getClass0(), readMethod);
if (params.length != 0) {
throw new IntrospectionException("bad read method arg count: "
+ readMethod);
}
propertyType = getReturnType(getClass0(), readMethod);
if (propertyType == Void.TYPE) {
throw new IntrospectionException("read method " +
readMethod.getName() + " returns void");
}
}
if (writeMethod != null) {
Class<?>[] params = getParameterTypes(getClass0(), writeMethod);
if (params.length != 1) {
throw new IntrospectionException("bad write method arg count: "
+ writeMethod);
}
if (propertyType != null && !params[0].isAssignableFrom(propertyType)) {
throw new IntrospectionException("type mismatch between read and write methods");
}
propertyType = params[0];
}
} catch (IntrospectionException ex) {
throw ex;
}
return propertyType;
}
{code}
This introspecting code first determines the type of the property by looking at what the getter returns (the readMethod in the codei).
This seems plausible, as one cannot overload methods returning different types in java. Then it looks at the setter method (and it implicitly assumes there is only one setter, but I can cope with that)
Then it checks the parameter count of the setter, which should be 1, fine.
But finally there is a type check between the getter and the setter, which _might_ not be completely accurate:
{code}
if (propertyType != null && !params[0].isAssignableFrom(propertyType)) {
throw new IntrospectionException("type mismatch between read and write methods");
}
{code}
As I interpret this, the code checks if the type returned by the getter, can be used to set the value of the property.
This may actually have been the intention, but in my case, it would have helped if the test had been the other way round, i.e. testing whether the setter is able to set a value of the type returned by the getter.
So the scenario that happened here is that the getter/setter test failed, causing an IntrospectionException which was *swallowed* by the catch in the propertyDescriptor.getPropertyType() method and replaced by a null return.
This null return then bubbles back to {{AstValue.setValue()}} setting {{targetType}} to null and while the context is flagged as resolved, AstValue.setValue() then unresolves the property and the call to convertToType simply returns null and does not change its resolved status, and then 5 lines below the method tries to use {{targetType}} as an object pointer because {{value}} is also null in my case.
So here are three questions to ponder over:
1. Is the getter/setter type comparison correct?
2. Is it really nice to swallow the IntrospectionException in PropertyDescriptor.getPropertyType()
3. What should happen in AstValue.setValue() when targetType is evaluated to null? NPE hurts.
-- Chris
> NPE in AstValue.setValue()
> --------------------------
>
> Key: WFLY-4405
> URL: https://issues.jboss.org/browse/WFLY-4405
> Project: WildFly
> Issue Type: Bug
> Components: JSF
> Affects Versions: 8.2.0.Final
> Environment: MacOS X, RHEL6
> Reporter: Christer Bernérus
> Assignee: Farah Juma
>
> When submitting a page made up using a custom tag, or a composite component, WildFly reports NPE in AstValue.setValue().
> The code where this happens is in javax.el-3.0.1-b05.jar and looks like this:
> {code:title=AstValue.java|borderStyle=solid}
> /* Note by kchung 10/2013
> * The spec does not say if the value should be cocerced to the target
> * type before setting the value to the target. The conversion is kept
> * here to be backward compatible.
> */
> ctx.setPropertyResolved(false);
> Class<?> targetType = elResolver.getType(ctx, t.base, property);
> if (ctx.isPropertyResolved()) {
> ctx.setPropertyResolved(false);
> Object targetValue = elResolver.convertToType(ctx, value, targetType);
> if (ctx.isPropertyResolved()) {
> value = targetValue;
> } else {
> if (value != null || targetType.isPrimitive()) {
> value = ELSupport.coerceToType(value, targetType);
> }
> }
> }
> {code}
> From my debugging session, I have seen that the code in
> {{elResolver.getType()}} may return null, which happens to me,
> but the subsequent code in {{AstValue.setValue()}} happily uses that null valued variable {{targetType}} for calling the method {{isPrimitive()}} which of course causes an NPE.
> Strangely enough, this behaviour seems intermittent in that restarting WildFly might make the bug go away or come back, but redeployment does not.
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
More information about the jboss-jira
mailing list