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

[5.0] File upload is broken when submitting a form without selecting a file #6189

Closed
Marwelln opened this issue Oct 22, 2014 · 27 comments
Closed

Comments

@Marwelln
Copy link
Contributor

After the recent changes (I did composer update a couple of minutes ago) you can't submit a form if you have enctype="multipart/form-data" and input[type="file"] and leaves the input field empty (no file selected).

Link to error message.

Fatal error: Uncaught exception 'InvalidArgumentException' with message 'An uploaded file must be an array or an instance of UploadedFile.' in /var/www/foobar/public/liferaft/file-upload-fails/vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/FileBag.php on line 59

Reproduce

1.

composer create-project laravel/laravel . dev-develop

2.

php artisan clear-compiled

3.

Set debug to true in config/app.php.

4.

Open up app/Providers/RouteServiceProvider.php and change the map method to this:

$router->group(['namespace' => 'App\Http\Controllers'], function() use ($router) {
    $router->get('/', 'HomeController@index');
    $router->put('/', 'HomeController@put');
});

5.

Add the following code to app/Http/HomeController.php:

public function put() {
    dd(\Input::all());
}

6.

Change resources/views/hello.php to this:

<!doctype html>
<html lang="en">
<body>
    <form method='post' enctype='multipart/form-data'>
        <input type='hidden' name='_method' value='put' />
        <input type='hidden' name='_token' value='<?= csrf_token(); ?>' />

        <input type='file' name='image' />
        <button>Send</button>
    </form>
</body>
</html>

If you try to submit the form without selecting a file you will get Uncaught exception 'InvalidArgumentException' with message 'An uploaded file must be an array or an instance of UploadedFile.', but if you select an image, you will get an array from Input::all().

Bad (temporary) solution

A solution is to remove an file arrays with a null value before using the SymfonyRequest::dublicate method.

Change the content of the createFromBase method from

if ($request instanceof static) return $request;

return (new static)->duplicate(

    $request->query->all(), $request->request->all(), $request->attributes->all(),

    $request->cookies->all(), $request->files->all(), $request->server->all()
);

to

if ($request instanceof static) return $request;

$files = [];
foreach ($request->files->all() as $index => $file) {
    if (null !== $file) $files[$index] = $file;
}

return (new static)->duplicate(

    $request->query->all(), $request->request->all(), $request->attributes->all(),

    $request->cookies->all(), $files, $request->server->all()
);

You can now submit the form without selecting a file.

@youanden
Copy link

Update - check out @mantasradzevicius post of @lucasmichot 's fix below. It's more holistic and isn't based on javascript or editing core files.

+1

I've been having trouble with laravel-stapler after manually migrating and setting it up. Same result if using a multipart form type.

My solution is hilarious but doesn't require editing core files if you're okay with alienating non-js users:

$(document).ready ->
  $('.profile-form').on 'submit', (e) ->
    $photoInput = $('.profile-photo-input input')
    $photoInput.remove() if $photoInput.val() == ""
    return true

Essentially, I remove the input if the value hasn't been set to anything. Super hacky but it might fit for someone in the interim.

@mikedugan
Copy link

+1

@jeffberry
Copy link

+1

Hoping for a fix soon.

@etcinit
Copy link
Contributor

etcinit commented Oct 27, 2014

I was about to create an issue for this too. I wrote a test case for it:

In HttpRequestTest.php:

 public function testInputWithEmptyFilename()
    {
        $invalidFiles = [
            'file' => [
                'name' => null,
                'type' => null,
                'tmp_name' => null,
                'error' => 4,
                'size' => 0
            ]
        ];

        $baseRequest = \Symfony\Component\HttpFoundation\Request::create('/?boom=breeze', 'GET', array('foo' => array('bar' => 'baz')), array(), $invalidFiles);

        $request = Request::createFromBase($baseRequest);
    }

@leothelocust
Copy link

+1

@jmfleming
Copy link

  • 1

@MathieuDoyon
Copy link

+1

7 similar comments
@mouhsinelonly
Copy link

+1

@davidnknight
Copy link

+1

@wlkns
Copy link

wlkns commented Nov 6, 2014

+1

@alexandre-leites
Copy link

+1

@jufy
Copy link

jufy commented Nov 10, 2014

+1

@alexleonard
Copy link

+1

@ingria
Copy link

ingria commented Nov 11, 2014

+1

@alexleonard
Copy link

Thanks @youanden for the temporary JS fix. Works a treat.

@youanden
Copy link

@alexleonard glad I could be of service.

