[Validator] Add the Base64 constraint
#53360
Conversation
b6290b8 to
77ce42b
Compare
| $value = preg_replace('/^data:.+;base64,/', '', $value); | ||
| } | ||
|
|
||
| if (false === base64_decode($value, true)) { |
There was a problem hiding this comment.
Ensure false is strictly returned because an empty string is a valid base64 string
There was a problem hiding this comment.
This way of checking consumes memory for no good reasons because it generates the decoded payload and trashes is right away.
A regexp would be better.
Also, there are two styles of base64 encoding (url-safe and not-url-safe), so we need a way to tell which one we want.
There was a problem hiding this comment.
I updated the check to use a regex to check base64 validity.
Also, the urlEncoded option has been added. Is that what you had in mind?
77ce42b to
6d9a1c9
Compare
src/Symfony/Component/Validator/Constraints/Base64Validator.php
Outdated
Show resolved
Hide resolved
| parent::__construct($options, $groups, $payload); | ||
|
|
||
| $this->message = $message ?? $this->message; | ||
| $this->allowDataUri = $allowDataUri; |
There was a problem hiding this comment.
I'm not sure I understand how this is supposed to be useful.
Do you really have a use case where data URLs are accepted as input?
The main issue I see is the fact that the URL prefix is optional when enabling this: either you accept without the prefix, or you accept with the prefix, but both? really?
There was a problem hiding this comment.
About the use case, yes: we have use cases where a user uploads, let's say, a PNG. The input data looks like data:image/png;base64,iVBORw0KGgo.... However, we also accept, in other DTOs, "simple" base64 without data uri. I think giving this option helps the constraint being flexible for different cases devs will encounter.
That said, I agree that accepting both is useless. It is either one case or another. I'll update the code to have an option like requiresDataUri which makes more sense.
4e44d1c to
f49858b
Compare
f49858b to
9fa7059
Compare
| public string $messageInvalidString = 'The given string is not a valid Base64 encoded string.'; | ||
| public string $messageMissingDataUri = 'The given string is missing a data URI.'; | ||
|
|
||
| public function __construct(public bool $requiresDataUri = false, public bool $urlEncoded = false, string $messageInvalidString = null, string $messageMissingDataUri = null, array $groups = null, mixed $payload = null, array $options = null) |
There was a problem hiding this comment.
CPP mean parameters should be on their own lines
Also:
| public function __construct(public bool $requiresDataUri = false, public bool $urlEncoded = false, string $messageInvalidString = null, string $messageMissingDataUri = null, array $groups = null, mixed $payload = null, array $options = null) | |
| public function __construct(public bool $dataUri = false, public bool $urlSafe = false, string $message = null, string $messageMissingDataUri = null, array $groups = null, mixed $payload = null, array $options = null) |
| public string $messageInvalidString = 'The given string is not a valid Base64 encoded string.'; | ||
| public string $messageMissingDataUri = 'The given string is missing a data URI.'; |
There was a problem hiding this comment.
| public string $messageInvalidString = 'The given string is not a valid Base64 encoded string.'; | |
| public string $messageMissingDataUri = 'The given string is missing a data URI.'; | |
| public string $messageInvalidString = 'This value is not a valid Base64 encoded string.'; | |
| public string $messageMissingDataUri = 'This value is missing a data URI prefix.'; |
There was a problem hiding this comment.
BTW, the message/naming about data URI is misleading: the prefix could be there but be invalid.
| } | ||
|
|
||
| if ($constraint->urlEncoded) { | ||
| $value = rawurldecode($value); |
There was a problem hiding this comment.
Base64 URL safe encoding is not that. It's about using -_ instead of +/.
| if ($constraint->requiresDataUri) { | ||
| preg_match('/^data:.+;base64,/', $value, $matches); | ||
|
|
||
| if (0 === \count($matches)) { |
There was a problem hiding this comment.
why not use if (!preg_match)?
| } | ||
|
|
||
| preg_match('/^[a-zA-Z0-9\/\r\n+]*(==)?$/', $value, $matches); | ||
| if (0 === \count($matches)) { |
| } | ||
|
|
||
| if ($constraint->requiresDataUri) { | ||
| preg_match('/^data:.+;base64,/', $value, $matches); |
There was a problem hiding this comment.
Since this is a validator, it'd be great to also validate what is just after data:. I don't know the rules, but google or gpt will.
|
After discussing with Nicolas, we agreed this constraint doesn't make really sense. Thus, this can still be done by using a regex. For anyone wondering how to achieve this, use the |
Base64 encoding is commonly employed when handling file uploads through DTO. This constraint would really save some time to devs by ensuring that the Base64-encoded data representing the uploaded file content is correctly formatted and valid with a single attribute.
Because of the main use I see is file uploads in this format, I also added an option,
allowDataUri, which allows the input to be prefixed by something likedata:image/png;base64,.