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] validateDimensions() handle image/svg MIME #30204

Merged
merged 2 commits into from
Oct 8, 2019
Merged

[6.x] validateDimensions() handle image/svg MIME #30204

merged 2 commits into from
Oct 8, 2019

Conversation

0xb4lint
Copy link
Contributor

@0xb4lint 0xb4lint commented Oct 7, 2019

Added image/svg MIME support to validateDimensions() function.

During the image dimensions validation there is a getMimeType() function called on a Illuminate\Http\UploadedFile class (inherited from Symfony\Component\HttpFoundation\File\UploadedFile <- Symfony\Component\HttpFoundation\File\File) class.

It utilizes the Symfony\Component\Mime\MimeTypes which calls the guessMimeType() on the Symfony\Component\Mime\FileinfoMimeTypeGuesser class.

At this point finfo class is used:

if (!$finfo = new \finfo(FILEINFO_MIME_TYPE, $this->magicFile)) {
	return null;
}

return $finfo->file($path);

I've found it can return two different values for SVG files: image/svg+xml and image/svg.

I didn't find any documentation about this behavior (only a magic test: https://github.com/php/php-src/blob/master/ext/fileinfo/tests/magic), but it can be easily reproduced via this Gist:
https://gist.github.com/0xB4LINT/0d90ddaa02c91147635667b2a8b90f9b

The only difference between the source files is the lack of <?xml ... ?> row.

By the W3's SVG standard document there are more valid document structures defined:
<?xml ... ?><svg ... or <svg xmlns="...

Extends #30009: replaced getClientMimeType() with getMimeType(), but doesn't handle the image/svg MIME.

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.

2 participants