Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Multipart Forms #106

Open
SergioBenitez opened this issue Jan 4, 2017 · 36 comments

Comments

Projects
None yet
@SergioBenitez
Copy link
Owner

commented Jan 4, 2017

Rocket currently doesn't provide built-in support for multipart forms, but it should!

@SergioBenitez SergioBenitez added this to the 0.3.0 milestone Jan 4, 2017

@flosse

This comment has been minimized.

Copy link

commented Jan 4, 2017

Do you know a quick workaround to support it?

@flosse

This comment has been minimized.

Copy link

commented Jan 4, 2017

I tried to use mime_multipart::read_multipart like this:
read_multipart(&mut data.open(),true)
but if fails :(

@SergioBenitez

This comment has been minimized.

Copy link
Owner Author

commented Jan 4, 2017

You should be able to adapt the multipart crate pretty easily here. I can't vouch for it, however, as I haven't tried it myself.

@flosse

This comment has been minimized.

Copy link

commented Jan 4, 2017

Thanks for that fast reply!

@flosse flosse referenced this issue Jan 4, 2017

Closed

support rocket #57

@SergioBenitez

This comment has been minimized.

Copy link
Owner Author

commented Jan 4, 2017

Sure! Grabbing lunch now, but can whip something up afterward!

@Zeludon

This comment has been minimized.

Copy link

commented Jan 17, 2017

How was the lunch, more importantly how is multipart integration coming?

@SergioBenitez

This comment has been minimized.

Copy link
Owner Author

commented Jan 18, 2017

@Zeludon Lunch was great, thanks! 😋

The simplest route towards multipart support is via an existing crate. The most complete of those available is multipart. Unfortunately its API is rather restrictive at the moment, and so it wouldn't be a good fit for Rocket. Nonetheless, I've proposed a change to the API to the author/maintainer at abonander/multipart#58 that would allow its integration with Rocket.

@Maaarcocr

This comment has been minimized.

Copy link

commented Feb 3, 2017

I'm working on a small utility website where I need to upload a csv file from a form. Has there been any progress for this?

Thanks for this great library, I'm in love with it!

@abonander

This comment has been minimized.

Copy link

commented Mar 9, 2017

@SergioBenitez If there's any more blockers, let me know.

@shepmaster

This comment has been minimized.

Copy link

commented Apr 15, 2017

I answered a related question on Stack Overflow. Note that there are limitations of this approach.

#![feature(plugin)]
#![plugin(rocket_codegen)]

extern crate rocket;
extern crate multipart;

#[post("/", data = "<upload>")]
fn index(upload: DummyMultipart) -> String {
    format!("I read this: {:?}", upload)
}

#[derive(Debug)]
struct DummyMultipart {
    alpha: String,
    one: i32,
    file: Vec<u8>,
}

use std::io::{Cursor, Read};
use rocket::{Request, Data, Outcome};
use rocket::data::{self, FromData};
use multipart::server::Multipart;

impl FromData for DummyMultipart {
    type Error = ();

    fn from_data(request: &Request, data: Data) -> data::Outcome<Self, Self::Error> {
        // All of these errors should be reported
        let ct = request.headers().get_one("Content-Type").expect("no content-type");
        let idx = ct.find("boundary=").expect("no boundary");
        let boundary = &ct[(idx + "boundary=".len())..];

        let mut d = Vec::new();
        data.stream_to(&mut d).expect("Unable to read");

        let mut mp = Multipart::with_body(Cursor::new(d), boundary);

        // Custom implementation parts

        let mut alpha = None;
        let mut one = None;
        let mut file = None;

        mp.foreach_entry(|mut entry| {
            match entry.name.as_str() {
                "alpha" => {
                    let t = entry.data.as_text().expect("not text");
                    alpha = Some(t.into());
                },
                "one" => {
                    let t = entry.data.as_text().expect("not text");
                    let n = t.parse().expect("not number");
                    one = Some(n);
                },
                "file" => {
                    let mut d = Vec::new();
                    let f = entry.data.as_file().expect("not file");
                    f.read_to_end(&mut d).expect("cant read");
                    file = Some(d);
                },
                other => panic!("No known key {}", other),
            }
        }).expect("Unable to iterate");

        let v = DummyMultipart {
            alpha: alpha.expect("alpha not set"),
            one: one.expect("one not set"),
            file: file.expect("file not set"),
        };

        // End custom

        Outcome::Success(v)
    }
}

fn main() {
    rocket::ignite().mount("/", routes![index]).launch();
}
$ curl -X POST -F alpha=omega -F one=2 -F file=@hello http://localhost:8000/
I read this: DummyMultipart { alpha: "omega", one: 2, file: [104, 101, 108, 108, 111, 44, 32, 119, 111, 114, 108, 100, 33, 10] }
@SergioBenitez

This comment has been minimized.

Copy link
Owner Author

commented Apr 16, 2017

@shepmaster Thanks for the snippet! One particular concern with your code is that you read all of the form data into memory with no limit. This means that, for instance, if a user uploads a 500MB file, the entire file will reside in memory even though it could have been streamed to disk. Or, worse, if a user sends you data infinitely, the request will exhaust the memory resources on the server, likely crashing it.

@abonander

This comment has been minimized.

Copy link

commented Apr 16, 2017

What's the status on native integration? Not much else I can do on my end, AFAIK.

@shepmaster

This comment has been minimized.

Copy link

commented Apr 16, 2017

@SergioBenitez absolutely, and thank you for pointing that out. However, many people don't need to worry about that level security for their own application for whatever reason. A solution in the hand is worth two in the bush. 😺

@SergioBenitez

This comment has been minimized.

Copy link
Owner Author

commented Apr 19, 2017

@abonander My primary concern is correctness. Given that each time (three so far) I've tried to use your library I've run into showstoppers regarding correctness, I'm not particularly confident that it will function as required. I'd like to see abonander/multipart#59 closed. Parsing and networking bugs can be some of the most insidious, and they can lead to extreme confusion when they pop up. I'd really, really like to avoid this.

@abonander

This comment has been minimized.

Copy link

commented Apr 19, 2017

I tried fuzzing and it found several more bugs; last I tried it, I believe it ran for several hours without issue. Should I let it run overnight? For a week? A month? Should I have it run on CI somehow? I haven't seen any good rule of thumb here.

I've added a number of edge cases to the integration test. No new issues have cropped up, unless there's some you haven't told me about.

@SergioBenitez

This comment has been minimized.

Copy link
Owner Author

commented Apr 25, 2017

@abonander Are you testing potential networking issues somehow? Also, do you have an idea of how the performance of the library compares to some ideal?

Unfortunately, I don't think we're ready to add multipart support directly to Rocket just yet. As such, I'm moving this to 0.4. If no breaking changes are required, we might see this as a point release in 0.3.

@SergioBenitez SergioBenitez modified the milestones: 0.4.0, 0.3.0 Apr 25, 2017

@abonander

This comment has been minimized.

Copy link

commented Apr 25, 2017

Are you testing potential networking issues somehow?

What kinds of issues that aren't covered by randomized read sizes in the integration test, or fuzzing? Any I/O errors are propagated upwards.

Also, do you have an idea of how the performance of the library compares to some ideal?

What would you consider an ideal?

@gbip

This comment has been minimized.

Copy link

commented Aug 4, 2017

Is it planned to land soon in rocket ? I am currently lacking this feature for a project... 😄
I mean every web application needing to do file upload from a form will need to support enctype=multipart/form-data, so I think this is quite an important feature !
At the moment I'm going with the solution posted by @shepmaster (thanks by the way).

@semirix

This comment has been minimized.

Copy link

commented Nov 16, 2017

To anyone interested in another workaround, I devised this jQuery function to convert file uploads into base64, avoiding the need for multipart forms. The base64 string is added to a hidden element that replaces the old file input. It should work on IE10 and above but I've only tested in chrome and firefox. This will require upping the limits on all forms. This would be a reasonably viable workaround if you could set size limits per form.

$.fn.base64Upload = function () {
    return this.each(function () {
        var self = this;
        
        if (window.File && window.FileReader && window.FileList && window.Blob) {
            var inputName = $(self).attr("name");
            var encoded = $("<input>")
                .attr("type", "hidden")
                .attr("name", inputName)
                .insertAfter($(self));
    
            $(self).change(function (event) {
                var file = event.target.files[0];
                if (file) {
                    var reader = new FileReader();
                    reader.onload = function(readerEvt) {
                        var binaryString = readerEvt.target.result;
                        encoded.val(btoa(binaryString));
                    };
                    reader.readAsBinaryString(file);
                }
            });
            $(self).val("");
            $(self).removeAttr("name");
        } else {
            console.error("Cannot use base64 upload, File APIs are not supported in this browser!");
        }
    
        return self;
    });
};

Call like this:

$("input[type=file]").base64Upload();
@TheNeikos

This comment has been minimized.

Copy link

commented Dec 21, 2017

We're closing in on the 1 year anniversary! Is there a roadmap or list of things that need to be done for this to work? Is it just the multipart crate?

@abonander

This comment has been minimized.

Copy link

commented Dec 21, 2017

There is also the formdata crate if multipart is still unsatisfactory. I never got a reply to my last comment so I don't know what the blockers are. I've fixed a number of bugs in multipart since the previous discussion and I'm soon to land some significant API improvements (on master but not yet published).

@tcr-ableton

This comment has been minimized.

Copy link

commented Jan 31, 2018

Any news on that front? :-)

@abonander

This comment has been minimized.

Copy link

commented Jan 31, 2018

The core crate builds, I've been slowly fixing the examples. I remember that Rocket wanted to use multipart behind the scenes so I don't know if I should spend time building integration on my end.

@tcr-ableton

This comment has been minimized.

Copy link

commented Jan 31, 2018

Thanks @abonander for your quick response :-)

Do you have an example of your own on how to use the multipart crate with rocket? I couldn't find any on the multipart crate repository... Can I consider the one previously posted in this discussion as the reference?

Cheers!

@abonander

This comment has been minimized.

Copy link

commented Jan 31, 2018

Yes, mostly, but I wouldn't use data.stream_to(&mut vec), I would use data.open() and call Multipart::with_body(data_stream, boundary). There's no need to read it to memory first.

With the new API (0.14), there is no explicit distinction between files and text, just an .is_text() method to tell you when it's probably safe to read the field to a string if you like. This gives the user control over whether or not they want to read large text fields into memory.

@tcr-ableton

This comment has been minimized.

Copy link

commented Feb 1, 2018

Thanks a lot for your help @abonander
Greatly appreciated!

@flosse

This comment has been minimized.

Copy link

commented Feb 12, 2018

Any news here?

@abonander

This comment has been minimized.

Copy link

