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

[Validator] Dump `ConstraintViolation::$invalidValue` at `ConstraintViolation::__toString()` #29389

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@phansys
Contributor

phansys commented Nov 30, 2018

Q A
Branch master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

@phansys phansys force-pushed the phansys:constraint_invalid_value branch 2 times, most recently from 715a406 to af3e467 Nov 30, 2018

@nicolas-grekas nicolas-grekas added this to the next milestone Nov 30, 2018

@phansys phansys force-pushed the phansys:constraint_invalid_value branch 3 times, most recently from 49e4a1c to 9615000 Nov 30, 2018

@nicolas-grekas nicolas-grekas added Feature and removed BC Break labels Dec 1, 2018

} elseif (null === $this->invalidValue || \is_bool($this->invalidValue)) {
$context .= var_export($this->invalidValue, true);
} else {
$context .= (string) $this->invalidValue;

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Dec 1, 2018

Member

when a string, should we really reflect its value? seems dangerous to me (like what when this string is 1GB long - or when it contains sensitive data, or is a hack payload, etc?)

This comment has been minimized.

@phansys

phansys Dec 1, 2018

Contributor

I understand your concern, since this method currently just dumps values which are under the validator's responsibilities. Could we have at least an opt-in argument to choose if the invalid value should be dumped? My use case is based on the validation of a entity which has many constraints in their properties. So, in order to simplify the logging of failed validations, I'm logging something like this:

implode("\n", iterator_to_array($violations));

Then, when get a violation in my log I know by instance, if I should update the validator to support an unhandled valid value (a false positive).
I know I could achieve this behavior by traversing the violations and calling getInvalidValue() by my own risk; but I thought it could be a nice addition for this method.

@phansys phansys force-pushed the phansys:constraint_invalid_value branch from 9615000 to 9d8f3c4 Dec 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment