[Validator] Improvement: provide file basename for constr. violation messages in FileValidator. #26261
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicolas-grekas
requested changes
Feb 22, 2018
| @@ -138,10 +138,12 @@ public function validate($value, Constraint $constraint) | |||
| } | |||
|
|
|||
| $sizeInBytes = filesize($path); | |||
| $basename = ($value instanceof UploadedFile) ? $value->getClientOriginalName() : basename($path); | |||
There was a problem hiding this comment.
brackets should be remove
There was a problem hiding this comment.
Removed as requested - I use brackets for readability purposes, no side effects
Do you want me to create PR on master branch?
I have updated tests to reflect this change, so, it is kinda covered already. Only thin that can be tested is getting the basename of file - do you want me to write that test? |
fabpot
approved these changes
Oct 10, 2018
…messages in FileValidator.
fabpot
force-pushed
the
feature/expose-filename-in-file-validator
branch
from
5892e0e to
a77abad
Compare
|
Thank you @TheCelavi. |
fabpot
added a commit
that referenced
this pull request
Oct 10, 2018
…str. violation messages in FileValidator. (TheCelavi) This PR was squashed before being merged into the 4.2-dev branch (closes #26261). Discussion ---------- [Validator] Improvement: provide file basename for constr. violation messages in FileValidator. | Q | A | ------------- | --- | Branch? | 3.4 <!-- see below --> | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | no | License | MIT | Doc PR | N/A `\Symfony\Component\Validator\Constraints\FileValidator` provides absolute path to file on server when user, per example, uploads empty file, too large file, of wrong mime type, etc... Absolute path to file on server does not have value to the end user, on top of that, exposing it can be a security issue - end user should not be aware of server filesystem. Basename of file, however, has value (per example: MyAwesomeSheet.xlsx, MyCV.doc, etc..) - if something is wrong with file upload (size, mime, etc...). If basename is exposed, we can construct messages like: "Your file 'MyCV.doc' is not allowed for upload due to....whatever"... This PR provides basename of file so end user of `\Symfony\Component\Validator\Constraints\FileValidator` can construct error messages of higher value for end user. Commits ------- a77abad [Validator] Improvement: provide file basename for constr. violation messages in FileValidator.
This was referenced Nov 3, 2018
Closed
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
\Symfony\Component\Validator\Constraints\FileValidatorprovides absolute path to file on server when user, per example, uploads empty file, too large file, of wrong mime type, etc...Absolute path to file on server does not have value to the end user, on top of that, exposing it can be a security issue - end user should not be aware of server filesystem.
Basename of file, however, has value (per example: MyAwesomeSheet.xlsx, MyCV.doc, etc..) - if something is wrong with file upload (size, mime, etc...).
If basename is exposed, we can construct messages like: "Your file 'MyCV.doc' is not allowed for upload due to....whatever"...
This PR provides basename of file so end user of
\Symfony\Component\Validator\Constraints\FileValidatorcan construct error messages of higher value for end user.