-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Validator] File: add option to check extension #47710
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
Conversation
c0270d7
to
8b3fff2
Compare
Related 😄 #39063 |
My patch is a bit more advanced than #39063 and prevents real security issues.
The other PR was only checking the extension, without checking the actual content of the file. My patch can be seen as an easier way to configure the accepted MIME types and an extra check for consistency between the mime type and the extension. When using only mime-type validation, you may have issues:
With my patch, if you only allow |
My patch also prevents uploading files with misleading extensions (.e.g., a GIF named |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just some minor things. Thanks.
72ba357
to
93b8e11
Compare
Thank you @dunglas. |
This PR was squashed before being merged into the 6.2 branch. Discussion ---------- [Validator] File: add option to check extension | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | n/a | License | MIT | Doc PR | todo This patch adds an `extensions` option to the `File` constraint as an alternative to `mimeTypes` which checks the mime type of the file, its extension, and the consistency between them. I have a use case where I want to assert that: 1. the file is of a given mime type 2. the file has an extension, the extension is in the allow list, and the extension corresponds with the actual mime type of the content I added a new `extension` option to the `File` constraint to do so. Usage: ```php #[File(extensions: 'jpg')] // image.jpg is allowed, image.jpeg isn't, allowed mime types are autodetected, the content of the file is automatically checked #[File(extensions: ['xml' => ['text/xml', 'application/xml'], 'txt'])] // XML files are allowed as long as the extension is .XML, .txt files are allowed if their mime type is text (allowed mime type are auto-detected) ``` Commits ------- 1613e55 [Validator] File: add option to check extension
93b8e11
to
1613e55
Compare
… (dunglas) This PR was squashed before being merged into the 6.2 branch. Discussion ---------- Validator: fix FileValidator when value is an UploadedFile | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | n/a| License | MIT | Doc PR | n/a The code I introduced in #47710 is currently broken if the file to validate is a `UploadedFile` instance, as we must check the original extension of the file submitted by the client, not the one (that doesn't exist) of the temporary file created on the server by PHP. This patch fixes the issue. Commits ------- e24ef9d Validator: fix FileValidator when value is an UploadedFile
This patch adds an
extensions
option to theFile
constraint as an alternative tomimeTypes
which checks the mime type of the file, its extension, and the consistency between them.I have a use case where I want to assert that:
I added a new
extension
option to theFile
constraint to do so.Usage: