magiclen / rocket-multipart-form-data

This crate provides a multipart parser for the Rocket framework.
MIT License
35 stars 14 forks source link

Optional field #8

Closed ThibaudDauce closed 4 years ago

ThibaudDauce commented 4 years ago

Hi,

I can't find a way to have an optional image file in my MultipartFormDataOptions.

I currently have:

let options = MultipartFormDataOptions::with_multipart_form_data_fields(vec![
        MultipartFormDataField::text("title"),
        MultipartFormDataField::text("description"),
        MultipartFormDataField::file("image")
            .content_type_by_string(Some(mime::IMAGE_STAR))
            .unwrap(),
    ]);

But it fails when no image is provided. If I remove the MultipartFormDataField::file("image"), I cannot fetch the image later when needed…

Is it possible? Did I miss something?

magiclen commented 4 years ago

What did you mean it fails when no image is provided? What kind of failures?

ThibaudDauce commented 4 years ago

When I send the form without filling the image field :

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: DataTypeError("image")', src/partners.rs:126:78
let options = MultipartFormDataOptions::with_multipart_form_data_fields(vec![
    MultipartFormDataField::text("title"),
    MultipartFormDataField::text("description"),
    MultipartFormDataField::file("image")
        .content_type_by_string(Some(mime::IMAGE_STAR))
       .unwrap(),
]);

// this is the line partners.rs:126 
let multipart = MultipartFormData::parse(request.content_type().unwrap(), data, options).unwrap(); 

When I fill the field with an image, there is no problem, all the code works.

If I comment the MultipartFormDataField:

let options = MultipartFormDataOptions::with_multipart_form_data_fields(vec![
    MultipartFormDataField::text("title"),
    MultipartFormDataField::text("description"),
    // MultipartFormDataField::file("image")
    //     .content_type_by_string(Some(mime::IMAGE_STAR))
    //     .unwrap(),
]);

… later when I try to get the image with multipart_form_data.files.remove("image"), it always returns None even when I fill the image field.

magiclen commented 4 years ago

The DataTypeError occurred because you put some data which however was not an image into the image field in your HTTP message body.

ThibaudDauce commented 4 years ago

My image field is a simple <input type="file" name="image">, how can something else got into the image field? Is my browser sending an empty string for an empty input type="file?

magiclen commented 4 years ago

That's weird. If you don't put anything in your input element (named image), the image field shouldn't be included in the POST request.

magiclen commented 4 years ago

Did you run my example? Try not to select any image and click the upload button.

ThibaudDauce commented 4 years ago

I didn't try to run your example, but my form is really simple, it's just some inputs and I send without touching the unique input type="file". Here are my data thanks to data.stream_to(&mut io::stdout());

-----------------------------16332065432802205852057743242
Content-Disposition: form-data; name="title"

Super asso
-----------------------------16332065432802205852057743242
Content-Disposition: form-data; name="image"; filename=""
Content-Type: application/octet-stream

-----------------------------16332065432802205852057743242
Content-Disposition: form-data; name="description"

<p><strong>Description 2</strong></p>
-----------------------------16332065432802205852057743242--

There is an empty image… It's really weird…

ThibaudDauce commented 4 years ago

I also tried with Chromium, same result: an empty image is sent.

------WebKitFormBoundaryzMuAAPjDe1I32QBr
Content-Disposition: form-data; name="title"

Super asso
------WebKitFormBoundaryzMuAAPjDe1I32QBr
Content-Disposition: form-data; name="image"; filename=""
Content-Type: application/octet-stream

------WebKitFormBoundaryzMuAAPjDe1I32QBr
Content-Disposition: form-data; name="description"

<p><strong>Description 2</strong></p>
------WebKitFormBoundaryzMuAAPjDe1I32QBr--
ThibaudDauce commented 4 years ago

Here is my form if you want to check it:

<form method="post" action="{{ uri!(crate::partners::update: self.partner.id) }}" enctype="multipart/form-data">
    <div class="flex flex-col mb-4 max-w-lg">
        <label class="mx-4 mb-1" for="title">Titre</label>
        <input type="text" name="title" id="title" class="py-2 text-lg px-4 border focus:border-black" value="{{ partner.title }}">
    </div>
    <div class="flex flex-col mb-4 max-w-lg">
        <label class="mx-4 mb-1" for="image">Changer l'image</label>
        <input type="file" name="image" id="image" class="py-2 text-lg px-4 border focus:border-black">
    </div>
    <div class="flex flex-col mb-4">
        <label class="mx-4 mb-1" for="editor">Description</label>
        <textarea name="description" id="editor">{{ partner.description|safe }}</textarea>
    </div>
    <button class="w-full border text-lg px-4 py-2 mt-4">Modifier</button>
</form>
magiclen commented 4 years ago

It seems like the empty file input will be included in the request body... Now I know that. I will check it later and find a way to make this crate do better.

ThibaudDauce commented 4 years ago

Thanks. Seems to be a common problem https://github.com/playframework/playframework/issues/6203

I see two options:

I you want I can PR the first version, the second one will be harder for me…

magiclen commented 4 years ago

I want to check both of the filename and Content-Type and also check if entry.data is empty, but I need to test more web browsers to confirm they behave the same way.

Only checking the filename is not a good approach. After all, not every file upload request is from HTML forms on web browsers.

magiclen commented 4 years ago

After thinking this through, I believe it just needs to check the filename and entry.data. I cannot find in what situation we need to send an empty file with an empty filename.

ThibaudDauce commented 4 years ago

I just tested it on my project, it works perfectly! Thanks a lot for the fast fix!