From 0fcad3574693440a6109f55a417f8df172b186b8 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 30 Mar 2017 20:24:20 -0700 Subject: [PATCH] CRM-19813 - CRM_Utils_Hook - Don't allow conflicts in property names Traditionally, the names of the hook parameters were advisory/aesthetic. With the introduction of GenericHookEvent, they become first class symbols -- and they can theoretically conflict with properties of `GenericHookEvent` or `Event`. In particular, the parameter `$name` conflicts with `Event::$name` in Symfony 2.x. --- CRM/Utils/Hook.php | 42 ++++++++++++++-------------- Civi/Core/Event/GenericHookEvent.php | 30 ++++++++++++++++++++ 2 files changed, 51 insertions(+), 21 deletions(-) diff --git a/CRM/Utils/Hook.php b/CRM/Utils/Hook.php index 15b846168d..5124809af6 100644 --- a/CRM/Utils/Hook.php +++ b/CRM/Utils/Hook.php @@ -1011,7 +1011,7 @@ abstract class CRM_Utils_Hook { * Your query must return two columns: * the contact 'data' to display in the autocomplete dropdown (usually contact.sort_name - aliased as 'data') * the contact IDs - * @param string $name + * @param string $queryText * The name string to execute the query against (this is the value being typed in by the user). * @param string $context * The context in which this ajax call is being made (for example: 'customfield', 'caseview'). @@ -1021,8 +1021,8 @@ abstract class CRM_Utils_Hook { * * @return mixed */ - public static function contactListQuery(&$query, $name, $context, $id) { - return self::singleton()->invoke(array('query', 'name', 'context', 'id'), $query, $name, $context, $id, + public static function contactListQuery(&$query, $queryText, $context, $id) { + return self::singleton()->invoke(array('query', 'queryText', 'context', 'id'), $query, $queryText, $context, $id, self::$_nullObject, self::$_nullObject, 'civicrm_contactListQuery' ); @@ -1171,13 +1171,13 @@ abstract class CRM_Utils_Hook { * * @param array $options * Associated array of option values / id - * @param string $name + * @param string $groupName * Option group name * * @return mixed */ - public static function optionValues(&$options, $name) { - return self::singleton()->invoke(array('options', 'name'), $options, $name, + public static function optionValues(&$options, $groupName) { + return self::singleton()->invoke(array('options', 'groupName'), $options, $groupName, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, 'civicrm_optionValues' ); @@ -1841,55 +1841,55 @@ abstract class CRM_Utils_Hook { /** * This hook is called while preparing a profile form. * - * @param string $name + * @param string $profileName * @return mixed */ - public static function buildProfile($name) { - return self::singleton()->invoke(array('name'), $name, self::$_nullObject, self::$_nullObject, self::$_nullObject, + public static function buildProfile($profileName) { + return self::singleton()->invoke(array('profileName'), $profileName, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, 'civicrm_buildProfile'); } /** * This hook is called while validating a profile form submission. * - * @param string $name + * @param string $profileName * @return mixed */ - public static function validateProfile($name) { - return self::singleton()->invoke(array('name'), $name, self::$_nullObject, self::$_nullObject, self::$_nullObject, + public static function validateProfile($profileName) { + return self::singleton()->invoke(array('profileName'), $profileName, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, 'civicrm_validateProfile'); } /** * This hook is called processing a valid profile form submission. * - * @param string $name + * @param string $profileName * @return mixed */ - public static function processProfile($name) { - return self::singleton()->invoke(array('name'), $name, self::$_nullObject, self::$_nullObject, self::$_nullObject, + public static function processProfile($profileName) { + return self::singleton()->invoke(array('profileName'), $profileName, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, 'civicrm_processProfile'); } /** * This hook is called while preparing a read-only profile screen * - * @param string $name + * @param string $profileName * @return mixed */ - public static function viewProfile($name) { - return self::singleton()->invoke(array('name'), $name, self::$_nullObject, self::$_nullObject, self::$_nullObject, + public static function viewProfile($profileName) { + return self::singleton()->invoke(array('profileName'), $profileName, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, 'civicrm_viewProfile'); } /** * This hook is called while preparing a list of contacts (based on a profile) * - * @param string $name + * @param string $profileName * @return mixed */ - public static function searchProfile($name) { - return self::singleton()->invoke(array('name'), $name, self::$_nullObject, self::$_nullObject, self::$_nullObject, + public static function searchProfile($profileName) { + return self::singleton()->invoke(array('profileName'), $profileName, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, self::$_nullObject, 'civicrm_searchProfile'); } diff --git a/Civi/Core/Event/GenericHookEvent.php b/Civi/Core/Event/GenericHookEvent.php index faee1e7e3a..9c47029c9b 100644 --- a/Civi/Core/Event/GenericHookEvent.php +++ b/Civi/Core/Event/GenericHookEvent.php @@ -110,6 +110,22 @@ class GenericHookEvent extends \Symfony\Component\EventDispatcher\Event { */ private $returnValues = array(); + /** + * List of field names that are prohibited due to conflicts + * in the class-hierarchy. + * + * @var array + */ + private static $BLACKLIST = array( + 'name', + 'dispatcher', + 'propagationStopped', + 'hookBlacklist', + 'hookValues', + 'hookFields', + 'hookFieldsFlip', + ); + /** * Create a GenericHookEvent using key-value pairs. * @@ -122,6 +138,7 @@ class GenericHookEvent extends \Symfony\Component\EventDispatcher\Event { $e->hookValues = array_values($params); $e->hookFields = array_keys($params); $e->hookFieldsFlip = array_flip($e->hookFields); + self::assertValidHookFields($e->hookFields); return $e; } @@ -142,9 +159,22 @@ class GenericHookEvent extends \Symfony\Component\EventDispatcher\Event { $e->hookValues = $hookValues; $e->hookFields = $hookFields; $e->hookFieldsFlip = array_flip($e->hookFields); + self::assertValidHookFields($e->hookFields); return $e; } + /** + * @param array $fields + * List of field names. + */ + private static function assertValidHookFields($fields) { + $bad = array_intersect($fields, self::$BLACKLIST); + if ($bad) { + throw new \RuntimeException("Hook relies on conflicted field names: " + . implode(', ', $bad)); + } + } + /** * @return array * Ex: array(0 => &$contactID, 1 => &$contentPlacement). -- 2.25.1