Avoid matching multipart parameters annotated with @ModelAttribute #3277
Conversation
The ProxyHandlerMethodArgumentResolver now avoids matching multipart parameters annotated with @ModelAttribute. This allows multipart parameters to be handled by RequestParamMethodArgumentResolver which properly handles multipart arguments. Fixes spring-projects#3258 Related tickets spring-projects#2937 Signed-off-by: Chris Bono <[email protected]>
| if (parameter.getParameterAnnotation(ProjectedPayload.class) != null | ||
| || parameter.getParameterAnnotation(ModelAttribute.class) != null) { | ||
| // Annotated parameter (excluding multipart @ModelAttribute) | ||
| if (parameter.hasParameterAnnotation(ProjectedPayload.class) || |
There was a problem hiding this comment.
It seems that the proxy handler does not properly handle multipart arguments, but the request param method handler does properly support multipart params. Is this the proper fix or should the proxy handler also support multipart args properly? cc: @odrotbohm @mp911de
There was a problem hiding this comment.
We have various aspects here. We should deprecate unannotated proxying as that has proven to cause quite some issues. We also need to have a plan how to achieve this and how our migration path would look like.
I'm thinking about:
- (backport) 3.4.x: Introduce a warning log if a parameter is not annotated with
@ProjectedPayloadthat this style is deprecated and that we will drop support for projections if a parameter is not annotated with@ProjectedPayload - 3.5: stop supporting parameters that are not annotated with
@ProjectedPayloadbut keep the logging - 4.0: remove logging
Introducing warn logging can cause quite some fallout, so we should be mindful about the conditions and maybe once we have seen a parameter, that we do not repeat logging (caching?) for that particular occurence.
The ProxyHandlerMethodArgumentResolver now avoids matching multipart parameters annotated with @ModelAttribute. This allows multipart parameters to be handled by RequestParamMethodArgumentResolver which properly handles multipart arguments.
In 3.3.3 the
ProxyingHandlerMethodArgumentResolverwould report that it does not support the@ModelAttribute MultipartFile fileparameter and then the next handler in the chain (RequestParamMethodArgumentResolver) was able to properly handle the multipart file arg.In 3.3.4 (commit) the
ProxyingHandlerMethodArgumentResolverreports that it does support the@ModelAttribute MultipartFile fileparameter and then it fails to properly handle the multipart file arg.Fixes #3258
Related tickets #2937