clj-commons / aleph

Asynchronous streaming communication for Clojure - web server, web client, and raw TCP/UDP
http://aleph.io
MIT License
2.54k stars 241 forks source link

The client ignores content type when using byte arrays in multipart data #649

Closed p-himik closed 1 year ago

p-himik commented 1 year ago

clj-http uses org.apache.http.entity.mime.content.ByteArrayBody for byte arrays and it allows setting content type.

Netty uses attributes and file uploads. The former don't allow setting content type, the latter do. The issue with Aleph is that it uses file uploads only for on-disk files, even though Netty provides io.netty.handler.codec.http.multipart.MemoryFileUpload.

Seems like the solution here is to change aleph.http.multipart/encode-request in such a way so that:

arnaudgeiser commented 1 year ago

Hello Eugene,

Thank you for the feedback and the issue. I might be able to have a look during the week-end. Whether you can't wait, do not hesitate to open a pull request.

KingMob commented 1 year ago

@arnaudgeiser and @DerGuteMoritz are probably more familiar with this part of the code.

Looking at encode-request, it seems we ignore :mime-type if the part isn't a File, but that hardly seems fair. It seems like we should be able to manually add a Content-Type header for those parts if supplied.

After looking at the Netty MemoryFileUpload class's behavior, there are a few more consequences to saying a part is a file than just getting a Content-Type header. It's possibly ok, but needs a closer look. We'd have to assign a file name, and it'll add Content-Disposition headers (as well as other headers) which the other end may treat differently from the existing behavior.

Continuing to use an Attribute and manually adding the Content-type header might be all we need, if we can do that. However, I was looking, and I don't see any way to add extra headers to Attribute parts in Netty, so MemoryFileUpload (or MixedFileUpload if we want to set an upper limit on how much memory to consume) may be our only options.

arnaudgeiser commented 1 year ago

An issue is open regarding this on Netty (Eugene added a comment already) [1]. I also had a look and using MemoryFileUpload might be the correct approach. I spent a little bit of time on VertX (found nothing), Micronaut [2] and Reactor Netty [3].

[1] : https://github.com/netty/netty/issues/6800 [2] : https://github.com/micronaut-projects/micronaut-core/blob/1d8a98ba11ec0d6d280812b9a433f2e23ba3dcca/http-client-core/src/main/java/io/micronaut/http/client/multipart/InputStreamPart.java [3] : https://github.com/reactor/reactor-netty/blob/0296f6e03669c7337b7e9cd341461ee071d8e922/reactor-netty-http/src/main/java/reactor/netty/http/client/HttpClientFormEncoder.java#L292