Warp: Multipart should Stream File Upload

[cetra3]

Having a look through the multipart filter source, it looks like it loads the entire file into memory:

https://github.com/seanmonstar/warp/blob/5c269562a823c5340f3dfc14bdd11af592c03dea/src/filters/multipart.rs#L112-L116

Maybe returning a Stream of Bytes would be more appropriate than a Vec<u8>, especially for larger files. I can submit a PR to make the change for you if there is interest in changing this.

[seanmonstar]

Yea, as part of the initial implementation, the easiest was to just buffer the full contents. I certainly want to provide a way to prevent that. The tricky part that I punted on was coming up with a decent API.

[cetra3]

Ok, so I have had a look at the current multipart code & there are a couple of issues:

• The multipart crate is not async as far as I can tell, and read_entry is blocking when reading in fields. If you are reading in 1GB of data this is an issue.

• Personal taste but setting a limit of 2MB for a multipart upload as a default is quite "magical". Other libraries I have used that set these sorts of limits by default cause a lot of pain in production. I would advocate no limit unless set

[surban]

Is there a workaround we could currently use for streaming file uploads?

[cetra3]

@surban I have been working on this slightly with a branch in another lib called mpart-async.

I have added a warp example which is still not as ergonomic as I'd like, but it is sort of where I am heading with this:

async fn mpart(
    mime: Mime,
    body: impl Stream<Item = Result<impl Buf, warp::Error>> + Unpin,
) -> Result<impl warp::Reply, Infallible> {
    let boundary = mime.get_param("boundary").map(|v| v.to_string()).unwrap();

    let mut stream = MpartStream::new(boundary, body.map_ok(|mut buf| buf.to_bytes()));

    while let Ok(Some(mut field)) = stream.try_next().await {
        println!("Field received:{}", field.name().unwrap());
        if let Ok(filename) = field.filename() {
            println!("Field filename:{}", filename);
        }

        while let Ok(Some(bytes)) = field.try_next().await {
            println!("Bytes received:{}", bytes.len());
        }
    }

    Ok(format!("Thanks!\n"))
}

You can test with curl:

curl -F test=field -F [email protected] http://localhost:3030/

[frederikbosch]

This works wonderful, any plans on merging the branch? :)

[cetra3]

Hey @frederikbosch if you meant to my mpart-async library? I have just merged/published version 0.4.0 which has the warp example.

I would like to know if there is an easier way to make this work with warp and will be experimenting a little bit to get that to work.

[frederikbosch]

Wonderful, thanks! I wish I could help with the experiment, but with Rust I am merely a consumer of open-source code, I lack the quality to provide. Hopefully I can improve so I could offer help in the future.

[pdolezal]

Is multipart/mixed supposed to work as well?

I suggested the feature in #466, which was closed recently, and I was asked to raise the suggestion rather here:

It would be nice to have multipart/mixed supported.

Perhaps it could be possible to support multiple multipart subtypes at once while leveraging their common roots for the implementation and providing a unified API (I see here a relationship to #323). Moreover multipart/mixed parser could be used as the default parser for any yet unsupported or unknown multipart types, as the RFC suggests.

The use cases for multipart/mixed content type, that I'm concerned with, are related to various RESTful services that handle composite requests from non-browser clients or perform some bulk processing where the response may consist of multiple parts, possibly with different content types. Interoperability with such services is yet another concern.

[frederikbosch]

From what I can see in the source, it is not dependent on the content type. See the tests here.

[cetra3]

If it's the same structure as a normal multipart request I don't see why it can't work. As far as I can tell the multipart/mixed just changes the semantics of what the request is (i.e, multiple parts referring to the same thing). If there are any special changes to the parsing, then may be best to submit a PR.

[pdolezal]

I filed the original issue because warp contains a filter for multipart/form-data, but other multipart types are not supported in a similar way. It would be nice to have a filter for any multipart type, maybe with some nice extensions for specific multipart types.

All multipart types have a common root in RFC 2046 (where most of them are defined), while multipart/form-data is defined in RFC 7578. Despite multipart/form-data inherits the structure from the common root, the RFC specifies refinements that do not apply to the other multipart types.

I didn't study mpart-async in depth, just glanced at it, so correct me if I'm wrong, but it seems to me that it is not general enough – for instance, it can't handle arbitrary headers in the parts or assumes that there are some specific fields, like _name_.

[cetra3]

Yes, it can handle arbitrary headers and while it doesn't assume that name is present, if you try and read it it will throw an error that it's not there, but will still parse parts fine.