Support for multipart form extractors

[jacob-pro]

If you have a look on crates.io there are a proliferation of crates for doing multipart form uploads with files in actix.

Including:

• actix-easy-multipart by me
• actix-multipart-extract by @JSH32
• actix-multipart-derive by @robjtede
• actix-web-multipart-file by @KeenS
• actix_extract_multipart by @kosekidev
• awmp by @kardeiz
• actix-form-data by @asonix

I think each of these have some unique improvements and features that the others do not. It seems silly to have so many unofficial crates, and is not very friendly to people who just want to quickly get started with actix.

I am wondering if we should have an effort to consolidate all of these crates into one official one in the actix project?

[asonix]

Hey there! As someone mentioned here, I have a requirement for any consolidation work on this front. I haven't looked at each of these options so I can't speak for what features they do or don't have, but I think it is important to be able to support streaming files from a multipart upload into various upstreams (files, object storage, etc). Just aggregating a file upload into a Bytes or a Vec isn't flexible enough, especially if uploaded files are potentially large

That's the primary motivation behind my library, which I use in my image host API pict-rs
(example: https://git.asonix.dog/asonix/pict-rs/src/branch/main/src/main.rs#L814-L833)

[jacob-pro]

Yep I think all some of these support at least writing files. In my implementation stuff gets written to a NamedTempFile which you can then choose to persist.

What I might do a bit later when I have time is write up a matrix of all the different features these crates have, so we can get an idea of what an official crate would need to include.

[asonix]

I also think that maybe moving to just an Extractor would be a better idea than my current api, which is a combination Middleware + Extractor. Maybe I'll play with updating my library to work more like that

edit (more details):
Currently, I support multiple different "Form" middlewares that you can register for different routes. To make this work as a more extractor-driven API, I would need to change the Form from a middleware to extension data, then move the stream processing logic to a FromRequest that accesses that extension. This would allow adding a single form as top-level data if you only need one upload endpoint for simplicity, but different scopes would still need different forms to handle multiple endpoints (nothing a couple example programs couldn't demonstrate)

edit 2 (even more details):
An alternative to registering different forms at different scopes would be to allow tagging a form implementation with some additional data (e.g. a unit struct) and then referencing that same unit struct in the extractor. This would simplify scoping forms to routes, but would increase the complexity of the extractor type

[JSH32]

Mine uses serde and does not support streaming. It was mostly just a proof of concept but I've been locally working on something similar to a builder but with a procmacro to auto parse into the struct which is what I think would be best.

[asonix]

@JSH32 fwiw the proc-macro "extract into this struct" method is really nice, and ideally that would be the final API. Maybe if there was an annotation you could put on a field like #[stream_with(handler)] to allow custom stream processing while still storing the result in the struct

[JSH32]

@asonix You could allow both Vec<u8> for getting just the bytes or something which is an AsyncRead (or something like that) to put the stream into the struct.

[jacob-pro]

@JSH32 yeah I was impressed with your serde compatible macro - it is a lot better than my one!

[asonix]

@JSH32 that doesn't quite work, because you need to process each field in order. If you embed a file stream directly into a struct for processing later, you don't read the Multipart stream any farther and can't "deserialize" fields that come afterwards.

[JSH32]

@JSH32 that doesn't quite work, because you need to process each field in order. If you embed a file stream directly into a struct for processing later, you don't read the Multipart stream any farther and can't "deserialize" fields that come afterwards.

The problem is that a lot of the time the processor for the field will need to have context of the rest of the data, and reading the pure request is kind of unintuitive. Maybe save the original stream as a temporary file and provide a StreamExt for reading the bytes from temporary?

[asonix]

Saving to temporary files solves the problem for a number of use-cases, but prevents creating a pure "file proxy" implementation, which could stream files in-line to some other service. It requires either using tmpfs (which means holding files in memory) or using a filesystem, which adds requirements for deploying the service

I like the ToStream trait, but maybe it could look something like this:

trait FromStream {
    type Error: ResponseError;
    type Future: Future<Output = Result<Self, Self::Error>>;
    
    // implementations must ensure they consume the entire stream if they complete successfully,
    // otherwise the multipart stream will be stuck and no further fields can be processed (leading to a deadlock)
    fn from_stream(req: HttpRequest, stream: actix_multipart::Field /* or maybe some other opaque stream type */) -> Self::Future;
}

struct StreamMetadata {
    object_storage_id: String,
}

impl FromStream for StreamMetadata {
    type Error = actix_web::Error;
    type Future = Pin<Box<dyn Future<Output = Result<Self, Self::Error>>>>;
    
    fn from_stream(req: HttpRequest, stream: Field) -> Self::Future {
        // extract useful things from HttpRequest here
        // ...
        
        Box::pin(async move {
            // upload to minio or s3 or something here
            // ...
            
            Ok(StreamMetadata { object_storage_id: /* blah */ })
        })
    }
}

#[derive(Multipart)]
struct MyForm {
    file_field: StreamMetadata
}

[JSH32]

That definitely works. Still kind of meh though because you cant get the full metadata of a route for conditionally uploading based on something like a user and will need to do processing of your form data outside of the route but I'm pretty sure its the only way to do it right.

[asonix]

This FromStream trait could be implemented for PathBuf (or maybe a custom TemporaryPathbuf) that would stream the files onto a disk and return their path in the deserialized struct

[jacob-pro]

Thanks @asonix @JSH32

I've had a think about it and I think these are all the features we will need:

What we need to do with streams of bytes:

1. Store the bytes in memory, e.g.

   pub struct BytePart {
       pub bytes: Vec<u8>,
       pub filename: Option<String>,
       pub mime: Mime,
   }

2. Temporary files on disk (that can later be persisted)

   pub struct FilePart {
       /// The file data itself stored as a temporary file on disk.
       pub file: NamedTempFile,
       /// The size in bytes of the file.
       pub size: usize,
       /// The `filename` value in the `Content-Disposition` header.
       pub filename: Option<String>,
       /// The Content-Type specified as reported in the uploaded form.
       pub mime: mime::Mime,
   }

3. An arbitrary user implemented future that streams the bytes elsewhere (e.g. cloud object storage)

   pub struct ObjectPart {
       /// The file has been uploaded to an external service
       pub object_id: String,
   }

As suggested by @asonix the solution could be to require each field to implement a trait, something like:

pub trait HandlePart {
   // Providing the request context allows pulling from app_data() to read config etc.
   fn handler(req: HttpRequest, stream: Field) -> Future<Output = Result<Self>>;
}

struct Form {
    bytes_in_memory: BytePart,
    file_on_disk: FilePart,
    file_in_s3: ObjectPart
}

What about simple text fields?

We could provide a handler for T: FromStr which would cover all of these?

We could provide a handler for T: Deserialize, and use it with the serde_plain crate which would convert text/plain into primitive rust types and basic enums.

Edit: We can provide another part implementation:

/// Deserializes the part in memory as plain text (using serde_plain)
#[derive(Deref)]
struct TextPart<T: DeserializeOwned>(T);

This would allow deserializing a plain text field into a simple type, e.g. strings, ints, floats, and plain enums.

It also does not restrict us to a single deserialization format, because we could introduce additional implementations such as:

/// Deserializes the part in memory as JSON
#[derive(Deref)]
struct JsonPart<T: DeserializeOwned>(T);

How should we handle Lists and Optionals?

We would need to provide our own containers that implement the handler trait. (We can't implement directly on Vec/Option
because it would conflict with the generic Deserialize implementation).

According to the spec:

To match widely deployed implementations, multiple files MUST be sent
by supplying each file in a separate part but all with the same
"name" parameter.

Using something like compile_type_eq (thanks @JSH32) the macro could detect the use of Vec and Option wrapped fields:

struct Form {
    files: Vec<S3Upload>,
    maybe: Option<String>,
}

A Vec can be used when multiple parts arrive with the same "name". An an option can be used to represent an optional part that may or may not be in the form.

Unknown and Extra fields

Serde allows using #[serde(deny_unknown_fields)] to prevent surplus fields - we should probably have something similar?

A further question is what to do if multiple fields are uploaded with the same name, but we are not expecting a Vector?

My crate has the #[from_multipart(deny_extra_parts)] which errors if:

• A form is uploaded with an unknown field name
• A form is uploaded with a duplicate field name (and the type is not a Vector)

But these two cases should really be separately configurable.

The duplicate field case should actually have 3 actions - keep first, keep last or error?

Renaming fields

We need to be able to support something similar to #[serde(rename = "name")] for cases where we can't use the field-name
e.g. rust keywords or different casing styles.

Data limits

We need to be able to set a global limit on the amount of data read into memory, for basic DDoS protection,
just as the official actix extractors support.
It would have to be up to the implementor of the handler to choose whether to respect / consume from the in-memory limits.

But we will also want to provide field level limits as @JSH32 has done, which will be useful for files etc.

pub struct ExampleForm {
    #[multipart(max_size = 5MB)]
    file_field: File,
}

Let me know if you can think of anything I have missed?

[jacob-pro]

@robjtede Please can you advise if this sort of extractor would be accepted into acix-extras and what would be an appropriate name for it?

Or actually maybe it should go into the actix-multipart crate itself - which would clear up a lot of the naming confusion?

If so, I am happy to write a lot of the code for this - but would just like to know where to get started from an organizational standpoint? Maybe we can have a WIP/feature branch in the actix repository which we can open PRs against?

[JSH32]

The handler will already be generated by a derive macro for introspecting field names/options so Vec/Option can just be done there and conditionally code can be generated for that.