Kin-Man Chung wrote:
Your test for bad beans is much weaker than what it is now. For instance,
you do not catch the case where the bean class is abstract.
Ah, yes, you are correct. That can also be easily checked with an additional line of code, of course.
However, I am sympathetic to your view on this. If I can be convinced that
we have covered all checks that we can do without too much trouble, I'll
fix Jasper.
I don't know of any other issues that I'm overlooking besides the possibility of the constructor failing -- which is, of course, what I'm trying to overlook :-)
getConstructor() will only return public constructors (as per Javadoc and the Java sources), so that's handled already.
I'll add and test the missing line (or possibly 2) shortly.
Actually I was also missing a check to see if the class was public :-(
Here's my new proposed diff:
22a23 > import java.lang.reflect.Constructor; 23a25 > import java.lang.reflect.Modifier; 1224c1226,1231 < bean.newInstance(); --- > int beanModifiers = bean.getModifiers(); > if ( !Modifier.isPublic( beanModifiers ) ) > throw new IllegalAccessException( "Bean class is not public" ); > if ( Modifier.isAbstract( beanModifiers ) || Modifier.isInterface( beanModifiers ) ) > throw new InstantiationException( "Bean class is abstract" ); > Constructor constructor = bean.getConstructor( new Class[] {} );
-- Jess Holle