MockMvc's MVC_RESULT_ATTRIBUTE lost with HandlerMappingIntrospector and RouterFunctions in use

[jzheaux]

Related to spring-projects/spring-security-samples#9

A contributor shared the following sample application: https://github.com/hantsy/spring-webmvc-auth0-sample

The tests result in a NullPointerException because the MockMvc.MVC_RESULT_ATTRIBUTE is missing.

HandlerExecutionChain chain = super.getHandler(request);
if (chain != null) {
	DefaultMvcResult mvcResult = getMvcResult(request); // returns null
	mvcResult.setHandler(chain.getHandler());
	mvcResult.setInterceptors(chain.getInterceptors());
}
return chain;

It gets removed due to the following arrangement:

• Spring Security's CorsFilter by default uses HandlerMappingIntrospector
• HandlerMappingIntrospector uses RequestAttributeChangeIgnoringWrapper which ignores all but PATH_ATTRIBUTE
• RequestPredicates#restoreAttributes attempts to restore the attributes to a previous state by clearing the attribute set and then re-adding each attribute one by one

Before CorsFilter runs, MVC_RESULT_ATTRIBUTE is present in the request. When RequestPredicates#restoreAttributes is run, it removes all attributes. Then, when it tries to add the original set back in, RequestAttributeChangeIgnoringWrapper only adds PATH_ATTRIBUTE back in.

For the specific sample, the tests can be repaired by removing the CorsFilter or by exposing a custom CorsConfigurationSource bean since either of those will prevent RequestAttributeChangeIgnoringWrapper from wrapping the request.

I was also able to fix the tests by adding the following to RequestAttributeChangeIgnoringWrapper:

@Override
public void removeAttribute(String name) {
	if (name.equals(ServletRequestPathUtils.PATH_ATTRIBUTE) || name.equals(UrlPathHelper.PATH_ATTRIBUTE)) {
		super.removeAttribute(name);
	}
}

At least in this isolated case, it seems reasonable that if an attribute cannot be set, it also should not be able to be removed.

[sbrannen]

See also: discussion on Stack Overflow

[rstoyanchev]

Thanks for the detailed report and investigation @jzheaux. HandlerMappingIntrospector is only intended to lookup info that Spring Security needs, and the intent is to have no side effects. This why the request is wrapped but it is true that side effects can also come in the form of removals.

I will experiment with a fix that maintains an independent map of attributes at the wrapper level, which would leave the original request attributes untouched for further use after the lookup when the wrapper would no longer be in use.