Affects: 5.3.4
When I create a MultiValueMap
with a MultipartBodyBuilder
, the filename
associated to the FilePart
I use is not set in the ContentDisposition
. This was the case in the version Spring Frameworks 5.3.3 and previous.
The commit including this modification is this one: https://github.com/spring-projects/spring-framework/commit/e537844a093d313595e20cc440506fe8b04bc7db by @poutsma.
To make it work, I have to call the filename
method on the PartBuilder
manually, like this:
val body = MultipartBodyBuilder().apply {
part("file", filePart).filename(filePart.filename())
}
.build()
The main problem caused is webClient is not treating data provided as a file due to the lack of filename
in the ContentDisposition
, especially in the case PartGenerator
case, where the filename
is used as criteria to distinguish formField
or file
: https://github.com/spring-projects/spring-framework/blob/dfb7ca733ad309b35040e0027fb7a2f10f3a196a/spring-web/src/main/java/org/springframework/http/codec/multipart/PartGenerator.java#L140
I could provide a PR to add this micro-modification, but I would like this to be validated before "implementation", because I can't decide if this behaviour modification have been made on purpose or if this is just a bug.
Comment From: poutsma
The idea behind e537844 was that because we now copy all the headers—including the Content-Disposition
header that contains the filename, we no longer have to copy the filename explicitly. As long as the Part
returns the correct headers, it should work.
Can you please show me the code used that triggers this problem, by providing a complete minimal sample (something that we can unzip or git clone, build, and deploy) that reproduces the problem?
Comment From: davinkevin
This is the code I use to test my upload
endpoint.
https://gitlab.com/davinkevin/Podcast-Server/-/blob/65642d8c35203436a409f9df9100819d0afe197f/backend/src/test/kotlin/com/github/davinkevin/podcastserver/item/ItemHandlerTest.kt#L1471
The same thing should happen if I use the WebClient
for upload purpose from my main code.
The idea behind e537844 was that because we now copy all the headers—including the Content-Disposition header that contains the filename, we no longer have to copy the filename explicitly. As long as the Part returns the correct headers, it should work.
Yeah, but this imply developer has already defined the correct ContentDisposition
. In the previous version, the content disposition was directly defined by the https://github.com/spring-projects/spring-framework/commit/e537844a093d313595e20cc440506fe8b04bc7db#diff-e3b48cd5ce815887e17c572c046b0b246591c55899f8dc12fe4a8376e74c8ff9L136 which was simpler, because automatically made following Part
type.
Comment From: poutsma
After team discussion, we decided that will not reintroduce the former behaviour that copied the filename from the FilePart
. This is because of the overhead of HttpHeader.setContentDispositionFormData
—which creates a new ContentDisposition.Builder
and ContentDisposition
for each call, combined with our opinion that implementations of FilePart
should return the correct headers, which already includes the filename.
Comment From: davinkevin
Ok for me, but, could you provide the "official" way for us to provide a FilePart
to a WebClient
/MultipartBodyBuilder
?
Comment From: poutsma
You can add a file part like so (in java)
Flux<DataBuffer> content = ...
MultipartBodyBuilder bodyBuilder = new MultipartBodyBuilder();
bodyBuilder.part("file", content)
.filename("orig")
.header("foo", "bar); // use builder to further customise part
MultiValueMap<String, HttpEntity<?>> parts = bodyBuilder.build();
Comment From: davinkevin
Thanks 👍. This is exactly what I have done in Kotlin. Thanks for the answer!