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

RESTEasy Reactive: NPE due to some exceptions #22408

Closed
FroMage opened this issue Dec 20, 2021 · 24 comments
Closed

RESTEasy Reactive: NPE due to some exceptions #22408

FroMage opened this issue Dec 20, 2021 · 24 comments
Labels
area/rest kind/bug Something isn't working
Milestone

Comments

@FroMage
Copy link
Member

FroMage commented Dec 20, 2021

Describe the bug

RR is generating a javax.ws.rs.NotAllowedException: HTTP 405 Method Not Allowed exception, and for some reason I'm getting an exception mapper of type rest.Application$GeneratedExceptionHandlerFor$RuntimeException$OfMethod$mapIt_Subclass@5fa4a2d5, which returns a null Response, which we then set as context.result.

Then we move on to the exception handler chain:

[org.jboss.resteasy.reactive.server.handlers.ExceptionHandler@246aa690, 
org.jboss.resteasy.reactive.server.handlers.ResourceResponseFilterHandler@2072b0b7, 
org.jboss.resteasy.reactive.server.handlers.ResourceResponseFilterHandler@5da63c4b, 
org.jboss.resteasy.reactive.server.handlers.ResponseHandler@27ee9f20, 
org.jboss.resteasy.reactive.server.handlers.ResponseWriterHandler@5641e36b]

And I'm getting the resource response filters which look at the Response, which is null and it generates an NPE:

Thread [vert.x-eventloop-thread-31] (Suspended (exception NullPointerException))	
	ContainerResponseContextImpl.response() line: 37	
	ContainerResponseContextImpl.getEntity() line: 142	
	TemplateResponseFilter.filter(ResteasyReactiveContainerRequestContext, ContainerResponseContext) line: 22	
	TemplateResponseFilter$GeneratedServerResponseFilter$filter_Subclass(TemplateResponseFilter$GeneratedServerResponseFilter$filter).filter(ResteasyReactiveContainerRequestContext, ContainerResponseContext) line: not available	
	TemplateResponseFilter$GeneratedServerResponseFilter$filter_Subclass.filter$$superforward1(ResteasyReactiveContainerRequestContext, ContainerResponseContext) line: not available	
	TemplateResponseFilter$GeneratedServerResponseFilter$filter_Subclass$$function$$6.apply(Object) line: not available	
	AroundInvokeInvocationContext.proceed() line: 54	
	InvocationInterceptor.proceed(Invocation$Builder, InvocationContext, ManagedContext, InvocationTree) line: 62	
	InvocationInterceptor.monitor(InvocationContext) line: 49	
	InvocationInterceptor_Bean.intercept(InterceptionType, Object, InvocationContext) line: not available	
	InterceptorInvocation.invoke(InvocationContext) line: 41	
	AroundInvokeInvocationContext.perform(Object, Method, Function<InvocationContext,Object>, Object[], List<InterceptorInvocation>, Set<Annotation>) line: 41	
	InvocationContexts.performAroundInvoke(Object, Method, Function<InvocationContext,Object>, Object[], List<InterceptorInvocation>, Set<Annotation>) line: 32	
	TemplateResponseFilter$GeneratedServerResponseFilter$filter_Subclass.filter(ResteasyReactiveContainerRequestContext, ContainerResponseContext) line: not available	
	TemplateResponseFilter$GeneratedServerResponseFilter$filter_Subclass(ResteasyReactiveContainerResponseFilter).filter(ContainerRequestContext, ContainerResponseContext) line: 10	
	TemplateResponseFilter$GeneratedServerResponseFilter$filter_Subclass.filter$$superforward1(ContainerRequestContext, ContainerResponseContext) line: not available	
	TemplateResponseFilter$GeneratedServerResponseFilter$filter_Subclass$$function$$5.apply(Object) line: not available	
	AroundInvokeInvocationContext.proceed() line: 54	
	InvocationInterceptor.proceed(Invocation$Builder, InvocationContext, ManagedContext, InvocationTree) line: 62	
	InvocationInterceptor.monitor(InvocationContext) line: 49	
	InvocationInterceptor_Bean.intercept(InterceptionType, Object, InvocationContext) line: not available	
	InterceptorInvocation.invoke(InvocationContext) line: 41	
	AroundInvokeInvocationContext.perform(Object, Method, Function<InvocationContext,Object>, Object[], List<InterceptorInvocation>, Set<Annotation>) line: 41	
	InvocationContexts.performAroundInvoke(Object, Method, Function<InvocationContext,Object>, Object[], List<InterceptorInvocation>, Set<Annotation>) line: 32	
	TemplateResponseFilter$GeneratedServerResponseFilter$filter_Subclass.filter(ContainerRequestContext, ContainerResponseContext) line: not available	
	ResourceResponseFilterHandler.handle(ResteasyReactiveRequestContext) line: 24	
	ResourceResponseFilterHandler.handle(AbstractResteasyReactiveContext) line: 9	
	QuarkusResteasyReactiveRequestContext(AbstractResteasyReactiveContext<T,H>).run() line: 141	
	RestInitialHandler.beginProcessing(Object) line: 49	
	ResteasyReactiveVertxHandler.handle(RoutingContext) line: 17	
	ResteasyReactiveVertxHandler.handle(Object) line: 7	
	RouteState.handleContext(RoutingContextImplBase) line: 1193	
	RoutingContextImpl(RoutingContextImplBase).iterateNext() line: 163	
	RoutingContextImpl.next() line: 141	
	StaticResourcesRecorder$2.handle(RoutingContext) line: 67	
	StaticResourcesRecorder$2.handle(Object) line: 55	
	RouteState.handleContext(RoutingContextImplBase) line: 1193	
	RoutingContextImpl(RoutingContextImplBase).iterateNext() line: 126	
	RoutingContextImpl.next() line: 141	
	StaticHandlerImpl.handle(RoutingContext) line: 116	
	StaticHandlerImpl.handle(Object) line: 53	
	StaticResourcesRecorder$1.handle(RoutingContext) line: 42	
	StaticResourcesRecorder$1.handle(Object) line: 38	
	RouteState.handleContext(RoutingContextImplBase) line: 1193	
	RoutingContextImpl(RoutingContextImplBase).iterateNext() line: 126	
	RoutingContextImpl.next() line: 141	
	StaticHandlerImpl.handle(RoutingContext) line: 116	
	StaticHandlerImpl.handle(Object) line: 53	
	StaticResourcesRecorder$1.handle(RoutingContext) line: 42	
	StaticResourcesRecorder$1.handle(Object) line: 38	
	RouteState.handleContext(RoutingContextImplBase) line: 1193	
	RoutingContextImpl(RoutingContextImplBase).iterateNext() line: 163	
	RoutingContextImpl.next() line: 141	
	VertxHttpRecorder$5.handle(RoutingContext) line: 362	
	VertxHttpRecorder$5.handle(Object) line: 340	
	RouteState.handleContext(RoutingContextImplBase) line: 1193	
	RoutingContextImpl(RoutingContextImplBase).iterateNext() line: 163	
	RoutingContextImpl.next() line: 141	
	AuthenticationFailedExceptionMapper.myFilter(RoutingContext) line: 60	
	AuthenticationFailedExceptionMapper_RouteHandler_myFilter_d2ab735b63bd0e474c8ab4f3c0c3587458c080cf.invoke(RoutingContext) line: not available	
	AuthenticationFailedExceptionMapper_RouteHandler_myFilter_d2ab735b63bd0e474c8ab4f3c0c3587458c080cf(RouteHandler).handle(RoutingContext) line: 97	
	AuthenticationFailedExceptionMapper_RouteHandler_myFilter_d2ab735b63bd0e474c8ab4f3c0c3587458c080cf(RouteHandler).handle(Object) line: 22	
	RouteState.handleContext(RoutingContextImplBase) line: 1193	
	RoutingContextImpl(RoutingContextImplBase).iterateNext() line: 163	
	RoutingContextImpl.next() line: 141	
	HttpAuthorizer_Subclass(HttpAuthorizer).doPermissionCheck(RoutingContext, Uni<SecurityIdentity>, int, SecurityIdentity, List<HttpSecurityPolicy>) line: 121	
	HttpAuthorizer_Subclass(HttpAuthorizer).checkPermission(RoutingContext) line: 104	
	HttpAuthorizer_Subclass.checkPermission$$superforward1(RoutingContext) line: not available	
	HttpAuthorizer_Subclass$$function$$1.apply(Object) line: not available	
	AroundInvokeInvocationContext.proceed() line: 54	
	InvocationInterceptor.proceed(Invocation$Builder, InvocationContext, ManagedContext, InvocationTree) line: 62	
	InvocationInterceptor.monitor(InvocationContext) line: 51	
	InvocationInterceptor_Bean.intercept(InterceptionType, Object, InvocationContext) line: not available	
	InterceptorInvocation.invoke(InvocationContext) line: 41	
	AroundInvokeInvocationContext.perform(Object, Method, Function<InvocationContext,Object>, Object[], List<InterceptorInvocation>, Set<Annotation>) line: 41	
	InvocationContexts.performAroundInvoke(Object, Method, Function<InvocationContext,Object>, Object[], List<InterceptorInvocation>, Set<Annotation>) line: 32	
	HttpAuthorizer_Subclass.checkPermission(RoutingContext) line: not available	
	HttpSecurityRecorder$3.handle(RoutingContext) line: 227	
	HttpSecurityRecorder$3.handle(Object) line: 219	
	RouteState.handleContext(RoutingContextImplBase) line: 1193	
	RoutingContextImpl(RoutingContextImplBase).iterateNext() line: 163	
	RoutingContextImpl.next() line: 141	
	HttpSecurityRecorder$2.handle(RoutingContext) line: 200	
	HttpSecurityRecorder$2.handle(Object) line: 60	
	RouteState.handleContext(RoutingContextImplBase) line: 1193	
	RoutingContextImpl(RoutingContextImplBase).iterateNext() line: 163	
	RoutingContextImpl.next() line: 141	
	VertxHttpHotReplacementSetup$5.handle(AsyncResult<Boolean>) line: 196	
	VertxHttpHotReplacementSetup$5.handle(Object) line: 185	
	FutureImpl$3.onSuccess(T) line: 141	
	PromiseImpl<T>(FutureBase<T>).lambda$emitSuccess$0(Listener, Object) line: 54	
	1100382954.run() line: not available	
	AbstractEventExecutor.safeExecute(Runnable) line: 164	
	NioEventLoop(SingleThreadEventExecutor).runAllTasks(long) line: 469	
	NioEventLoop.run() line: 503	
	SingleThreadEventExecutor$4.run() line: 986	
	ThreadExecutorMap$2.run() line: 74	
	FastThreadLocalRunnable.run() line: 30	
	VertxThread(Thread).run() line: 829	

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

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