@colinyoung87
Copy link

@youanden I used your fix, did the trick until a fix came out, thanks!

I've rewritten it a little with jquery to work across all file forms:

$('form[enctype="multipart/form-data"]').on('submit', function(e){
    $(this).find('input[type=file]').each(function(){
        var file = $(this);
        if (file.val() == "") file.remove();
    });
    return true;
});

@lucidlemon
Copy link

+1 and +1 for marking it as important

@gregoryduckworth
Copy link

+1

@mn7z
Copy link

mn7z commented Nov 13, 2014

As a temp solution how to apply this fix:

  • Make a App\Http\Request class which extends Illuminate\Http\Request (example below)
  • Apply the fix either in this issue or overriding duplicate method #6256.

Example

namespace App\Http;

use Illuminate\Http\Request as LaravelRequest;

class Request extends LaravelRequest
{

    /**
     * {@inheritdoc}
     */
    public function duplicate(
        array $query = null,
        array $request = null,
        array $attributes = null,
        array $cookies = null,
        array $files = null,
        array $server = null
    ) {
        $files = array_filter((array)$files);

        return parent::duplicate(
            $query, 
            $request, 
            $attributes, 
            $cookies, 
            $files, 
            $server
        );
    }
}
  • modify your public/index.php to capture requests with your new class:
$response = $kernel->handle(
    $request = App\Http\Request::capture()
);
$response->send();
  • When a permanent fix is released revert public/index.php and remove your Request class.

Does this work: yes
SideEffects: not known

@markvdputten
Copy link

@colinyoung87 your temproary jQuery solution works perfect, thanks for that!

@youanden
Copy link

@mantasradzevicius Thanks. I can confirm this worked after changing the namespace accordingly.

@gorkhali125
Copy link

@colinyoung87 Thanks for the solution. This worked perfectly.

@jonathanpmartins
Copy link

Perhaps it can be a Symfony problem. what do you think ?

Fatal error: Uncaught exception 'InvalidArgumentException' with message 'An uploaded file must be an array or an instance of UploadedFile.' in /var/www/html/.../vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/FileBag.php:59 Stack trace: #0 /var/www/html/.../vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/FileBag.php(73): Symfony\Component\HttpFoundation\FileBag->set('image', NULL) #1 /var/www/html/.../vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/FileBag.php(48): Symfony\Component\HttpFoundation\FileBag->add(Array) #2 /var/www/html/.../vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/FileBag.php(37): Symfony\Component\HttpFoundation\FileBag->replace(Array) #3 /var/www/html/.../vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/Request.php(448): Symfony\Component\HttpFoundation\FileBag->__construct(Array) #4 /var/www/html/.../vendor/laravel/framework/src/Illuminate/Http/Reque in /var/www/html/.../vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/FileBag.php on line 59

FileBag->set('image', NULL), receaves NULL, and that is what is dispatching a throw!

...in Symfony on the FileBag.php file, convertFileInformation method returns NULL if no files was selected on the upload.

jonathanpmartins added a commit to jonathanpmartins/symfony that referenced this issue Nov 15, 2014
...without selecting a file in Laravel 5! My tests revealed that with this tweek the problem described at this issue are resolved: laravel/framework#6189 

...in Symfony on the FileBag.php file, convertFileInformation() method returns NULL if no files was selected on the upload. In this case "FileBag->set('image', NULL)" receives NULL, and that is what is dispatching a throw!

I don't make unit tests, but this will no more break my code flow. I'm not sure if this is the real deal! If someone could look deeper into the problem, I appreciated! 

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | 
| License       | MIT
| Doc PR        |
jonathanpmartins added a commit to jonathanpmartins/symfony that referenced this issue Nov 15, 2014
...without selecting a file in Laravel 5! My tests revealed that with this tweek the problem described at this issue are resolved: laravel/framework#6189 

...in Symfony on the FileBag.php file, convertFileInformation() method returns NULL if no files was selected on the upload. In this case "FileBag->set('image', NULL)" receives NULL, and that is what is dispatching a throw!

I don't make unit tests, but this will no more break my code flow. I'm not sure if this is the real deal! If someone could look deeper into the problem, I appreciated! 

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | 
| License       | MIT
| Doc PR        |
@sfunaro
Copy link

sfunaro commented Nov 16, 2014

+1
@colinyoung87 temporary solution works perfectly!

@pomirleanu
Copy link

+1

@FranciscoCaldeira
Copy link

I have this problem using the multipartGraphQL() when testing the upload of a file! +1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.