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.3] Fix fatal error on file uploads #15350

Merged
merged 1 commit into from
Sep 9, 2016
Merged

[5.3] Fix fatal error on file uploads #15350

merged 1 commit into from
Sep 9, 2016

Conversation

nhowell
Copy link
Contributor

@nhowell nhowell commented Sep 8, 2016

This PR fixes the code from #15250 which broke file uploads in Laravel 5.3.7.

Catchable fatal error: Object of class Symfony\Component\HttpFoundation\File\UploadedFile could not be converted to boolean in vendor\laravel\framework\src\Illuminate\Http\Request.php on line 883

You can't convert SplFileInfo, and therefore Symfony's UploadedFile, to a boolean. See: http://stackoverflow.com/questions/17488674/why-cant-splfileinfo-be-converted-to-boolean

Edit: I should add that it looks like this has been fixed at some point after PHP 5.6.4, but as 5.6.4 is the minimum version required for Laravel 5.3 I think this PR is still needed.

PR #15250 broke file uploads in Laravel 5.3.7.
@nhowell nhowell changed the title Fix fatal error on file uploads [5.3] Fix fatal error on file uploads Sep 8, 2016
@themsaid
Copy link
Member

themsaid commented Sep 8, 2016

@nhowell What version of PHP are you using? The issue you refer to is fixed since PHP 5.5.21 and laravel 5.3 requires PHP >=5.6.4.

@nhowell
Copy link
Contributor Author

nhowell commented Sep 8, 2016

I'm using PHP 5.6.4 (for development purposes).

@themsaid
Copy link
Member

themsaid commented Sep 8, 2016

Strange, can you please add a test to make sure it won't break in the future?

@nhowell
Copy link
Contributor Author

nhowell commented Sep 8, 2016

The issue you refer to is fixed since PHP 5.5.21 and laravel 5.3 requires PHP >=5.6.4.

PHP 5.5.21 and PHP 5.6.5 were both released the same day, so it appears to me that PHP 5.6.4 did not contain the fix. It isn't clear to me from the PHP changelog when the fix landed in PHP 5.6, though.

@themsaid
Copy link
Member

themsaid commented Sep 8, 2016

This test would do, please add it in tests/Http/HttpRequestTest.php to make sure things would work in the future.

public function testInputWithValidFile()
    {
        $files = [
            'file' => new Symfony\Component\HttpFoundation\File\UploadedFile(__FILE__, 'foo.php'),
        ];

        $baseRequest = SymfonyRequest::create('/?boom=breeze', 'GET', ['foo' => ['bar' => 'baz']], [], $files);

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

        $this->assertInstanceOf(\Symfony\Component\HttpFoundation\File\UploadedFile::class, $request->files->get('file'));
    }

@nhowell
Copy link
Contributor Author

nhowell commented Sep 8, 2016

can you please add a test to make sure it won't break in the future?

How would one do that? The existing tests should cover it, right? But Travis CI is running the tests with PHP 5.6.5, and those tests pass, so the bug must've been fixed with PHP 5.6.5.

@taylorotwell taylorotwell merged commit eb6e0fd into laravel:5.3 Sep 9, 2016
@nhowell nhowell deleted the patch-2 branch September 9, 2016 13:21
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 this pull request may close these issues.

None yet

3 participants