Forcing multipart overrides Content-Type header change has been reverted as part of a merge commit.
Closed this issue · 8 comments
Describe the bug
- forceMultiPart method in HttpRequestMultiPart class was taking care of removing Content-Type header if already present in the header list.
MultipartBody forceMultiPart(boolean value) {
this.forceMulti = value;
this.headers.remove("Content-Type");
return this;
}
Code link: d152ee9
- This fix has been removed as part of a merge commit. Looks like by mistake.
dd532cf - Now for a mult-part request, Content-Type header is present twice, causing the request fail to recognise as a multipart request.
Expected behavior
- We need to get the fix back into the main branch .
Environmental Data:
- Java Version [17]
- Unirest Version [4.4.0]
@ryber Could you please analyse and help with the fixed build?
can you give me an example of what behavior you are actually seeing and what you expect to happen? Maybe with an example?
@ryber Please consider the below example.
HttpRequestWithBody request = Unirest.post("https://<host>.com/upload");
request.header("Content-Type", "application/json");
String response = request
.multiPartContent()
.field("file", new File(<file_path>))
.asString().getBody();
This code is sending two entries of the "Content-Type" header to the server. One added as default and the other created for multiPartContent() with boundary details.
content-type: application/json
content-type: multipart/form-data; boundary=d130eaef-6067-4c99-b38a-aa19f47d77e5;charset=UTF-8.
.
The server reads the first one and raises the below exception
org.springframework.web.multipart.MultipartException: Current request is not a multipart request
.
.
You had added the fix in the previous versions of unirest (See version 3.13.13), but somehow the code is missing in the latest build
You can see your change here. You can find out how this fix is missing in the latest code.
github.com/Kong/unirest-java/commit/d152ee9441da2eabc8806c353b9f5c0fa2bc50f6
ah, I don't think it was reverted exactly, 4 was on a different branch for a while. Things needed to get patched across both branches. I'll get it moved. But I would recommend against setting headers twice. The fact it even does this feels weird and magical and I hate magic.
Thanks @ryber for the commit! Could you help publish the build as well?
I was also on the same opinion, but my code is structured such that it has a wrapper on Unirest which sets the default content type as "application/json".
Whenever there needs a different type, we use headerReplace() to replace the default type.
.
In the multiPartContent() case, we can not manually replace or set the type as it has to add the boundary details as well later.
I'm currently unable to deploy because Maven changed a bunch of stuff and I think I need to get someone from Kong to do something for me. I don't work for them, and I haven't even spoken to anyone there in years
got it out in 4.4.4
Thanks @ryber! Helpful.