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

RequestMapping method returning Future with null result causes NullPointerException [SPR-16072] #20621

Closed
spring-issuemaster opened this issue Oct 16, 2017 · 9 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Oct 16, 2017

martian opened SPR-16072 and commented

RequestMapping returns a Future with anytype generic parameter and async result is null leads to NullPointerException
It's fix with 5.0.0.RELEASE


Affects: 4.3.12

Reference URL: https://github.com/zhanhb/future-null-pointer-exception

Issue Links:

  • #20624 setArguments(null) on MethodInvoker no longer coerces null to Object[0]

Referenced from: commits 6f65b63, c2438cb

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 16, 2017

Juergen Hoeller commented

Could you post a stacktrace of that NPE please? I have backported quite a few nullability refinements from 5.0 already, so I'm up for fixing this one as well... but it'd be great to know which of the many refinements in 5.0 actually addressed this issue here.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 16, 2017

martian commented

https://travis-ci.org/zhanhb/future-null-pointer-exception
org.springframework.web.util.NestedServletException: Request processing failed; nested exception is java.lang.NullPointerException
at java.lang.Class.isAssignableFrom(Native Method)
at org.springframework.web.servlet.mvc.method.annotation.ModelAndViewMethodReturnValueHandler.supportsReturnType(ModelAndViewMethodReturnValueHandler.java:72)
at org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite.selectHandler(HandlerMethodReturnValueHandlerComposite.java:90)
at org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite.handleReturnValue(HandlerMethodReturnValueHandlerComposite.java:77)
at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:113)
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:827)
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:738)
at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:85)
at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:967)
at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:901)
at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:970)
at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:861)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:635)
at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:846)
at org.springframework.test.web.servlet.TestDispatcherServlet.service(TestDispatcherServlet.java:65)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:742)
at org.springframework.mock.web.MockFilterChain$ServletFilterProxy.doFilter(MockFilterChain.java:160)
at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:127)
at org.springframework.test.web.servlet.MockMvc.perform(MockMvc.java:155)
at com.example.demo.DemoApplicationTests.test(DemoApplicationTests.java:43)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 16, 2017

Rossen Stoyanchev commented

We support ListenableFuture and CompletableFuture since those can be adapted to a DeferredResult. The only way to support a plain Future is to call get() and block in order to obtain the result and I don't think we do that. So I don't see how it could work on 5.0 in the end even if it avoids the NPE.

Based on the above stack trace my guess is that Future falls through the checks of all return value handlers and ends up being treated as a model attribute and that's most likely not intended (at least not as an unresolved Future).

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 16, 2017

Juergen Hoeller commented

From my perspective, the real bug here is that our return type check causes an NPE against such a Future handle with a null value. I intend to backport that part to 4.3.13 but won't change anything about actually supported return types.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 16, 2017

Rossen Stoyanchev commented

Okay sounds good to me. I was just qualifying the expectations around what works.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 16, 2017

Juergen Hoeller commented

This should be resolved in the upcoming 4.3.13.BUILD-SNAPSHOT now, based on a selective backport from 5.0. Please give a try and let me know whether it works for you...

Please note that, as per Rossen's comment, your Future declaration may not behave as expected. However, there should be no NullPointerException anymore now.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 17, 2017

martian commented

I get NullPointerException with spring-core-4.3.13.BUILD-20171016.211232-3.jar
https://travis-ci.org/zhanhb/future-null-pointer-exception/builds/288665336

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 17, 2017

Juergen Hoeller commented

Indeed, this was missing overridden MethodParameter.getParameterType variants which we're covering as well now, in particular the ones with use of ResolvableType.

Please give the upcoming 4.3.13.BUILD-SNAPSHOT yet another try...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 17, 2017

martian commented

The test is passed now, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.