nette / routing

Nette Routing: two-ways URL conversion
https://doc.nette.org/routing
Other
231 stars 3 forks source link

Missing mechanism to parameters value comparison without referencing #8

Closed vaclavpavek closed 3 years ago

vaclavpavek commented 3 years ago

Description

In Route.php#L220 is used === to compare parameters. If used ENUMs or ValueObjects as a parameter with usage of (re)storeRequest or (de)serialization or clone, the result can never be true.

Missing mechanism to value comparison without referencing like Equalable utils.

Steps To Reproduce

$classA = new \stdClass();

$classB = clone $classA;
\Tester\Assert::equal($classA, $classB);    // true
\Tester\Assert::true($classA == $classB);   // true
\Tester\Assert::true($classA === $classB);  // false

$classC = unserialize(serialize($classA));
\Tester\Assert::equal($classA, $classC);    // true
\Tester\Assert::true($classA == $classC);   // true
\Tester\Assert::true($classA === $classC);  // false
jkuchar commented 3 years ago

Hi!

The first problem is that PHP has no standard way of defining object logical identity. As inspiration, I'm referencing our equalable utils, which implements equals() method.

It uses different strategies to determine, if objects are equals.

https://github.com/grifart/equalable-utils

This fails hard when there is parametr with fixity constant and parameter value is Value object. It will never match the route.

The second problem is that there is no way to affect how objects are serialized/deserialized in store/restoreRequest(). This can break assumptions which programmers has on their objects. It basically creates other values for enums. Or if you pass Doctrine entity, it breaks collections, which then appears to be empty. Minimal fix for this would be to explictly ban objects from Request serialization process. This would result into early failure, which is easy to debug.