Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Validator] Improvement: provide file basename for constr. violation messages in FileValidator.#26261
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Validator] Improvement: provide file basename for constr. violation messages in FileValidator.#26261
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas left a comment
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.
Thanks for this proposal
That's a new feature so it should target master.
Can you please add some tests also?
| } | ||
| $sizeInBytes =filesize($path); | ||
| $basename = ($valueinstanceof UploadedFile) ?$value->getClientOriginalName() :basename($path); |
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.
brackets should be remove
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.
Removed as requested - I use brackets for readability purposes, no side effects
TheCelavi commentedFeb 22, 2018
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? |
…messages in FileValidator.
5892e0e toa77abadComparefabpot commentedOct 10, 2018
Thank you@TheCelavi. |
…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.
\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.