AstProperty: Missing Exception by Evaluation of a Property of a Null Base?

Flowable Engine
1

Dear Flowable experts,

org.flowable.common.engine.impl.de.odysseus.el.tree.impl.ast.AstProperty#eval

starts with the the following code:

Object base = prefix.eval(bindings, context);
if (base == null) {
	return null;
}
Object property = getProperty(bindings, context);
if (property == null && strict) {
	return null;
}
...

As a result, when a custom expression is resolved (e.g. ${customApi.nonExistingObject.nonExistingProperty}), when nonExistingObject equals null and is a base, evaluation of nonExistingProperty returns null. Expectation would be that there is actually an exception thrown.

We’ve tried to incorporate this logic into a custom ELResolver, but short circuiting to null when base is null results in ELResolvers not coming into play for nonExistingProperty.

Would you see any drawbacks in the following potential contribution from our side for the start of the eval?

Object base = prefix.eval(bindings, context);
Object property = getProperty(bindings, context);
if (base == null) {
	if (property != null){
		throw new PropertyNotFoundException(LocalMessages.get("error.property.property.notfound", property, base));
	}
	return null;
}
if (property == null && strict) {
	return null;
}

Thank you very much in advance!

Best regards,
Vasil Tsimashchuk

2

Further investigation has also shown that there is already a similar logic in the AstMethod class. There null base results in an exception for eval only and not for invoke. In addition, it’s also present in e.g. AstProperty#getValueReference.
As a result, the proposal is rather to throw the following exception throw new PropertyNotFoundException(LocalMessages.get("error.property.base.null", prefix)).

3

When I test ${pojo.nested.someProperty} locally, it returns null if nested is null. But if I do ${pojo.nested.test()}, it also returns null. So I’m not sure where you’re seeing the inconsistency?

But irregardless, we can’t go and change this behavior, it would potentially break many existing process models for many users.

1 Like
4

Thanks Joram!
We are reconsidering on our end.