commented Feb 13, 2018

I've released multipart 0.14.0 which has some bugfixes and API improvements. You can't use the previously posted example verbatim so I guess I should add one for Rocket to multipart. I guess I can also straight-up add integration with Rocket on my end if it's desired, though I know Rocket wanted to handle multipart/form-data behind the scenes.

@abonander

This comment has been minimized.

Copy link

commented Feb 13, 2018

I've thrown together a quick example using Multipart's .save() API since it makes things simpler: https://github.com/abonander/multipart/blob/master/examples/rocket.rs

This does save larger fields (>10kB) to a temporary directory; SaveBuilder (from .save()) has documentation explaining how to tune its behavior.

@expenses

This comment has been minimized.

Copy link

commented Feb 13, 2018

@abonander Thanks for doing the work on your end! @SergioBenitez Can we get this integrated some time soon please?

@kardeiz

This comment has been minimized.

Copy link

commented Feb 13, 2018

A word of warning: if you don't buffer the data stream and if multipart doesn't consume all of the data, you will get the following error from Rocket: Data left unread. Force closing network stream.

This happened to me yesterday with multipart 0.14.0. This new multipart has a size_limit applied by default (not sure what the exact value is); it was rejecting one of my large file uploads, causing the data to not be consumed, causing Rocket to error.

@abonander

This comment has been minimized.

Copy link

commented Feb 13, 2018

size_limit is currently 8 MiB, it's mostly arbitrary but I wanted to be conservative with it. You can adjust it with .size_limit() which accepts u64 or Option<u64>.

I wouldn't consider it an error not to consume all the data in the network stream; with multipart/form-data anything after the closing boundary is supposed to be ignored. You can always resume the save process after an error by using the existing Entries you have from SaveResult::Partial and passing it to SaveBuilder::with_entries().

@jendakol

This comment has been minimized.

Copy link

commented May 4, 2018

@abonander I highly appreciated your example how could the Multipart be used with the Rocket. Even though I used it in my project in slightly different way the idea is still there. Anyone interested in using those two together, feel free to check my solution: https://github.com/jendakol/rbackup/blob/2ff8ed0cc6a734feab503de2d656ecd45287aafe/src/lib.rs#L114

@SergioBenitez

This comment has been minimized.

Copy link
Owner Author

commented Jul 23, 2018

Unfortunately, we won't be able to get built-in support for this in for 0.4: pushing to 0.5.

@SergioBenitez SergioBenitez modified the milestones: 0.4.0, 0.5.0 Jul 23, 2018

@liufuyang

This comment has been minimized.

Copy link

commented Aug 3, 2018

Looking forward to this as well...

@ocschwar

This comment has been minimized.

Copy link

commented Sep 30, 2018

Also looking forward to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.