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

ServletRequestDataBinder assumes Standard servlet multipart handling #26999

Closed
svschouw-bb opened this issue May 28, 2021 · 1 comment
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Milestone

Comments

@svschouw-bb
Copy link

Affects: Spring Framework (spring-web) 5.3.7


In our project we have one controller which needs to handle very large file uploads using a streaming solution similar to this post.

However, since Spring Framework 5.3 (d61c0ee), the ServletRequestDataBinder, when it sees a "multipart/" content type which is not handled by a MultipartResolver yet it tries to resolve it itself by calling StandardServletPartUtils.bindParts(...), which in turn calls HttpServletRequest.getParts() which throws an exception because multipart is not configured on servlet level.

Switching to standard servlet multipart wouldn't help in this case, because that would make the request non-streaming.

I'm not entirely sure what a good solution would be, since as far as I know you can't really ask the servlet whether multipart is configured, and HttpServletRequest.getParts() throwing an IllegalStateException could also be due to exceeding file limitations. Maybe some addition to the MultipartResolver interface, or something in WebMvcConfigurationSupport to configure it?

Extra context

Some extra context, in case it matters: we are using the CommonsMultipartResolver because we have many controllers which need to handle normal file uploads. For the very large file upload we use a PUT request instead of POST, because CommonsMultipartResolver.isMultipart(...) only returns true for POST requests, so it doesn't try to process the multipart request yet in the DispatcherServlet.

We use multipart instead of just using the file itself as the body, because we also need to include some other information which might not fit the url query parameters.

The ServletRequestDataBinder gets triggered (I think) because we also include a model object as a method parameter which gets filled from url query parameters and gets validated. If you want I can include an example of the controller method.

@jhoeller jhoeller self-assigned this May 28, 2021
@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression labels May 28, 2021
@jhoeller jhoeller added this to the 5.3.8 milestone May 28, 2021
@jhoeller jhoeller modified the milestones: 5.3.8, 5.3.9 Jun 8, 2021
@jhoeller
Copy link
Contributor

Since this fallback binding was only ever meant for form uploads (and that's also the only kind of request that the Servlet spec actually defines for its multipart support - #26826), I've restricted our fallback multipart binding in all applicable places to POST requests with multipart/form-data content. Any other kind of multipart request is left untouched by the binders by default, requiring a MultipartResolver setup for consideration in the binding algorithm. This should address the regression you reported while still being in line with the motivation for the original change in 5.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

2 participants