interceptor fails with IllegalArgumentException when parameters are modified and the target method uses primitive types

sirf commented 3 years ago

Describe the bug

When an @AroundInvoke interceptor is used on a method that accepts one or many arguments with primitive types, and one of the arguments are modified, the application crashes with an IllegalArgumentException.

java.lang.IllegalArgumentException: The parameter type [class java.lang.Integer] does not match the type for the target method [int]
    at io.quarkus.arc.impl.AbstractInvocationContext.validateParameters(AbstractInvocationContext.java:81)
    at io.quarkus.arc.impl.AbstractInvocationContext.setParameters(AbstractInvocationContext.java:62)
    at org.acme.getting.started.MyInterceptor.intercept(MyInterceptor.java:30)
    at org.acme.getting.started.MyInterceptor_Bean.intercept(MyInterceptor_Bean.zig:286)
    at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:41)
    at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:50)
    at io.quarkus.arc.runtime.devconsole.InvocationInterceptor.proceed(InvocationInterceptor.java:57)
    at io.quarkus.arc.runtime.devconsole.InvocationInterceptor.monitor(InvocationInterceptor.java:43)
    at io.quarkus.arc.runtime.devconsole.InvocationInterceptor_Bean.intercept(InvocationInterceptor_Bean.zig:521)
    at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:41)
    at io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:41)
    at io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:32)
    at org.acme.getting.started.ReactiveGreetingService_Subclass.greetings(ReactiveGreetingService_Subclass.zig:264)
    at org.acme.getting.started.ReactiveGreetingService_ClientProxy.greetings(ReactiveGreetingService_ClientProxy.zig:131)
    at org.acme.getting.started.ReactiveGreetingResource.greetings(ReactiveGreetingResource.java:30)
    at org.acme.getting.started.ReactiveGreetingResource_Subclass.greetings$$superaccessor4(ReactiveGreetingResource_Subclass.zig:646)
    at org.acme.getting.started.ReactiveGreetingResource_Subclass$$function$$4.apply(ReactiveGreetingResource_Subclass$$function$$4.zig:43)
    at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:54)
    at io.quarkus.arc.runtime.devconsole.InvocationInterceptor.proceed(InvocationInterceptor.java:57)
    at io.quarkus.arc.runtime.devconsole.InvocationInterceptor.monitor(InvocationInterceptor.java:43)
    at io.quarkus.arc.runtime.devconsole.InvocationInterceptor_Bean.intercept(InvocationInterceptor_Bean.zig:521)
    at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:41)
    at io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:41)
    at io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:32)
    at org.acme.getting.started.ReactiveGreetingResource_Subclass.greetings(ReactiveGreetingResource_Subclass.zig:601)
    at org.acme.getting.started.ReactiveGreetingResource$quarkusrestinvoker$greetings_fc5e8e89f58b8153f92fa0c6d8b7cae3808086b5.invoke(ReactiveGreetingResource$quarkusrestinvoker$greetings_fc5e8e89f58b8153f92fa0c6d8b7cae3808086b5.zig:47)
    at org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:29)
    at org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:7)
    at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:122)
    at org.jboss.resteasy.reactive.server.handlers.RestInitialHandler.beginProcessing(RestInitialHandler.java:47)
    at org.jboss.resteasy.reactive.server.vertx.ResteasyReactiveVertxHandler.handle(ResteasyReactiveVertxHandler.java:17)
    at org.jboss.resteasy.reactive.server.vertx.ResteasyReactiveVertxHandler.handle(ResteasyReactiveVertxHandler.java:7)
    at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1038)
    at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:137)
    at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:132)
    at io.quarkus.vertx.http.runtime.StaticResourcesRecorder.lambda$start$1(StaticResourcesRecorder.java:65)
    at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1038)
    at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:101)
    at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:132)
    at io.vertx.ext.web.handler.impl.StaticHandlerImpl.lambda$sendStatic$1(StaticHandlerImpl.java:206)
    at io.vertx.core.impl.ContextImpl.lambda$null$0(ContextImpl.java:327)
    at io.vertx.core.impl.ContextImpl.executeTask(ContextImpl.java:366)
    at io.vertx.core.impl.EventLoopContext.lambda$executeAsync$0(EventLoopContext.java:38)
    at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:164)
    at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:472)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:500)
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    at java.base/java.lang.Thread.run(Thread.java:832)

Expected behavior

Should not crash.

Actual behavior

Crashes.

To Reproduce

I've attached a simple reproducer. It's just the getting-started-reactive project with the addition of a simple interceptor that attempts to change the first argument to the intercepted method. getting-started-reactive.tar.gz

@Dependent
@Interceptor
@Three
@Priority(Interceptor.Priority.APPLICATION)
public class MyInterceptor {

        @AroundInvoke
        public Object intercept(final InvocationContext context) throws Exception {

            final Object[] params = context.getParameters();
            params[0] = 3;
            context.setParameters(params);
            return context.proceed();
        }
}

The interceptor is applied to this method in ReactiveGreetingService

    @Three
    public Multi<String> greetings(int count, String name) {

and the bug is triggered by GET http://localhost:8080/hello/greeting/10/johan after starting the app in dev mode.

NOTE The application will crash with the exact same error even if the int-argument is left untouched, e.g. if we remove params[0] = 3 and instead try to do params[1] = "johan"

Environment

Output of `java -version`

openjdk version "14.0.2" 2020-07-14 OpenJDK Runtime Environment (build 14.0.2+12-Ubuntu-1) OpenJDK 64-Bit Server VM (build 14.0.2+12-Ubuntu-1, mixed mode, sharing)

Quarkus version or git rev

1.13.0.Final

Build tool (ie. output of `mvnw --version` or `gradlew --version`)

pache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f) Maven home: /home/.../.m2/wrapper/dists/apache-maven-3.6.3-bin/1iopthnavndlasol9gbrbg6bf2/apache-maven-3.6.3 Java version: 14.0.2, vendor: Private Build, runtime: /usr/lib/jvm/java-14-openjdk-amd64 Default locale: en_US, platform encoding: UTF-8 OS name: "linux", version: "5.8.0-45-generic", arch: "amd64", family: "unix"

gsmet commented 3 years ago

/cc @mkouba

sirf commented 3 years ago

FYI I just realized it's not even necessary to modify any parameter in order to trigger the issue. This is enough to cause the app to crash. context.setParameters(context.getParameters());

A workaround is of course to change the signature of the intercepted method to accept Integer instead of int, etc.

mkouba commented 3 years ago

Yes, we need to box the primitive method parameter types during validation because the params are represented as Object[].

@sirf Would you care to send a PR or should I fix it? FYI there's a util method that can be used here: https://github.com/quarkusio/quarkus/blob/main/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/Types.java#L20-L26

sirf commented 3 years ago

@mkouba I'll give it a shot.

quarkusio / quarkus