Add support for multipart/form-data requests

[Ogeon]

Integrate https://github.com/cybergeek94/multipart to make it possible to read requests of the type multipart/form-data through BodyReader. My hope is that this integration should be as seamless as possible.

[abonander]

I'm guessing you're going to want this first: https://github.com/cybergeek94/multipart/issues/5, that way you can include it with

[dependencies.multipart]
version = "*"
default-features = false
features = ["server"]

and only compile the parts you need.

You can see how I implemented it for Hyper's request object in src/server/hyper.rs, or I can do it if you like.

There might still be some bugginess with the request reader implementation, I'm working on it with another guy right now.

[Ogeon]

That would, of course, be nice to have, but it's no requirement if it works anyway. I haven't really had much time to investigate it and think about how it should be implemented, but you are still welcome to give it a try, if you want.

[abonander]

Feature flags added: https://github.com/cybergeek94/multipart/issues/10

[Ogeon]

Nice, thank you! I'll take a look at how to implement this as soon as I get some time over.

[abonander]

Actually, I made it quite simple. All you have to do is implement the server::HttpRequest trait:

/// A server-side HTTP request that may or may not be multipart.
pub trait HttpRequest: Read {
    /// Return `true` if this request is a `multipart/form-data` request, `false` otherwise.
    fn is_multipart(&self) -> bool;
    /// Get the boundary string of this request if it is `multipart/form-data`.
    fn boundary(&self) -> Option<&str>;
}

It doesn't look like you have a single type that is Read (BodyReader) and has the information to fulfill the API contract of HttpRequest (Context). This trait was designed with Hyper's server::request::Request object in mind, which contains the request headers as well as a Read impl for reading out the body. I believe Iron's request object is structured similarly.

Now, obviously, it would be preferable if neither of us had to break our existing APIs to make this work. However, I'm okay with breaking my API to accommodate you since you have a larger userbase; the few people that multipart gets directly broken for will probably understand. (Though, with some clever design, most end-user code shouldn't be affected.)

I'm thinking something like the following:

/// A server-side HTTP request that may or may not be multipart.
pub trait HttpRequest {
    type Body: Read;
    /// Return `true` if this request is a `multipart/form-data` request, `false` otherwise.
    fn is_multipart(&self) -> bool;
    /// Get the boundary string of this request if it is `multipart/form-data`.
    fn boundary(&self) -> Option<&str>;
    /// Get the body reader from this HTTP request.
    fn to_body(self) -> Self::Body;
}

However, I'm wondering if I should rework my API to be less greedy when it comes to owning the request object. I really only ever need &mut <request>, even with the Hyper APIs. Taking ownership is only necessary in the Hyper client API, which shouldn't even be concerned with the server-side.

[Ogeon]

I had a quick look and it seems like I would be able to bundle the reader and the content type in a minimal request and implement HttpRequest for it. I can then rewrite BodyReader as an enum and do some clever error handling to make it all play together.

An other alternative is to be as lazy as possible and do the multipart check within the BodyReader I can give it the content type and create the Multipart when it's needed.

In any case, I don't think you will have to change your API. At least not for this.

[abonander]

Why not add a method to Context that returns an adaptor implementing Read + HttpRequest? It could even come already wrapped in Multipart if you don't mind coupling to that API.

impl<'a, 'b, 's> Context<'a, 'b, 's> {
    /// Take this context as a multipart request if it is one.
    fn as_multipart<'m>(&'m mut self) -> Option<Multipart<MultipartContext<'a>>> { ... }
}

/// These can all be the same lifetime since they're guaranteed to be shorter than any of the ones on `Context`.
pub struct MultipartContext<'a> {
    headers: &'a Headers,
    reader: &'a mut BodyReader<'a, 'a>,
}

impl<'a> Read for MultipartContext<'a> { ... }

impl<'a> HttpRequest for MultipartContext<'a> { ... }

[Ogeon]

That would be a quite good solution, but it should be possible to destructure the Context, so we can't expect the headers and reader to always be accessible. I would also like the body reading methods to be collected in the same place.

[abonander]

Hmm, maybe a wrapper that the user constructs manually with the Headers and BodyReader structs. A macro could make this easier, if macros are allowed to move out of structs without consuming them. I think macro hygiene rules would disallow this.

[Ogeon]

Macros has to obey same rules as we do, so no magic...

[Ogeon]

My current plan is to extract the boundary string and give it to the BodyReader. Making one clone of it shouldn't be too expensive, I think. I would the add an as_multipart method to it.

[Ogeon]

I made a PR (#50) with a preliminary implementation of my idea.