(dev/core#174) Cache-keys for CRM_Utils_Cache_* should avoid reserved chars #12348
Conversation
|
(Standard links)
|
totten
force-pushed
the
master-clean-keys
branch
2 times, most recently
from
149704e to
3c0dbb7
Compare
| protected static function createCacheKey() { | ||
| $cacheKey = "CRM_OG_" . serialize(func_get_args()); | ||
| protected static function createCacheKey($id) { | ||
| $cacheKey = "CRM_OG_" . preg_replace('/[^a-zA-Z0-9]/', '', $id) . '_' . md5(serialize(func_get_args())); |
There was a problem hiding this comment.
why not use cleanKey here?
There was a problem hiding this comment.
Could do it -- it'll work. The reason I didn't was cosmetic...
Anecdotally, cleanKey(...serialize(func_get_args())...) degraded to md5 form for every example I observed. (Which makes sense -- there's a lot of inputs, and serialize format uses a lot of meta chars.) This formula provides an intelligible prefix.
| * Ex: '_abcd1234abcd1234' or 'ab_xx/cd_xxef'. | ||
| * A similar key, but suitable for use with PSR-16-compliant cache providers. | ||
| */ | ||
| public static function cleanKey($key) { |
There was a problem hiding this comment.
Can we add a test that throws a few strings at this function?
|
jenkins, test this please |
CRM/Contact/BAO/ContactType.php
Outdated
| @@ -385,8 +385,9 @@ public static function getSelectElements( | |||
| } | |||
|
|
|||
| $argString = $all ? 'CRM_CT_GSE_1' : 'CRM_CT_GSE_0'; | |||
| $argString .= $isSeparator ? '_1' : '_0'; | |||
| $argString .= $isSeparator ? '_1' : ' _0'; | |||
There was a problem hiding this comment.
why add a space here?
There was a problem hiding this comment.
Nice catch! Don't think that should be there. I'll squash it.
|
@totten this seems mergeable - and I endorse you to merge it - but there was just one line I queried above where a space was added that seemed like a possible mistake |
totten
force-pushed
the
master-clean-keys
branch
from
f46079c to
405a038
Compare
totten
force-pushed
the
master-clean-keys
branch
from
405a038 to
8b25b1f
Compare
|
Thanks, @eileenmcnaughton. I've rebased and sqaushed the mistaken character. Adding "merge on pass" based on that feedback. |
|
Merging as per the tag |
Overview
PSR-16 defines some characters as supported (
A-Za-z0-9_.), and some as reserved/forbidden ({}()/\@:), and the leaves the remainder to the implimenter.We have a few scenarios in which reserved characters are passed to an instance of
CRM_Utils_Cache_Interface. Change them.Before
CRM_Core_BAO_Cache,SettingsManager,CRM_Core_OptionGroup,CRM_Extension_Mapperpass keys toCRM_Utils_Cache_*that involve reserved characters.After
CRM_Core_BAO_Cache,SettingsManager,CRM_Core_OptionGroup,CRM_Extension_Mapperpass keys toCRM_Utils_Cache_*that don't use any reserved characters.Technical Details
CRM_Core_BAO_CacheandCRM_Core_OptionGroupare a bit more radical (using several reserved characters). Passing these through md5 makes valid keys, but they would be hard to read, so the formula is a little more complicated.