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

Principal check in ServletRequestMethodArgumentResolver can result in type mismatches [SPR-15214] #19779

Closed
spring-issuemaster opened this issue Feb 1, 2017 · 4 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Feb 1, 2017

George Hawkins opened SPR-15214 and commented

The Principal check in ServletRequestMethodArgumentResolver can result in the method returning a value that is clearly of a type that's unassignable to something of paramType.

This gist is a JUnit test that cuts things down to a minimum and demonstrates that the Principal check will happily return something that is a Principal subclass in the apparent belief it will be assignable to any other subclass of Principal - i.e. the same as thinking that a Double value will be assignable to an Integer on the basis that both are subclasses of Number.

Note: just to be clear this test is expected to fail - it's demonstrating our problem case.


Affects: 4.3.6

Reference URL: https://gist.github.com/george-hawkins/3b030c04a55d03b85a99bc69228648b5

Issue Links:

  • #19780 MockHttpServletRequest.getReader() returns null in case of no content

Referenced from: commits e44533f, fc11321, f117b80, 6014ca9

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 1, 2017

Juergen Hoeller commented

Like with other such assignability checks in ServletRequestMethodArgumentResolver, we assume that if you declare any Principal subclass you always mean to resolve against the user principal associated with the request, expecting it to have a principal of that type. However, point taken, instead of letting this run into a ClassCastException we should raise a meaningful exception that the expected principal subclass doesn't match the request at runtime.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 1, 2017

Juergen Hoeller commented

Like with a mismatching request argument type, we're throwing an IllegalStateException for a user principal type mismatch as well now.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 20, 2017

Andy Wilkinson commented

Was it intended that an IllegalArgumentException is now thrown when the Principal is null? Trying to build Boot against 4.3.7 snapshots is failing in a few places with:

org.springframework.web.util.NestedServletException: Request processing failed; nested exception is java.lang.IllegalStateException: Current user principal is not of type [java.security.Principal]: null
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:982)
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:861)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:687)
	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:790)
	at org.springframework.mock.web.MockFilterChain$ServletFilterProxy.doFilter(MockFilterChain.java:167)
	at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:134)
	at org.springframework.test.web.servlet.MockMvc.perform(MockMvc.java:155)
	at org.springframework.boot.actuate.endpoint.mvc.HalBrowserMvcEndpointDisabledIntegrationTests.endpointsDoNotHaveLinks(HalBrowserMvcEndpointDisabledIntegrationTests.java:87)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks.evaluate(RunBeforeTestMethodCallbacks.java:75)
	at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:86)
	at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:84)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:252)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:94)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.springframework.test.context.junit4.statements.RunBeforeTestClassCallbacks.evaluate(RunBeforeTestClassCallbacks.java:61)
	at org.springframework.test.context.junit4.statements.RunAfterTestClassCallbacks.evaluate(RunAfterTestClassCallbacks.java:70)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.run(SpringJUnit4ClassRunner.java:191)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: java.lang.IllegalStateException: Current user principal is not of type [java.security.Principal]: null
	at org.springframework.web.servlet.mvc.method.annotation.ServletRequestMethodArgumentResolver.resolveArgument(ServletRequestMethodArgumentResolver.java:129)
	at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:121)
	at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:158)
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:128)
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:116)
	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:963)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:897)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:970)
	... 39 more
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 20, 2017

Juergen Hoeller commented

Nope. I'll fix this ASAP.

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