No response

Additional information

No response

@FroMage FroMage added the kind/bug Something isn't working label Dec 20, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Dec 20, 2021

/cc @geoand, @stuartwdouglas

@geoand
Copy link
Contributor

geoand commented Dec 20, 2021

Do you have sample code that results in this?

@FroMage
Copy link
Member Author

FroMage commented Dec 20, 2021

Not trivially, no. But I figure that any HTTP POST call to a GET method should do it if you have a response filter that calls ContainerResponseContext.getEntity()

@geoand
Copy link
Contributor

geoand commented Dec 21, 2021

I tried a simple example like you described in your comment but was not able to reproduce the problem

@FroMage
Copy link
Member Author

FroMage commented Dec 21, 2021

Well, do you agree that the filter's order is wrong, though? The ResponseHandler's job is to set the response, so we can't call it before the response filters who use that response, right?

@geoand
Copy link
Contributor

geoand commented Dec 21, 2021

Now I am confused... Are you saying that ResponseHandler's should or should not be called before the response filters? Because what we have now is:

        if (!clazz.resourceExceptionMapper().isEmpty() && (instanceHandler != null)) {
            // when class level exception mapper are used, we need to make sure that an instance of resource class exists
            // so we can invoke it
            abortHandlingChain.add(instanceHandler);
        }
        abortHandlingChain.add(ExceptionHandler.INSTANCE);
        abortHandlingChain.add(ResponseHandler.NO_CUSTOMIZER_INSTANCE);
        abortHandlingChain.addAll(responseFilterHandlers);
        abortHandlingChain.add(responseWriterHandler);

which makes sense to me

@FroMage
Copy link
Member Author

FroMage commented Dec 21, 2021

Well, this is what I have:

[org.jboss.resteasy.reactive.server.handlers.ExceptionHandler@246aa690, 
org.jboss.resteasy.reactive.server.handlers.ResourceResponseFilterHandler@2072b0b7, 
org.jboss.resteasy.reactive.server.handlers.ResourceResponseFilterHandler@5da63c4b, 
org.jboss.resteasy.reactive.server.handlers.ResponseHandler@27ee9f20, 
org.jboss.resteasy.reactive.server.handlers.ResponseWriterHandler@5641e36b]

@FroMage
Copy link
Member Author

FroMage commented Dec 21, 2021

Which is wrong, I guess, and the cause for my NPE.

@geoand
Copy link
Contributor

geoand commented Dec 21, 2021

We'll need a reproducer to see why that happens

@FroMage
Copy link
Member Author

FroMage commented Dec 21, 2021

Man, this is so far deeply linked with OIDC that I'm not sure I can give you an easy one. I'll try.

@geoand
Copy link
Contributor

geoand commented Dec 21, 2021

Thanks!

