Skip to content

Commit

Permalink
feature #26261 [Validator] Improvement: provide file basename for con…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
fabpot committed Oct 10, 2018
2 parents d13141f + a77abad commit 722c816
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/Symfony/Component/Validator/Constraints/FileValidator.php
Expand Up @@ -138,10 +138,12 @@ public function validate($value, Constraint $constraint)
}

$sizeInBytes = filesize($path);
$basename = $value instanceof UploadedFile ? $value->getClientOriginalName() : basename($path);

if (0 === $sizeInBytes) {
$this->context->buildViolation($constraint->disallowEmptyMessage)
->setParameter('{{ file }}', $this->formatValue($path))
->setParameter('{{ name }}', $this->formatValue($basename))
->setCode(File::EMPTY_ERROR)
->addViolation();

Expand All @@ -158,6 +160,7 @@ public function validate($value, Constraint $constraint)
->setParameter('{{ size }}', $sizeAsString)
->setParameter('{{ limit }}', $limitAsString)
->setParameter('{{ suffix }}', $suffix)
->setParameter('{{ name }}', $this->formatValue($basename))
->setCode(File::TOO_LARGE_ERROR)
->addViolation();

Expand Down Expand Up @@ -189,6 +192,7 @@ public function validate($value, Constraint $constraint)
->setParameter('{{ file }}', $this->formatValue($path))
->setParameter('{{ type }}', $this->formatValue($mime))
->setParameter('{{ types }}', $this->formatValues($mimeTypes))
->setParameter('{{ name }}', $this->formatValue($basename))
->setCode(File::INVALID_MIME_TYPE_ERROR)
->addViolation();
}
Expand Down
Expand Up @@ -177,6 +177,7 @@ public function testMaxSizeExceeded($bytesWritten, $limit, $sizeAsString, $limit
->setParameter('{{ size }}', $sizeAsString)
->setParameter('{{ suffix }}', $suffix)
->setParameter('{{ file }}', '"'.$this->path.'"')
->setParameter('{{ name }}', '"'.basename($this->path).'"')
->setCode(File::TOO_LARGE_ERROR)
->assertRaised();
}
Expand Down Expand Up @@ -279,6 +280,7 @@ public function testBinaryFormat($bytesWritten, $limit, $binaryFormat, $sizeAsSt
->setParameter('{{ size }}', $sizeAsString)
->setParameter('{{ suffix }}', $suffix)
->setParameter('{{ file }}', '"'.$this->path.'"')
->setParameter('{{ name }}', '"'.basename($this->path).'"')
->setCode(File::TOO_LARGE_ERROR)
->assertRaised();
}
Expand Down Expand Up @@ -357,6 +359,7 @@ public function testInvalidMimeType()
->setParameter('{{ type }}', '"application/pdf"')
->setParameter('{{ types }}', '"image/png", "image/jpg"')
->setParameter('{{ file }}', '"'.$this->path.'"')
->setParameter('{{ name }}', '"'.basename($this->path).'"')
->setCode(File::INVALID_MIME_TYPE_ERROR)
->assertRaised();
}
Expand Down Expand Up @@ -387,6 +390,7 @@ public function testInvalidWildcardMimeType()
->setParameter('{{ type }}', '"application/pdf"')
->setParameter('{{ types }}', '"image/*", "image/jpg"')
->setParameter('{{ file }}', '"'.$this->path.'"')
->setParameter('{{ name }}', '"'.basename($this->path).'"')
->setCode(File::INVALID_MIME_TYPE_ERROR)
->assertRaised();
}
Expand All @@ -403,6 +407,7 @@ public function testDisallowEmpty()

$this->buildViolation('myMessage')
->setParameter('{{ file }}', '"'.$this->path.'"')
->setParameter('{{ name }}', '"'.basename($this->path).'"')
->setCode(File::EMPTY_ERROR)
->assertRaised();
}
Expand Down

0 comments on commit 722c816

Please sign in to comment.