-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Validator] Add Base64Encoded
constraint
#59666
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
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
Base64Encoded
constraint
The PR description says this is a feature of this constraint, but the implementation tells me the opposite. |
I don't think the use case for check that a property is a string encoded as base64 is common enough to be in core. What is the actual use case ? |
this looks like a duplicate of #53360 that has been rejected |
Sorry, that was incorrect description, it was written after I delete the "optional data: URI prefixes." implementation. |
I have implemented this validator in multiple projects, sometimes with additional complexity, to check whether a Base64 string represents an image. In a recent case, I needed to validate if the value sent from the client was a Base64-encoded image. However, since this is my first contribution to Symfony, I decided to start with a simpler version that strictly validates whether a given string is Base64-encoded. |
The fact that your use case involved more complex validation than what this constraint does is actually more an argument against its inclusion in core, because it means that your use case would not benefit from it but would still have to reimplement the more complex logic anyway. |
@stof |
As explained in #53360 (comment), validating only base64 is something that can be done with a simple regex (that regex is wrong regarding padding though as |
@stof |
@refatalsakka the question is how often you need to validate that a property is valid base 64 without any extra check (i.e. what this constraint does) ? |
Putting aside the question whether this feature is worth adding to the core, I'm having some issues with the implementation actually. First, if I get base64 encoded data, it's probably a binary string, an encoded file maybe. Decoding the string into memory and encoding it again looks like a heavy and memory-intense operation – especially if we only want to validate the string. Then it's the way this implementation defines validity. The reencoded string must match the original input although there are different ways of formatting a base64 encoded string. For example, base64 encoded data is often stored in text blocks with wrapped lines, typically with 76 characters per line. Think of any kind of encryption keys, eg. for SSH, PGP or TLS. If you would decode such a file and encode it again, you'd get a single line string which differs from the original input despite the input being perfectly valid. |
Given base64 data can be anything really, validating just the shape of the value (its alphabet) doesn't look like validating anything in practice to me. I think a regexp is good enough if you want this. Let me close as this raised mostly friction and has been rejected for the same reasons in the past. |
Description
This PR introduces a new
Base64Encoded
constraint to the Symfony Validator component.Features:
data:
URI prefixes.Update features description
Supports optionaldata:
URI prefixes.