[Validator][RecursiveContextualValidator] Prevent validated hash collisions #36415
Conversation
a59494a to
f8cea76
Compare
| if (\is_object($group)) { | ||
| $groupHash = spl_object_hash($group); | ||
|
|
||
| if (!isset($this->validatedObjectsReferences[$groupHash])) { |
There was a problem hiding this comment.
Why is this needed for group sequences?
There was a problem hiding this comment.
GroupSequences can suffer from the same problem since they can be passed directly to the validate method as objects.
|
We should find a place where these arrays can be emptied. The memory leak should be avoided. |
I think we can clear constraints and group sequences arrays at the end of each validate methods. However, we can never clean the initialized objects. Would that work for you? |
f8cea76 to
146634d
Compare
src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php
Show resolved
Hide resolved
|
@fancyweb What's the status of this PR? |
|
I need to check Nicolas's suggestion, please keep it open. |
146634d to
6f28e2d
Compare
6f28e2d to
1ec0d3a
Compare
Unfortunately that does not work. I had to remove this part. |
|
Can you update the |
|
@xabbuh I looked at this a few days ago but it needs more work. My patch does not fix all cases because it does not take into account the current defaultPropertyPath. One of FormValidator tests fails because of this. You can have a look if you want to. |
…t hashes (fancyweb, xabbuh) This PR was merged into the 4.4 branch. Discussion ---------- [Validator] prevent hash collisions caused by reused object hashes | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #36415 | License | MIT | Doc PR | Commits ------- 8dd1a6e prevent hash collisions caused by reused object hashes 9645fa3 [Validator][RecursiveContextualValidator] Prevent validated hash collisions
RecursiveContextualValidatorusesspl_object_hashat different places to generate cache keys. The problem with this method is that there can be collisions (same hash for two different objects) since once an object is destroyed,spl_object_hashcan return a previously used hash.I encountered the problem on a project with constraints that are created on the fly. The first constraint is applied and is "cached". Then it is destroyed. Then another constraint happen to have the exact same
spl_object_hash. It is skipped while it would cause violations. I was able to reproduce the problem consistently in the unit test.The easy way to deal with this is to keep references to all objects (proposed solution). But I guess it could leak too much.
We could also try to "clean" the
ExecutionContextstores at the end of the three validate methods. However, we would still need to keep references to the initialized objects since they must really be initialized one time, even on multiple calls