Uploaded image for project: 'CiviCRM'
  1. CiviCRM
  2. CRM-13104

Hook results may redundantly merge (unverified)

    Details

    • Type: Bug
    • Status: Done/Fixed
    • Priority: Major
    • Resolution: Fixed/Completed
    • Affects Version/s: 4.3.5
    • Fix Version/s: 4.7
    • Component/s: Core CiviCRM
    • Labels:
      None
    • Documentation Required?:
      None
    • Funding Source:
      Core Team Funds

      Description

      The following code looks suspicious/buggy to me, but I haven't tested/verified the problem:

      https://github.com/civicrm/civicrm-core/blob/2efcf0c212127e3281834edf26b64d2bbd5e0bb5/CRM/Utils/Hook.php#L169

      Suppose we have:
      1. $civiModules == array('a','b')
      2. $fnName == 'example'
      3. function_exists('a_example') == true
      4. function_exists('b_example') == false
      5. a_example() returns array('first')

      It looks like the value of $fResult would"leak" between iterations – ie after calling a_example(), we would set $fResult and merge it in. On the next iteration, $fResult would still be set and would get merged in a second time (even though there's no b_example()).

      This probably hasn't been a big problem because most of our hooks don't return values – instead, we usually pass in alterable arrays (array-references) and ignore return values. Never-the-less, I believe some hooks have return values.

      Note: In working on this, it would make sense to add more tests to CRM_Utils_HookTest.

        Attachments

          Activity

            People

            • Assignee:
              colemanw Coleman Watts
              Reporter:
              timotten Tim Otten
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: