Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interceptor doesn't allow subclasses to be passed as parameter to intercepted method #16274

Closed
sirf opened this issue Apr 6, 2021 · 7 comments · Fixed by #16281
Closed

Interceptor doesn't allow subclasses to be passed as parameter to intercepted method #16274

sirf opened this issue Apr 6, 2021 · 7 comments · Fixed by #16281
Labels
area/arc Issue related to ARC (dependency injection) kind/bug Something isn't working
Milestone

Comments

@sirf
Copy link
Contributor

sirf commented Apr 6, 2021

Describe the bug

When an @AroundInvoke interceptor is used on a method that accepts one or many parameters,
and a parameter is assigned a subtype of the formal argument type, the application crashes with an IllegalArgumentException.

java.lang.IllegalArgumentException: The parameter type [class java.lang.String] does not match the type for the target method [interface java.lang.CharSequence]
	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.greeting(ReactiveGreetingService_Subclass.zig:379)
	at org.acme.getting.started.ReactiveGreetingService_ClientProxy.greeting(ReactiveGreetingService_ClientProxy.zig:162)
	at org.acme.getting.started.ReactiveGreetingResource.greeting(ReactiveGreetingResource.java:23)
	at org.acme.getting.started.ReactiveGreetingResource_Subclass.greeting$$superaccessor1(ReactiveGreetingResource_Subclass.zig:293)
	at org.acme.getting.started.ReactiveGreetingResource_Subclass$$function$$1.apply(ReactiveGreetingResource_Subclass$$function$$1.zig:33)
	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.greeting(ReactiveGreetingResource_Subclass.zig:250)
	at org.acme.getting.started.ReactiveGreetingResource$quarkusrestinvoker$greeting_46fb0bd11bb900dc0af3fb18a321c7b990ac6540.invoke(ReactiveGreetingResource$quarkusrestinvoker$greeting_46fb0bd11bb900dc0af3fb18a321c7b990ac6540.zig:39)
	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] = "foo";
			context.setParameters(params);
			return context.proceed();
		}
}

The interceptor is applied to this method in ReactiveGreetingService that is modified to accept a CharSequence instad of a String

    @Three
    public Uni<String> greetings(CharSequence name) {

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

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"

@sirf sirf added the kind/bug Something isn't working label Apr 6, 2021
@gsmet gsmet added area/arc Issue related to ARC (dependency injection) and removed triage/needs-triage labels Apr 6, 2021
@gsmet
Copy link
Member

gsmet commented Apr 6, 2021

/cc @mkouba

@mkouba
Copy link
Contributor

mkouba commented Apr 6, 2021

Hm, this one is interesting. The interceptors spec states "The parameter types must match the types for the target class
method or constructor..."
and the ArC implementation is a bit strict. I wonder if it's safe to allow subtypes as well... and it probably is. WDYT? @manovotn @Ladicek

@sirf
Copy link
Contributor Author

sirf commented Apr 6, 2021

Thorntail doesn't require an exact class match, that's why I figured Quarkus shouldn't either, but I'll admin that I have not actually read the spec 😇

@geoand
Copy link
Contributor

geoand commented Apr 6, 2021

Seems like a useful addition

@manovotn
Copy link
Contributor

manovotn commented Apr 6, 2021

Yeah, it ought to be safe to allow subtypes, so +1.
TCKs seem to have some assertions on this, but none should be detrimental; especially since you say Thorntail (which means Weld) allowed this as well.

@mkouba
Copy link
Contributor

mkouba commented Apr 7, 2021

@Ladicek
Copy link
Contributor

Ladicek commented Apr 7, 2021

Allowing subtypes should indeed be safe. Agree to add this.

(And the Weld implementation is funny :-) )

@quarkus-bot quarkus-bot bot added this to the 1.14 - main milestone Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants