HTTP multipart/form-data is broken in multiple IPFS implementation

[MichaelMure]

I tried to parse a multipart/form-data HTTP request produced by go-ipfs-api with golang's standard http package and it completely failed. After going down the rabbit hole, turns out that there is multiple issues, shared by multiple IPFS implementation:

1. In the HTTP Content-Disposition header, with go-ipfs-api , parameters are separated with a : instead of a ;
2. In the body part header, Content-Disposition of type file is invalid; it should be form-data (see https://tools.ietf.org/html/rfc7578#section-4.2)
3. In the body part header, Content-Disposition is missing the mandatory name parameter (again, see https://tools.ietf.org/html/rfc7578#section-4.2)

On the go side, I opened ipfs/go-ipfs-api#167 to address 1. , and ipfs/go-ipfs-files#8 to address 2. and 3. .

On the js side, I opened ipfs-inactive/js-ipfs-http-client#948 to address 2. and 3. . I don't know if 1. is an issue here.

I'm opening this issue because the problem seems to be more widespread than just those PR (see https://github.com/search?p=1&q=org%3Aipfs+Content-Disposition&type=Code as an example).

Another problem I faced but that is not covered by a RFC: when adding content that is not a file (say ipfs add with a data buffer), the body part header Content-Disposition doesn't have a proper filename parameter. While this is legal according to the RFCs, the consequence is that those part are parsed as form values (think text field entered by a user), and not files.

With the go standard library, one key difference is that a form file will be offloaded on disk if too big but a value is not. This means that the memory usage in this case will commonly be high. I'd like to suggest to always have a valid filename parameter to force the parsing of those potentially huge part as file, and to use an extra parameter to distinguish from an actual file and a buffer for proper handling in a daemon.

[MichaelMure]

Small correction: in the case of the golang http package, a maximum of (10MB+maxMemory) is allowed to read form values, so anything over that will error instead of allocating more memory as I stated. Default value for maxMemory is 32MB.

[MichaelMure]

Tagging some relevant individual: @magik6k @lgierth @daviddias @Stebalien

[Stebalien]

Thanks for reporting all this! Status:

• Fix Content-Disposition HTTP header go-ipfs-api#167: merged
• multifilereader: fix multipart/form-data go-ipfs-files#8: alternative at fix the content disposition header go-ipfs-files#14
• Empty filenames for stdin: relates to coreapi unixfs: change Wrap logic to make more sense  kubo#6019 (comment). Basically, we should be setting this on the client.

[Stebalien]

@daviddias This was a cross-implementation issue filed against both go and js.

[Stebalien]

Also, looking into this, all parts appear to have been fixed.