Skip to content

Commit ee9f196

Browse files
Azqueltmanovotn
authored andcommitted
Improve Invoker exception messages
If an invoker invocation causes a ClassCastException, NullPointerException or IllegalArgumentexception, validate the instance and arguments to see if the exception was caused by passing invalid values. If so, discard the original exception and throw a new exception with more information. Doing these checks only when the invocation throws an exception avoids the overhead of doing these checks before every successful invocation.
1 parent b6b5011 commit ee9f196

File tree

7 files changed

+560
-0
lines changed

7 files changed

+560
-0
lines changed

impl/src/main/java/org/jboss/weld/invokable/AbstractInvokerBuilder.java

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,19 @@ private boolean requiresCleanup() {
136136
// single, array-typed parameter at the end for variable arity methods
137137
mh = mh.asFixedArity();
138138

139+
// Check instance is not null
140+
if (!instanceLookup && !isStaticMethod) {
141+
Class<?> instanceType = mh.type().parameterType(0);
142+
Class<?> returnType = mh.type().returnType();
143+
MethodHandle checkInstanceNotNull = MethodHandles.insertArguments(MethodHandleUtils.CHECK_INSTANCE_NOT_NULL, 0,
144+
reflectionMethod);
145+
checkInstanceNotNull = checkInstanceNotNull
146+
.asType(checkInstanceNotNull.type().changeParameterType(0, instanceType));
147+
MethodHandle npeCatch = MethodHandles.throwException(returnType, NullPointerException.class);
148+
npeCatch = MethodHandles.collectArguments(npeCatch, 1, checkInstanceNotNull);
149+
mh = MethodHandles.catchException(mh, NullPointerException.class, npeCatch);
150+
}
151+
139152
// instance transformer
140153
if (instanceTransformer != null && !isStaticMethod) {
141154
MethodHandle instanceTransformerMethod = MethodHandleUtils.createMethodHandleFromTransformer(reflectionMethod,
@@ -155,8 +168,10 @@ private boolean requiresCleanup() {
155168

156169
// argument transformers
157170
// backwards iteration for correct construction of the resulting parameter list
171+
Class<?>[] transformerArgTypes = new Class<?>[argTransformers.length];
158172
for (int i = argTransformers.length - 1; i >= 0; i--) {
159173
if (argTransformers[i] == null) {
174+
transformerArgTypes[i] = reflectionMethod.getParameterTypes()[i];
160175
continue;
161176
}
162177
int position = instanceArguments + i;
@@ -172,6 +187,7 @@ private boolean requiresCleanup() {
172187
// internal error, this should not pass validation
173188
throw InvokerLogger.LOG.invalidTransformerMethod("argument", argTransformers[i]);
174189
}
190+
transformerArgTypes[i] = argTransformerMethod.type().parameterType(0);
175191
}
176192

177193
// return type transformer
@@ -323,6 +339,61 @@ private boolean requiresCleanup() {
323339
mh = MethodHandles.foldArguments(mh, MethodHandleUtils.CLEANUP_ACTIONS_CTOR);
324340
}
325341

342+
Class<?>[] expectedTypes = new Class<?>[transformerArgTypes.length];
343+
for (int i = 0; i < transformerArgTypes.length; i++) {
344+
if (argLookup[i]) {
345+
expectedTypes[i] = null;
346+
} else {
347+
expectedTypes[i] = transformerArgTypes[i];
348+
}
349+
}
350+
351+
if (reflectionMethod.getParameterCount() > 0) {
352+
// Catch NullPointerException and check whether it's caused by any arguments being null:
353+
Class<?> instanceType = mh.type().parameterType(0);
354+
MethodHandle checkArgumentsNotNull = MethodHandles.insertArguments(MethodHandleUtils.CHECK_ARGUMENTS_NOT_NULL, 0,
355+
reflectionMethod, expectedTypes);
356+
checkArgumentsNotNull = MethodHandles.dropArguments(checkArgumentsNotNull, 0, instanceType);
357+
MethodHandle npeCatch = MethodHandles.throwException(mh.type().returnType(), NullPointerException.class);
358+
npeCatch = MethodHandles.collectArguments(npeCatch, 1, checkArgumentsNotNull);
359+
mh = MethodHandles.catchException(mh, NullPointerException.class, npeCatch);
360+
361+
// Catch IllegalArgumentException and check whether it's caused by the args array being too short
362+
MethodHandle checkArgCountAtLeast = MethodHandles.insertArguments(MethodHandleUtils.CHECK_ARG_COUNT_AT_LEAST, 0,
363+
reflectionMethod, reflectionMethod.getParameterCount());
364+
checkArgCountAtLeast = MethodHandles.dropArguments(checkArgCountAtLeast, 0, instanceType);
365+
MethodHandle iaeCatch = MethodHandles.throwException(mh.type().returnType(), IllegalArgumentException.class);
366+
iaeCatch = MethodHandles.collectArguments(iaeCatch, 1, checkArgCountAtLeast);
367+
mh = MethodHandles.catchException(mh, IllegalArgumentException.class, iaeCatch);
368+
}
369+
370+
if ((!isStaticMethod && !instanceLookup) || reflectionMethod.getParameterCount() > 0) {
371+
// Catch ClassCastException and check whether it's caused by either the instance or the arguments being the wrong type
372+
Class<?> instanceType = mh.type().parameterType(0);
373+
MethodHandle checkTypes = null;
374+
if (reflectionMethod.getParameterCount() > 0) {
375+
checkTypes = MethodHandles.insertArguments(MethodHandleUtils.CHECK_ARGUMENTS_HAVE_CORRECT_TYPE, 0,
376+
reflectionMethod, expectedTypes);
377+
checkTypes = MethodHandles.dropArguments(checkTypes, 0, Object.class);
378+
}
379+
380+
if (!isStaticMethod && !instanceLookup) {
381+
MethodHandle checkInstanceType = MethodHandles.insertArguments(MethodHandleUtils.CHECK_INSTANCE_HAS_TYPE, 0,
382+
reflectionMethod, instanceType);
383+
if (checkTypes == null) {
384+
checkTypes = checkInstanceType;
385+
} else {
386+
checkTypes = MethodHandles.foldArguments(checkTypes, checkInstanceType);
387+
}
388+
}
389+
390+
MethodHandle cceCatch = MethodHandles.throwException(mh.type().returnType(), ClassCastException.class);
391+
cceCatch = MethodHandles.collectArguments(cceCatch, 1, checkTypes);
392+
393+
mh = mh.asType(mh.type().changeParameterType(0, Object.class)); // Defer the casting the instance to its expected type until we're inside the ClassCastException try block
394+
mh = MethodHandles.catchException(mh, ClassCastException.class, cceCatch);
395+
}
396+
326397
// create an inner invoker and pass it to wrapper
327398
if (invocationWrapper != null) {
328399
InvokerImpl<?, ?> invoker = new InvokerImpl<>(mh);
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
package org.jboss.weld.invokable;
2+
3+
import java.lang.reflect.Method;
4+
5+
import jakarta.enterprise.invoke.Invoker;
6+
7+
import org.jboss.weld.exceptions.IllegalArgumentException;
8+
import org.jboss.weld.logging.InvokerLogger;
9+
10+
/**
11+
* Utility methods for checking the arguments and instances being used by an {@link Invoker}.
12+
* <p>
13+
* Handles to these methods are obtained via {@link MethodHandleUtils}.
14+
*/
15+
public class InvokerValidationUtils {
16+
17+
private InvokerValidationUtils() {
18+
}
19+
20+
/**
21+
* Validate that an instance being called by an {@link Invoker} has the expected type or is {@code null}.
22+
*
23+
* @param invokerMethod the method being invoked
24+
* @param type the expected type of the instance
25+
* @param instance the instance the method is being invoked on
26+
* @throws ClassCastException if {@code instance} is not {@code null} and not of the expected type
27+
*/
28+
static void instanceHasType(Method invokerMethod, Class<?> type, Object instance) {
29+
if (instance != null && !type.isInstance(instance)) {
30+
throw InvokerLogger.LOG.wrongInstanceType(invokerMethod, instance.getClass(), type);
31+
}
32+
}
33+
34+
/**
35+
* Validate that an instance being called by an {@link Invoker} is not {@code null}.
36+
*
37+
* @param invokerMethod the method being invoked
38+
* @param instance the instance to check
39+
* @throws NullPointerException if {@code instance} is {@code null}
40+
*/
41+
static void instanceNotNull(Method invokerMethod, Object instance) {
42+
if (instance == null) {
43+
throw InvokerLogger.LOG.nullInstance(invokerMethod);
44+
}
45+
}
46+
47+
/**
48+
* Validate that if arguments are required then the arguments array is not {@code null} and any primitive arguments are not
49+
* {@code null}.
50+
*
51+
* @param invokerMethod the method being invoked
52+
* @param expectedTypes the expected type of each argument, a {@code null} entry indicates that the type of that parameter
53+
* should not be checked
54+
* @param args the array of arguments
55+
* @throws NullPointerException if arguments are required and {@code args} is {@code null} or a primitive argument is
56+
* {@code null}
57+
*/
58+
static void argumentsNotNull(Method invokerMethod, Class<?>[] expectedTypes, Object[] args) {
59+
if (invokerMethod.getParameterCount() == 0) {
60+
return; // If there are no arguments expected then there's nothing to check
61+
}
62+
if (args == null) {
63+
throw InvokerLogger.LOG.nullArgumentArray(invokerMethod);
64+
}
65+
for (int i = 0; i < expectedTypes.length; i++) {
66+
Class<?> expectedType = expectedTypes[i];
67+
Object arg = args[i];
68+
if (expectedType != null && arg == null && expectedType.isPrimitive()) {
69+
throw InvokerLogger.LOG.nullPrimitiveArgument(invokerMethod, i + 1);
70+
}
71+
}
72+
}
73+
74+
/**
75+
* Validate that an array of arguments for an {@link Invoker} has at least an expected number of elements
76+
*
77+
* @param invokerMethod the method being invoked
78+
* @param requiredArgs the expected number of arguments
79+
* @param args the array of arguments
80+
* @return {@code args}
81+
* @throws IllegalArgumentException if the length of {@code args} is less than {@code requiredArgs}
82+
*/
83+
static void argCountAtLeast(Method invokerMethod, int requiredArgs, Object[] args) {
84+
int actualArgs = args == null ? 0 : args.length;
85+
if (actualArgs < requiredArgs) {
86+
throw InvokerLogger.LOG.notEnoughArguments(invokerMethod, requiredArgs, actualArgs);
87+
}
88+
}
89+
90+
/**
91+
* Validate that each of the arguments being passed passed to a method by an {@link Invoker} has the correct type.
92+
* <p>
93+
* For each pair if type and argument from {@code expectedTypes} and {@code args}:
94+
* <ul>
95+
* <li>if the expected type is {@code null}, no validation is done
96+
* <li>if the expected type is a primitive type, check that the argument is not {@code null} and can be converted to that
97+
* primitive type using boxing and primitive widening conversions
98+
* <li>otherwise, check that the argument is an instance of the expected type
99+
* </ul>
100+
*
101+
* @param invokerMethod the method being invoked
102+
* @param expectedTypes an array of the expected type of each argument. May contain {@code null} to indicate that that
103+
* argument should not be validated.
104+
* @param args the array of values being passed as arguments
105+
* @throws ClassCastException if any of the arguments are not valid for their expected type
106+
* @throws NullPointerException if an argument for a primitive-typed parameter is not null
107+
*/
108+
static void argumentsHaveCorrectType(Method invokerMethod, Class<?>[] expectedTypes, Object[] args) {
109+
if (args == null) {
110+
// Not expected since we're in the catch block of ClassCastException, but guarantees we can safely access args
111+
throw InvokerLogger.LOG.nullArgumentArray(invokerMethod);
112+
}
113+
for (int i = 0; i < expectedTypes.length; i++) {
114+
Class<?> expectedType = expectedTypes[i];
115+
Object arg = args[i];
116+
if (expectedType != null) {
117+
int pos = i + 1; // 1-indexed argument position
118+
if (expectedType.isPrimitive()) {
119+
if (arg == null) {
120+
// Not expected since we're in the catch block of ClassCastException, but guarantees we can safely call methods on arg
121+
throw InvokerLogger.LOG.nullPrimitiveArgument(invokerMethod, pos);
122+
}
123+
if (!primitiveConversionPermitted(expectedType, arg.getClass())) {
124+
throw InvokerLogger.LOG.wrongArgumentType(invokerMethod, pos, arg.getClass(), expectedType);
125+
}
126+
} else {
127+
if (arg != null && !expectedType.isInstance(arg)) {
128+
throw InvokerLogger.LOG.wrongArgumentType(invokerMethod, pos, arg.getClass(), expectedType);
129+
}
130+
}
131+
}
132+
}
133+
}
134+
135+
/**
136+
* Validate whether a reference type can be converted to a primitive type via an unboxing and primitive widening conversion.
137+
*
138+
* @param primitive the target primitive type
139+
* @param actual the reference type to test
140+
* @return {@code true} if {@code actual} can be converted to {@code primitive} via an unboxing and primitive widening
141+
* conversion, otherwise {@code false}
142+
*/
143+
private static boolean primitiveConversionPermitted(Class<?> primitive, Class<? extends Object> actual) {
144+
if (primitive == Integer.TYPE) {
145+
return actual == Integer.class
146+
|| actual == Character.class
147+
|| actual == Short.class
148+
|| actual == Byte.class;
149+
} else if (primitive == Long.TYPE) {
150+
return actual == Long.class
151+
|| actual == Integer.class
152+
|| actual == Character.class
153+
|| actual == Short.class
154+
|| actual == Byte.class;
155+
} else if (primitive == Boolean.TYPE) {
156+
return actual == Boolean.class;
157+
} else if (primitive == Double.TYPE) {
158+
return actual == Double.class
159+
|| actual == Float.class
160+
|| actual == Long.class
161+
|| actual == Integer.class
162+
|| actual == Character.class
163+
|| actual == Short.class
164+
|| actual == Byte.class;
165+
} else if (primitive == Float.TYPE) {
166+
return actual == Float.class
167+
|| actual == Long.class
168+
|| actual == Integer.class
169+
|| actual == Character.class
170+
|| actual == Short.class
171+
|| actual == Byte.class;
172+
} else if (primitive == Short.TYPE) {
173+
return actual == Short.class
174+
|| actual == Byte.class;
175+
} else if (primitive == Character.TYPE) {
176+
return actual == Character.class;
177+
} else if (primitive == Byte.TYPE) {
178+
return actual == Byte.class;
179+
}
180+
throw new RuntimeException("Unhandled primitive type: " + primitive);
181+
}
182+
}

impl/src/main/java/org/jboss/weld/invokable/MethodHandleUtils.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ private MethodHandleUtils() {
2828
static final MethodHandle REPLACE_PRIMITIVE_LOOKUP_NULLS;
2929
static final MethodHandle THROW_VALUE_CARRYING_EXCEPTION;
3030
static final MethodHandle TRIM_ARRAY_TO_SIZE;
31+
static final MethodHandle CHECK_INSTANCE_HAS_TYPE;
32+
static final MethodHandle CHECK_INSTANCE_NOT_NULL;
33+
static final MethodHandle CHECK_ARG_COUNT_AT_LEAST;
34+
static final MethodHandle CHECK_ARGUMENTS_HAVE_CORRECT_TYPE;
35+
static final MethodHandle CHECK_ARGUMENTS_NOT_NULL;
3136

3237
static {
3338
try {
@@ -45,6 +50,16 @@ private MethodHandleUtils() {
4550
"throwReturnValue", Object.class));
4651
TRIM_ARRAY_TO_SIZE = createMethodHandle(ArrayUtils.class.getDeclaredMethod(
4752
"trimArrayToSize", Object[].class, int.class));
53+
CHECK_INSTANCE_HAS_TYPE = createMethodHandle(
54+
InvokerValidationUtils.class.getDeclaredMethod("instanceHasType", Method.class, Class.class, Object.class));
55+
CHECK_INSTANCE_NOT_NULL = createMethodHandle(
56+
InvokerValidationUtils.class.getDeclaredMethod("instanceNotNull", Method.class, Object.class));
57+
CHECK_ARG_COUNT_AT_LEAST = createMethodHandle(
58+
InvokerValidationUtils.class.getDeclaredMethod("argCountAtLeast", Method.class, int.class, Object[].class));
59+
CHECK_ARGUMENTS_HAVE_CORRECT_TYPE = createMethodHandle(InvokerValidationUtils.class
60+
.getDeclaredMethod("argumentsHaveCorrectType", Method.class, Class[].class, Object[].class));
61+
CHECK_ARGUMENTS_NOT_NULL = createMethodHandle(InvokerValidationUtils.class.getDeclaredMethod("argumentsNotNull",
62+
Method.class, Class[].class, Object[].class));
4863
} catch (NoSuchMethodException e) {
4964
// should never happen
5065
throw new IllegalStateException("Unable to locate Weld internal helper method", e);

impl/src/main/java/org/jboss/weld/logging/InvokerLogger.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import org.jboss.logging.Logger;
66
import org.jboss.logging.annotations.Cause;
77
import org.jboss.logging.annotations.Message;
8+
import org.jboss.logging.annotations.Message.Format;
89
import org.jboss.logging.annotations.MessageLogger;
910
import org.jboss.weld.exceptions.DeploymentException;
1011
import org.jboss.weld.exceptions.IllegalArgumentException;
@@ -65,4 +66,22 @@ public interface InvokerLogger extends WeldLogger {
6566

6667
@Message(id = 2014, value = "Invocation wrapper has unexpected parameters: {0} \nExpected param types are: {1}, Object[], Invoker.class", format = Message.Format.MESSAGE_FORMAT)
6768
DeploymentException wrapperUnexpectedParams(Object transformerMetadata, Object clazz);
69+
70+
@Message(id = 2015, value = "Cannot invoke {0} because the instance passed to the Invoker was null", format = Format.MESSAGE_FORMAT)
71+
NullPointerException nullInstance(Object method);
72+
73+
@Message(id = 2016, value = "Cannot invoke {0} because the instance passed to the Invoker has type {1} which cannot be cast to {2}", format = Format.MESSAGE_FORMAT)
74+
ClassCastException wrongInstanceType(Object method, Class<?> actualType, Class<?> expectedType);
75+
76+
@Message(id = 2017, value = "Cannot invoke {0} because {1} arguments were expected but only {2} were provided", format = Format.MESSAGE_FORMAT)
77+
IllegalArgumentException notEnoughArguments(Object method, int expectedCount, int actualCount);
78+
79+
@Message(id = 2018, value = "Cannot invoke {0} because argument {1} has type {2} which cannot be cast to {3}", format = Format.MESSAGE_FORMAT)
80+
ClassCastException wrongArgumentType(Object method, int pos, Class<?> actualType, Class<?> expectedType);
81+
82+
@Message(id = 2019, value = "Cannot invoke {0} because parameter {1} is a primitive type but the argument is null", format = Format.MESSAGE_FORMAT)
83+
NullPointerException nullPrimitiveArgument(Object method, int pos);
84+
85+
@Message(id = 2020, value = "Cannot invoke {0} because the args parameter is null and arguments are required", format = Format.MESSAGE_FORMAT)
86+
NullPointerException nullArgumentArray(Object method);
6887
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package org.jboss.weld.tests.invokable.exceptions;
2+
3+
import jakarta.enterprise.context.Dependent;
4+
5+
@Dependent
6+
public class ExceptionTestBean {
7+
8+
public String ping(String s, int i) {
9+
return s + i;
10+
}
11+
12+
public static String staticPing(String s, int i) {
13+
return s + i;
14+
}
15+
16+
public void voidPing(String s, int i) {
17+
String result = s + i;
18+
}
19+
20+
public String noargPing() {
21+
return "42";
22+
}
23+
}

0 commit comments

Comments
 (0)