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
Comment From: 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.
Comment From: 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.
Comment From: andrej-urvantsev
@bclozel can I help somehow more with the issue?
Comment From: 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 5cee607f2872f), 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.
Comment From: andrej-urvantsev
Oookaay, yeah - definitely worth to document this :) Better with some examples.
Comment From: 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.
Comment From: 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 |