[#28013] - md5 does not return unique hash value for two objects
- Closed
- 22 Feb 2020
- Medium
- Build: staging
- # 28013
Steps to reproduce the issue
- Add the line of code below under this line https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Form/Field/UsergrouplistField.php#L76
var_dump($hash);- Access to Users -> Manage -> Options. On that page, there are two form fields type usergrouplist, defined here https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_users/config.xml and https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_users/config.xml#L19
Expected result
Two different values of the $hash variable because it's two separate fields
Actual result
For some reasons, the $hash has same values for two fields, thus, the options is cached (options for the second field is always same with options for first field. In my local installation, it has same value d41d8cd98f00b204e9800998ecf8427e
System information (as much as possible)
PHP Version 7.3.11
Additional comments
Maybe use a different function like https://php.net/spl_object_hash or https://www.php.net/manual/en/function.spl-object-id.php
joomla-cms-bot
- | Labels |
Added:
?
|
||
spl_object_hash() isn’t reliable for a cache value, that assumes the object will have the exact same hash consistently across requests.
@mbabker From PHP manual
This function returns a unique identifier for the object. This id can be used as a hash key for storing objects, or for identifying an object, as long as the object is not destroyed. Once the object is destroyed, its hash may be reused for other objects.
So as I understand, on a page load, each object will have it own unique identifier and it's safe to use for our purpose. Or you are saying that the object ($this->element in our case) could be destroyed before the page is rendered, thus cause potential issue?
Wait, my bad, I thought this was a persistent cache at first.
I’d still be careful about using the SPL methods though as essentially you turn the cache into one that’s different based on the current instance of that field class.
Truthfully, that cache has some bad logic. The cache needs to account for the checksuperuser attribute and whether the current user is a super user and not just the element (whatever that actually is). Because THAT has more impact on whether the data should be different than whether the XML has a different structure.
(Basically the current cache logic is flawed in assuming that the exact same SimpleXMLElement will be repeatedly given to that field class without accounting for the fact that attributes and current user state must create a separate cache; why does this field even need a in-memory cache anyway?)
Actually, I think that cache is not needed there, and should be removed.
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-02-22 17:13:04 |
| Closed_By | ⇒ | joomdonation |
this did the expected result
$hash = spl_object_hash($this->element);