Align @RequestPart support in WebFlux with Spring MVC

[andrej-urvantsev]

Affects: 5.1.6

---

After upgrading Spring Boot from 2.0.2 to 2.1.4 we started to see an issue with multipart form data uploading:

@PostMapping(value = "/{id}", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
public Mono<Void> store(@PathVariable final String id,
        @RequestPart("metadata") final Map<String, String> metadata,
        @RequestPart("fieldOne") final List<String> fieldOne,
        @RequestPart("fieldTwo") final List<Double> fieldTwo)

That worked on spring boot 2.0.2, but on 2.1.4 it fails with decoding exception:

Cannot deserialize instance of `java.lang.Double` out of START_ARRAY token

So it actually tries to deserialize list of doubles as just a Double.

As a workaround we've changed method signature to use Double[] instead of List. After that it works as expected.

[bclozel]

Could you provide a sample application we could take a look at? It's hard to figure out where the problem is without a way to reproduce the issue.

[andrej-urvantsev]

@bclozel here you go
https://github.com/lazystone/spring-bugs/tree/22973

It works on SB 2.0.9, but if you change SB version to 2.1.x here
https://github.com/lazystone/spring-bugs/blob/22973/build.gradle.kts#L6

Then test will fail.

[andrej-urvantsev]

@bclozel can I help somehow more with the issue?

[bclozel]

Hi @lazystone , sorry about the late feedback.
Before Spring Framework 5.1, we would support that case but not binding multiple parts with the same name as a list.
With #21162 (and 5cee607), we're now supporting that case. This means that binding a list like this:

@PostMapping(value = "/{callId}", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
public Mono<Void> store(@RequestPart("people") final List<Person> people) {

is expecting multiple parts like so:

var bodyBuilder = new MultipartBodyBuilder();
bodyBuilder.part("people", new Person("Jane"), MediaType.APPLICATION_JSON);
bodyBuilder.part("people", new Person("John"), MediaType.APPLICATION_JSON);

In your sample, the List of String is being bound, but not by the codec you'd expect. The String array is not parsed as JSON, but rather by the StringDecoder (so you won't get the expected result here). Your sample is failing because we're trying to deserialize [0.0, 2.0, 4.0, 7.0] as a single Double by Jackson.

If you wish to bind a single part as a collection, then I think you should use the following:

@PostMapping(value = "/{callId}", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
public Mono<Void> store(
        @PathVariable final String callId,
        @RequestPart("metadata") final Map<String, String> metadata,
        @RequestPart("fieldOne") final String[] fieldOne,
        @RequestPart("fieldTwo") final Double[] fieldTwo) {

I think we can use this issue to improve the documentation and underline that List is supported for several parts with the same name.

[andrej-urvantsev]

Oookaay, yeah - definitely worth to document this :) Better with some examples.

[rstoyanchev]

This looks more like a regression to me. The sample doesn't even have multiple parts with the same name, and yet the resolver is trying to pass a List of all parts with that name. As a result there is no option to convert to a List, and while array does provide an option, it is a problem in its own right that arrays and lists aren't treated consistently.

Taking a step back and comparing to Spring MVC where @RequestPart("myPart") gets the first "myPart" and converts it to whatever any target type. If there are multiple parts with the same name, the only choice is to declare Collection (or an array) parameterized with MultipartFile or Part, i.e. there is no conversion for multiple parts with the same name, so that's a limitation indeed but for the less common case, while the rest is simple and consistent all around for lists and arrays.

In WebFlux, the support for Flux in combination with @RequestPart has been there from the start but it isn't very meaningful because @RequestPart implies map-like access (i.e. not streaming). It's more like a List, and support for that was added later in 5.1 which extends how it already behaved but List, unlike Flux, is also a target for conversion to a single value. So while it is possible in WebFlux to deserialize multiple parts with the same name, it's a benefit for a less common case while causing issues for the more common case.

I think we should correct this and align with Spring MVC even if the behavior has been there since the beginning of 5.1. It would be a breaking change if trying to convert multipart parts with the same name to a List but as it is the current behavior is too problematic to be ignored and I don't think documentation helps that much.

[rstoyanchev]

I think we need to address this but I've scheduled it for 5.3 since it will require breaking behavior, mainly with regards to decoding to List<T>. Below is the expected impact:

Parameter Type	Description / Current	After Change
Part	First part with given name	same
List<Part>	All parts with given name	same
Collection<Part>	All parts with given name (not supported)	new
Part[]	All parts with given name (not supported)	new
<T>	First part decoded	same
List<T>	All parts with given name	First part decoded to List
T[]	First part decoded to array	same
Flux<T>	All parts with given name	same