Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Validator] prevent hash collisions caused by reused object hashes#38387
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xabbuh commentedOct 2, 2020
| Q | A |
|---|---|
| Branch? | 3.4 |
| Bug fix? | yes |
| New feature? | no |
| Deprecations? | no |
| Tickets | Fix#36415 |
| License | MIT |
| Doc PR |
xabbuh commentedOct 2, 2020
ping@fancyweb |
| returnspl_object_hash($object); | ||
| } | ||
| privatefunctioncleanUpObjectRefs() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
shouldn't calls to thiscleanUpObjectRefs happen in afinally block instead, to be sure we also record the end of the callstack when some called code triggers an exception ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In fact keeping the object references in the validator instance does not work as expected as new validator instances can be created in one validation run. I decided to move the reference handling to the execution context instead which also saves us from worrying about have to clean up the stored references instead.
907ddf6 to752d2a4Compare
fancyweb left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As discussed on Slack, we would need a test that assert that using the same constraint instance on several paths on the sameRecursiveContextualValidator instance (by usingatPath()) works correctly.
15b77b1 to85f359cCompare85f359c to9c81f2aCompare89c48ea to099886fCompare099886f to8dd1a6eComparexabbuh commentedNov 13, 2020
Thank you@fancyweb. |