Support for form data via @RequestParam on WebFlux [SPR-16190]

[spring-projects-issues]

Miroslav Hrúz opened SPR-16190 and commented

In Spring 4 and Spring 5/ Web (Servlet API) you could write annotated controller method without explicitly say @GetMapping or @PostMapping and without any @RequestParam or @ModelAttribute

@RequestMapping("/get/and/post/form-data-www-urleconded")
public String getAndPost(String param1, String param2) {
	return "Got: param1="+param1+", param2="+param2;
}

Which does except others:

1. Binds /get/and/post/form-data-www-urleconded to be used with HTTP GET and param1, param2 parameters to HTTP query parameters
2. Binds /get/and/post/form-data-www-urleconded to be used with HTTP POST form data www-urlencoded and param1, param2 to HTTP POST form data.
3. Binds /get/and/post/form-data-www-urleconded to be used with HTTP POST multipart data (FormFieldPart) and param1, param2 to HTTP POST multipart data

In Spring WebFlux 5.0.1 it's not working. It works in Spring Web 5.0.1 and in Spring Web 4.x.

The desired use case is when you want to expose REST API for both HTTP GET and POST and you want to simple write the method ones, not to use @GetMapping with appropriate @RequestParam or @PostMapping with @ModelAttribute or @RequestBody. The beauty is not to use any argument annotation at all.

I've attached example projects for spring 4.x, Spring 5.0.1 Web and Spring 5.0.1 WebFlux.

requestMapping_for_post test is failing and should not.

@Test
public void requestMapping_for_get() {
	final WebTestClient client = WebTestClient.bindToController(new TestController()).build();

	final String responseBody = client.get()
			.uri("/get/and/post/form-data-www-urleconded?param1=111&param2=222")
			.exchange()
			.expectStatus().isOk()
			.expectBody(String.class)
			.returnResult().getResponseBody();

	Assert.assertEquals("Got: param1=111, param2=222", responseBody);
}

@Test
public void requestMapping_for_post() {
	final WebTestClient client = WebTestClient.bindToController(new TestController()).build();

	final MultiValueMap<String, String> formData = new LinkedMultiValueMap<>();
	formData.add("param1", "111");
	formData.add("param2", "222");

	final String responseBody = client.post()
			.uri("/get/and/post/form-data-www-urleconded")
			.body(BodyInserters.fromFormData(formData))
			.exchange()
			.expectStatus().isOk()
			.expectBody(String.class)
			.returnResult().getResponseBody();

	Assert.assertEquals("Got: param1=111, param2=222", responseBody);
}

---

Affects: 5.0.1

Attachments:

• FormDataWorkaround.java (2.81 kB)
• spring4-formData.zip (58.26 kB)
• spring5-webflux-formdata.zip (59.16 kB)
• spring5-web-formdata.zip (59.02 kB)

Issue Links:

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

[spring-projects-issues]

Miroslav Hrúz commented

I've tried also POST multipart data for simple fields (e.g. String) in Spring 4.x and 5.0.1 Web and WebFlux and it worked also for them except in 5.0.1 WebFlux.

Is there any e.g. performance reason for drop this feature?

Thank you for your response.

[spring-projects-issues]

Rossen Stoyanchev commented

Sorry for the slow response.

In the Servlet API, a request.getParameter comes from a query parameter or a value pair from the body of a form POST. So in Spring MVC an @RequestParam argument (or any simple type argument without an annotation is treated the same way) simply translates into a call to getParameter.

In Spring WebFlux @RequestParam resolves to query parameters only. To get it from the body of a form POST would require reading the request body, which is a blocking operation. You can see some more details at #20067. Note however that data binding still binds from both query params and form data. So you can create a command object that contains both "param1" and "param2" as fields, and that should work just fine.

[spring-projects-issues]

Zhiyuan Zheng commented

Thanks for the fantastic framework. When I was migrating my services to Webflux, I am facing some difficult questions. Our frontend uses the following entity to request the backend controller:

POST / HTTP/1.1
Content-Type: application/x-www-form-urlencoded

interface=xxx&content=yyy

As you can see, It's a form post. Prior to Webflux, I use @RequestParam("interface") to retrieve these parameters. In webflux, the semantics of @RequestParam has been changed, thus I can't retrieve form-post parameters by @RequestParam. In most circumstances, I can make up an entity object and works well. Here, I have a key in form called "interface". As far as I know, I cannot create an alias for form-post easily.

Can you have a look on this? Thanks!

[spring-projects-issues]

Zhiyuan Zheng commented

When the form contains preserved word as key (e.g. "int", "interface"), it's difficult to create a form post controller.

[spring-projects-issues]

Zhang Jie commented

@Zhiyuan Zheng According Springframework reference:

Unlike the Servlet API "request paramater" concept that conflate query parameters, form data, and multiparts into one, in WebFlux each is accessed individually through the ServerWebExchange. While @RequestParam binds to query parameters only, you can use data binding to apply query paramerters, form data, and multiparts to a command object.

You can also use @RequestBody & MultiValueMap

[spring-projects-issues]

Zhiyuan Zheng commented

@Zhang JIe

Thanks for your reply. As I said earlier, I have a key named "interface", so it's not easy to mapping the request to a command object since interface is a reversed word in Java.

Do you know some way to make alias for such fields like @JsonField in JSON binding ?

[spring-projects-issues]

Miroslav Hrúz commented

Sorry guys, I am looking at the answers and must interact :)

 I liked the @RequestMapping for both GET and POST because it was convenient and easy to write and maintain plenty of signatures in plenty of controllers.

It seems to me that "leaking abstraction", some implementation decisions are now involving the API itself. Why are Spring guys don't want to fix it? It works in WebMVC so it seems to me that it should work in webflux also. The reason that it's blocking operation and it's not easy to access it non blocking. Okay, but you have an annotation when it's working and you've whole async request/response object under your control. So it's possible to write it async. Just you don't want it because of:

1. it's complicated, you must change more interfaces and you don't want to do it in minor project update.

2. you've more valuable work to do.

3. it's connected with Project Reactor

I understand all of the reasons. Just want to say, it's a implementation detail which is influces the API and mostly it's bad thing...

 

 

[spring-projects-issues]

Miroslav Hrúz commented

Just for the record. Here is my workaround. 

FormDataWorkaround.java

[spring-projects-issues]

Zhang Jie commented

@Zhiyuan Zhen
If your form field is "interface", and pojo field name is "interFace" ( certainly can not be "interface"), that is a invalid use case.You can either use MultiValueMap (if you really want to use pojo, you can chage MultiValueMap to pojo manually) or rename form field to "interFace" or else. Finally, an unusual way is that, name your field as "interFace", name the getter and setter as "getInterface()" and "setInterface()", in @RequestMapping method, use @ModelAttribute
Besides, if you want to use @JsonField, post json, not application/x-www-form-urlencoded.

[spring-projects-issues]

Rossen Stoyanchev commented

Miroslav Hrúz, first a couple of clarifications:

1. @RequestMapping does work for both GET and POST, but I presume you meant @RequestParam does not work for both.
2. "Request parameters" are a Servlet API concept (not in any HTTP specs). It combines parameters from the query with parameters from the body (form or multipart data). I presume you know that but I mention it because technically @RequestParam does work for both GET and POST, it's just that it only provides access to query params.

Support for form data and multipart data via @RequestParam on @RequestMapping methods alone is feasible. The challenge is to provide the same in all places where expected.

Consider that annotation-based request mappings (which include the "params" conditions) work only with data readily available from the request. To change that would require automatically parsing the body to map the request, which could be done asynchronously but would require potentially lots of memory. The second example is @InitBinder methods used to initialize WebDataBinder instances, e.g. for data binding but also for simple tasks like type conversion or controller method arguments like @PathVariable, @RequestParam, etc. We don't want to automatically parse the body to achieve that.

So to answer your question, it's not that we don't want to fix this ticket, it's that a decision was made before, after multiple iterations (as described in #20067). We can not go back, but we can go forward and evolve, not necessarily seeking exact parallels with Spring MVC and the Servlet API, which isn't always going to be practical.

Could you explain in more detail, any specific reasons why binding onto a command object @ModelAttirbute is not a good fit, or not possible? The case of Zhiyuan Zheng is one such example, or is it perhaps something more along the lines of #18012? Having more detail around that helps to provide ideas on possible alternatives.

Zhiyuan Zheng could you clarify if you have the option to change those form field names to something more appropriate? Zhang Jie is right that you can use @RequestBody to obtain a MultiValueMap<String, String> but that only provides access to the raw values (no type conversion and no validation).

[spring-projects-issues]

Rossen Stoyanchev commented

Just for the record. Here is my workaround.

By the way, when the body cannot be read immediately, those subscribe methods in the FormDataWorkaround may not be called immediately, so consider them asynchronous. You have to compose this as a connected flow, for example through the zip operator like we do for data binding. Also instead of reflection you might be able to use the ServerWebExchange mutate method along with a similar option in ServerHttpRequest.

We could provide such a filter, that when enabled would pre-parse the body like that, automatically upgrading @RequestParam to also expose body data. But let's discuss issues first before solutions.

[spring-projects-issues]

Zhang Jie commented

Hi, all, I have found that, if we want to use custom @FormAttribute or something else to customize x-www-form-urlencoded, we can write a custom FormAttributeBeanInfoFactory which is similar to ExtendedBeanInfoFactory, and which can create FormAttributeBeanInfo similar to ExtendedBeanInfo using @FormAttribute to get property name.
When we config FormAttributeBeanInfoFactory into /META-INF/spring.factories with key org.springframework.beans.BeanInfoFactory, it will be used by WebDataBinder and BeanWrapperImpl, and will work as expected with @ModelAttribute.
I didn't test all test case, but i think it will be an alternative way to custom parameter name when binding an object.

[spring-projects-issues]

Rossen Stoyanchev commented

After some further thought, I'm scheduling for a potential fix in 5.1. I plan to experiment with the idea for @RequestParam, when used on an @RequestMapping method, to support checking form and possibly multipart data (i.e. to mirror Spring MVC).

To be clear, @RequestMapping(params) will still consider query params only, and hence avoid parsing the body only to map requests, and likewise when used on an @InitBinder methods, @RequestParam will be limited to the query.

This should be a good trade-off and mirror Spring MVC where it matters the most. While the underlying ServerWebExchange will continue to offer all 3 (query params, form data, multipart data) individually, vs the Servlet API which combines query and form data into one map ("request parameters"), the @RequestParam can be a little more flexible.

The alternative, to define a dedicated @FormParam annotation would be logical, but since we already have @RequestParam and its established meaning in Spring MVC, I believe it would make things more confusing overall by adding yet another option.