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:

• 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.

[Fleshgrinder]

I’m running into a similar issue. I have a Retrofit enabled interface that I want to use for the client and my REST controller to keep them in sync. However, I also want to have a HTML form to post to the very same endpoint.

Using @PostMapping + @ModelAttribute works for WebFlux and the HTML form but not for Retrofit. I added @RequestBody along with @ModelAttribute which works for WebFlux and Retrofit if the request contains JSON but fails for the HTML form with In a WebFlux application, form data is accessed via ServerWebExchange.getFormData()..

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.

I disagree. @RequestParam is confusing and its usage in WebFlux even more so. It would be much clearer to have @Query, @Path, @Field, @Part, … (or similar clear names). A @RequestParam can really be anything.

---

My workaround right now is to define two routes where one is just proxying to the other:

@RestController
class Example {
    @PostMapping("/post")
    fun rest(@RequestBody body: Body) =
        body

    @PostMapping("/post-form-fix")
    fun form(@ModelAttribute body: Body) =
        rest(body)
}

[Oualitsen]

New to webflux. Came from Java EE. I love webflux and I used it in so many projects. it works fine with json but I don't understand how it is hard to get a simple form value form a basic form. I just don't understand how can that be hard. By the way, I might be missing something but: when I call getFormData() it reurns an emapty map but when I call getMultipartData() it returns something very complicated to get.

[knuclechan]

Thanks for raising the issue, but I found the workaround is not implemented correctly. I've updated it a bit.

import java.lang.reflect.Field;
import java.util.List;

import org.springframework.http.codec.multipart.FormFieldPart;
import org.springframework.http.server.reactive.AbstractServerHttpRequest;
import org.springframework.stereotype.Component;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.server.ServerWebExchange;
import org.springframework.web.server.WebFilter;
import org.springframework.web.server.WebFilterChain;

import lombok.SneakyThrows;
import reactor.core.publisher.Mono;

@Component
public class PostParameterFilter implements WebFilter {

    private static /*final*/ Field QUERY_PARAMS_FIELD = null;
    static {
        try {
            QUERY_PARAMS_FIELD = AbstractServerHttpRequest.class.getDeclaredField("queryParams");
            QUERY_PARAMS_FIELD.setAccessible(true);
        } catch (NoSuchFieldException e) {
            throw new IllegalArgumentException("Could not get query params private field", e);
        }
    }

    public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
        final AbstractServerHttpRequest request = (AbstractServerHttpRequest) exchange.getRequest();
        final MultiValueMap<String, String> queryParams = new LinkedMultiValueMap<>();

        Mono<?> formMono = exchange.getFormData().map(map -> {
            //add all content from form data to query params
            if(map != null) queryParams.putAll(map);
            return map;
        });

        Mono<?> multipartMono = exchange.getMultipartData().map(map -> {
            if(map != null) {
                map.forEach((key, value) -> {
                    List<?> list = (List<?>) value;
                    list.forEach(item -> {
                        //add each form field parts to query params
                        if (item instanceof FormFieldPart) {
                            final FormFieldPart formFieldPart = (FormFieldPart) item;
                            queryParams.add(key, formFieldPart.value());
                        }
                    });
                });
            }

            return map;
        });

        return formMono.then(multipartMono)
            .flatMap((garbage) -> {
                if(queryParams.size() > 0) {
                    //add original query params to win identical name war
                    queryParams.putAll(request.getQueryParams());

                    //repair query params
                    doSetQueryParams(request, queryParams);
                }

                return chain.filter(exchange);
            });
    }

    @SneakyThrows
    private void doSetQueryParams(AbstractServerHttpRequest request, MultiValueMap<String, String> queryParams) {
        QUERY_PARAMS_FIELD.set(request, queryParams);
    }

}