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

[6.x] Allow a symfony file instance in validate dimensions #30009

Merged
merged 1 commit into from Sep 16, 2019

Conversation

@tobiasthaden
Copy link
Contributor

commented Sep 16, 2019

The Problem

When a user validates the dimensions of a Symfony\Component\HttpFoundation\File\File via the Illuminate/Validation/Concerns/ValidatesAttributes trait the request throws an error within validateDimensions because the getClientMimeType() method does not exist on the instance.

Only UploadedFile has the currently used getClientMimeType() method.

The Solution

This PR fixes that issue by using the getMimeType()method that exists on both valid file instances.

Additional

According to Symfony documentation:

The client mime type is extracted from the request from which the file
was uploaded, so it should not be considered as a safe value.

For a trusted mime type, use getMimeType() instead (which guesses the mime
type based on the file content).

What does this break?

The mime type will not be extracted from the request but guessed from the files content. So the uploaded files must be valid. In case of a .svg the file must contain the opening <xml> tag otherwise it will be interpreted as plain/text and the validation will fail.

I think that is not a breaking change, because uploaded files should – in the past also – always be valid.


Resolves #29962

@GrahamCampbell GrahamCampbell changed the title [6.x] allow a symfony file instance in validate dimensions [6.x] Allow a symfony file instance in validate dimensions Sep 16, 2019
@taylorotwell taylorotwell merged commit a43014f into laravel:6.x Sep 16, 2019
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.