@FroMage
Copy link
Member Author

FroMage commented Jan 6, 2022

Well, this is how we build the handler chain:

        List<ServerRestHandler> abortHandlingChain = new ArrayList<>(3);

        if (interceptorDeployment.getGlobalInterceptorHandler() != null) {
            abortHandlingChain.add(interceptorDeployment.getGlobalInterceptorHandler());
        }
        abortHandlingChain.add(new ExceptionHandler());
        if (!interceptors.getContainerResponseFilters().getGlobalResourceInterceptors().isEmpty()) {
            abortHandlingChain.addAll(interceptorDeployment.getGlobalResponseInterceptorHandlers());
        }
        abortHandlingChain.add(ResponseHandler.NO_CUSTOMIZER_INSTANCE);
        abortHandlingChain.add(new ResponseWriterHandler(dynamicEntityWriter));

So we do have the response filters before we set the response from the result.

@FroMage
Copy link
Member Author

FroMage commented Jan 6, 2022

And the response filters can call ContainerResponseContextImpl.getEntity() which requires a Response being set, which is not the case if we're getting a NotAcceptableException generated before the endpoint is called, so there's no Response and the filter will NPE if it tries to look at the current entity.

@FroMage
Copy link
Member Author

FroMage commented Jan 6, 2022

By comparison, the normal handlers are built this way:

            addResponseHandler(method, handlers);
            addHandlers(handlers, clazz, method, info, HandlerChainCustomizer.Phase.AFTER_RESPONSE_CREATED);
            responseFilterHandlers = new ArrayList<>(interceptorDeployment.setupResponseFilterHandler());
            handlers.addAll(responseFilterHandlers);
            handlers.add(responseWriterHandler);

@FroMage
Copy link
Member Author

FroMage commented Jan 6, 2022

So I assume that normally, response filters go after the response handler. I'll change it and make a PR.

@FroMage
Copy link
Member Author

FroMage commented Jan 6, 2022

Well, it's more than just that, because if I do this, I'm out of NPE, but I get a 200 OK and the exception is swallowed.

@FroMage
Copy link
Member Author

FroMage commented Jan 6, 2022

OK, even more than that. Turns out this time it's my mock that's generating this NotAcceptableException, because I messed up and only implemented a form response when our OIDC interceptor asked for JSON. With this fix, however, for some reason we're returning a 204 (created), which is wrong, and even more wrong our OIDC interceptor turns this into an empty 200 and all errors are hidden from me.

@FroMage
Copy link
Member Author

FroMage commented Jan 6, 2022

Found the bug that turns NotAcceptableException into 204!

This happens because I have an exception mapper that doesn't map to anything:

    @ServerExceptionMapper
    public Response mapIt(RuntimeException x) {
        return null;
    }

That was left around after a test.

ExceptionMapper says:

Returning {@code null} results in a {@link javax.ws.rs.core.Response.Status#NO_CONTENT} response.

So damn, this is correct. OK, that's one bug less. Now let's see about the others.

@FroMage
Copy link
Member Author

FroMage commented Jan 6, 2022

So, now, it looks like from the OIDC perspective, it's intercepting the callback call, making the rest client call to GH, which returns a 406 (fine), which triggers a throwing of io.quarkus.security.AuthenticationCompletionException, which I guess is fine, but right at the same time as it throws it, when we're going to start wondering how to map this exception, something already returned an answer to the client. It looks like it was my exception mapper that was badly coded. I should really try the latest main which allows me to register a handler for auth exceptions, this way I don't have to do it in Vert.x routes.

OK, so now I got to the bottom of this. I'll make a PR for this as soon as I can.

@geoand
Copy link
Contributor

geoand commented Jan 7, 2022

I should really try the latest main which allows me to register a handler for auth exceptions, this way I don't have to do it in Vert.x routes.

Definitely :)

@geoand
Copy link
Contributor

geoand commented Jan 12, 2022

Any update on this?

@FroMage
Copy link
Member Author

FroMage commented Jan 18, 2022

Lemme create a test.

FroMage added a commit to FroMage/quarkus that referenced this issue Jan 19, 2022
@FroMage
Copy link
Member Author

FroMage commented Jan 19, 2022

Finally: #23004

gsmet added a commit that referenced this issue Jan 19, 2022
RR: Fix abort chain order for #22408
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jan 19, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jan 21, 2022
@geoand
Copy link
Contributor

geoand commented Jan 24, 2022

Fixed in #23004

@geoand geoand closed this as completed Jan 24, 2022
@geoand geoand added this to the 2.6.3.Final milestone Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rest kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants