Skip to content

Remove support for "request params" from WebFlux [SPR-15508] #20067

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

Closed
spring-projects-issues opened this issue May 2, 2017 · 3 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: task A general task
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented May 2, 2017

Rossen Stoyanchev opened SPR-15508 and commented

In the Servlet API "request params" provide Map-like access to both URI query params and form data (parsed from the request body). The concept fits well in the Servlet API where "request params" are accessed in a blocking fashion whenever needed. It does not however fit with use of the Servlet 3.1 non-blocking I/O API which is mutually exclusive with any calls to get "request params" since those may block.

In M3 #19567 introduced support for Servlet API style "request params" in WebFlux by parsing form data eagerly (without blocking) and then caching it. This was a reasonable trade-off for simple forms that made it feasible to support the concept of "request params".

Recently #19114 introduced support for multipart form data. A multipart request may represent a browser form with a file input field, or it be a non-browser client sending a request with potentially large parts of data of any media type. Support for "request params" is now a challenge. While eager parsing + caching multipart form data may be okay for browser forms (in combination with a max file upload limit) it precludes support for streaming large multiparts sequentially, something that is a natural fit for WebFlux, e.g. a Flux<Part> controller method argument.

This ticket is to remove the concept of "request params" from WebFlux in favor of the more explicit, existing support for access to query params vs form data. We can continue to support data binding of both URI query params and form data (including multipart data) to a command object (via @ModelAttribute). In WebFlux however @RequestParam or @RequestMapping(params="...") would have to refer to URI query params only. This trade-off seems a better fit for WebFlux.

In short the concept of a single bucket (Map) for something so disparate as simple URI query params on one end and very large parts in a multipart form data request on the other is just not feasible while also providing support for on-demand, non-blocking parsing of large content.


Affects: 5.0 M5

Issue Links:

Referenced from: commits 1881727

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented May 2, 2017

Rossen Stoyanchev commented

Daniel Fernández, what this means for the use case from #19567 is you cannot map to two different @RequestMapping methods based on which button was pressed.

@spring-projects-issues
Copy link
Collaborator Author

Daniel Fernández commented

Thanks Rossen, understood. I'll take this into account and modify my examples then.

So the expected way to access form data will now be either using ServerWebExchange#getFormData() from the controller code, or of course binding form data to a form-backing bean, correct?

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Yes you can bind form data (including multipart form data) to a form-backing bean. Or at a lower level, and outside a controller, you can use ServerWebExchange#getFormData() and ServerWebExchange#getMultipartData() both of which are safe to call more than once.

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: task A general task
Projects
None yet
Development

No branches or pull requests

2 participants