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

MultiFileMap not being set when Standard Spring Security is Enabled #13837

Closed
codeconsole opened this issue Nov 14, 2024 · 0 comments · Fixed by #13838
Closed

MultiFileMap not being set when Standard Spring Security is Enabled #13837

codeconsole opened this issue Nov 14, 2024 · 0 comments · Fixed by #13838

Comments

@codeconsole
Copy link
Contributor

codeconsole commented Nov 14, 2024

MultPartRequests do not process correctly when using Spring Security (without plugin)

Replication instructions

just include the spring-boot-starter on any app that process uploads

implementation "org.springframework.boot:spring-boot-starter-security"

and notice that params.file does not bind.

Description

During a normal workflow the handler mapping is retrieved after the multipartRequest is parsed.
DispatcherServlet.doDispatch

DispatcherServlet.doDispatch(..) {
        // ...
	processedRequest = checkMultipart(request); // if true, wraps request with MultipartHttpServletRequest
	multipartRequestParsed = (processedRequest != request);

	// Determine handler for the current request.
	mappedHandler = getHandler(processedRequest); 
        // Creates GrailsParameterMap for first time with GrailsParameterMap because originalParams is null
}

public GrailsParameterMap getParams() {
if (params == null) {
resetParams();
}
return params;
}
/**
* @return The Grails params object
*/
public GrailsParameterMap getOriginalParams() {
if (originalParams == null) {
originalParams = new GrailsParameterMap(getCurrentRequest());
}
return originalParams;
}
/**
* Reset params by re-reading and initializing parameters from request
*/
public void resetParams() {
params = (GrailsParameterMap)getOriginalParams().clone();
}

public GrailsParameterMap(HttpServletRequest request) {
this.request = request;
final Map requestMap = new LinkedHashMap(request.getParameterMap());
if (requestMap.isEmpty() && ("PUT".equals(request.getMethod()) || "PATCH".equals(request.getMethod())) && request.getAttribute(REQUEST_BODY_PARSED) == null) {
// attempt manual parse of request body. This is here because some containers don't parse the request body automatically for PUT request
String contentType = request.getContentType();
if (MimeType.FORM.equals(new MimeType(contentType))) {
try {
Reader reader = request.getReader();
if(reader != null) {
String contents = IOUtils.toString(reader);
request.setAttribute(REQUEST_BODY_PARSED, true);
requestMap.putAll(WebUtils.fromQueryString(contents));
}
} catch (Exception e) {
LOG.error("Error processing form encoded " + request.getMethod() + " request", e);
}
}
}
if (request instanceof MultipartHttpServletRequest) {
MultiValueMap<String, MultipartFile> fileMap = ((MultipartHttpServletRequest)request).getMultiFileMap();
for (Entry<String, List<MultipartFile>> entry : fileMap.entrySet()) {
List<MultipartFile> value = entry.getValue();
if(value.size() == 1) {
requestMap.put(entry.getKey(), value.get(0));
}
else {
requestMap.put(entry.getKey(), value);
}
}
}
updateNestedKeys(requestMap);
}

But with SpringSecurity getHandler is called first in HandlerMappingIntrospector which results in originalParams being not null and MultiFileMap not being set.

HandlerMappingIntrospector.getMatchableHandlerMapping

	public MatchableHandlerMapping getMatchableHandlerMapping(HttpServletRequest request) throws Exception {
		CachedResult result = CachedResult.getResultFor(request);
		if (result != null) {
			return result.getHandlerMapping();
		}
		this.cacheLogHelper.logHandlerMappingCacheMiss(request);
		HttpServletRequest requestToUse = new AttributesPreservingRequest(request);
		return doWithHandlerMapping(requestToUse, false,
				(mapping, executionChain) -> createMatchableHandlerMapping(mapping, requestToUse));
	}

	private <T> T doWithHandlerMapping(
			HttpServletRequest request, boolean ignoreException,
			BiFunction<HandlerMapping, HandlerExecutionChain, T> extractor) throws Exception {

		Assert.state(this.handlerMappings != null, "HandlerMapping's not initialized");

		boolean parsePath = !this.pathPatternMappings.isEmpty();
		RequestPath previousPath = null;
		if (parsePath) {
			previousPath = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE);
			ServletRequestPathUtils.parseAndCache(request);
		}
		try {
			for (HandlerMapping handlerMapping : this.handlerMappings) {
				HandlerExecutionChain chain = null;
				try {
					chain = handlerMapping.getHandler(request);
                // ...

This is because standard Spring Security uses the RequestMatcherDelegatingAuthorizationManager which uses the HandlerMappingIntrospector.

org.grails.web.servlet.mvc.GrailsWebRequest.getOriginalParams(GrailsWebRequest.java:255)
org.grails.web.servlet.mvc.GrailsWebRequest.resetParams(GrailsWebRequest.java:264)
org.grails.web.servlet.mvc.GrailsWebRequest.getParams(GrailsWebRequest.java:245)
org.grails.web.mime.HttpServletResponseExtension.getMimeTypeForRequest(HttpServletResponseExtension.groovy:134)
org.grails.web.mime.HttpServletResponseExtension.getMimeType(HttpServletResponseExtension.groovy:127)
org.grails.web.mime.DefaultMimeTypeResolver.resolveResponseMimeType(DefaultMimeTypeResolver.groovy:41)
org.grails.web.mapping.mvc.UrlMappingsHandlerMapping.findRequestedVersion(UrlMappingsHandlerMapping.groovy:184)
org.grails.web.mapping.mvc.UrlMappingsHandlerMapping.getHandlerInternal(UrlMappingsHandlerMapping.groovy:132)
org.springframework.web.servlet.handler.AbstractHandlerMapping.getHandler(AbstractHandlerMapping.java:509)
org.springframework.web.servlet.handler.HandlerMappingIntrospector.doWithHandlerMapping(HandlerMappingIntrospector.java:390)
org.springframework.web.servlet.handler.HandlerMappingIntrospector.setCache(HandlerMappingIntrospector.java:268)
org.springframework.web.servlet.handler.HandlerMappingIntrospector.lambda$createCacheFilter$3(HandlerMappingIntrospector.java:241)
org.springframework.web.filter.CompositeFilter$VirtualFilterChain.doFilter(CompositeFilter.java:113)
org.springframework.web.filter.CompositeFilter.doFilter(CompositeFilter.java:74)

It does not happen with the Spring Security Core plugin because it does not use HandlerMappingIntrospector

--

Note: this same problem could occur in any Filter or code that calls GrailsWebRequest.getParams() prior to the DispatcherServlet invoking checkMultipart(request).

It seems like others have attempted workarounds by calling WebUtils.clearGrailsWebRequest() which IMO is a little hacky.

Debugging this I also noticed inefficiencies such as how GrailsControllerUrlMappings.matchAll works. It appears StandardServletMultipartResolver.resolveMultipart is invoked multiple times during the same request.

#13841

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant