Ticket #5081 (new Bugs)

Opened 2 years ago

Last modified 1 year ago

extensions.class.php: remove() & replace() don't work correctly for numeric priority

Reported by: pwalker Assigned to: mbrevda
Priority: trivial Milestone: 2.11
Component: Core Version: 2.7-branch
Keywords: extensions.class.php remove replace Cc: tonyclewis
Confirmation: Pending Distro:
Backend Engine: All Distro Ver:
Backend Ver: SVN Revision (if applicable):

Description (Last modified by pwalker)

The function remove($section, $extension, $priority) in extensions.class.php removes the wrong element when a numeric priority is used. e.g. 

$ext->remove("mycontext", "exten", 1);

tries to remove / removes the array's element with index 1 instead of the first element in the array (index 0).

current code: 

unset($this->_exts[$section][$extension][$priority]);

I guess this function hasn't ever been (and actually, normally shouldn't be) used before...

Update: The same applies for $ext->replace()

Update 2012-03-21: Attaching patch for current svn - it's the same as since 2.7

Attachments

extensions.class.php.27.patch (0.8 kB) - added by pwalker on 04/21/11 09:57:10.
Updated Patch for FreePBX 2.7
extensions.class.php.29.patch (0.8 kB) - added by pwalker on 04/22/11 15:57:10.
Patch for FreePBX 2.9
extensions.class.php.svn.patch (0.8 kB) - added by pwalker on 03/21/12 12:30:51.
Patch for svn as of 2012-03-21

Change History

04/19/11 05:56:29 changed by pwalker

  • priority changed from minor to trivial.
  • description changed.

04/19/11 11:31:41 changed by p_lindheimer

  • confirmation changed from Unreviewed to Pending.
  • milestone changed from Cut Line to 2.10.

pwalker,

I believe this is used in some third party modules and was using only tags. I almost hesitate to fix it and require a tag only since it becomes so volatile when dealing with priorities as they change. However … understandably until a tag can be placed where someone would need it, the priority is the only way… I hesitate to make the change in 2.9 so as not to break anything so I'm going to move this to 2.10 for now, if that creates issues with you let me know and we can take a closer look and thus see about getting it fixed in 2.9. Let me know.

04/19/11 11:45:39 changed by pwalker

  • version changed from 2.9-branch to 2.7-branch.

Philippe,

It wasn't my intention to have this fixed for 2.9 final or even delay the release! So, 2.10 is fine for me. (Sorry that I've selected 2.9-branch!) Would be nice to have this fixed one day, but no hurry! (It's been there for quite a while, I guess...) Concerning the "not to break anything": It shouldn't break the case when remove() is used with a tag - that's why the code for the fix/patch became a little fatter than expected (including some ifs/elses) and I've also done some testing for both cases.

04/21/11 09:53:54 changed by pwalker

  • description changed.

04/21/11 09:57:10 changed by pwalker

  • attachment extensions.class.php.27.patch added.

Updated Patch for FreePBX 2.7

04/21/11 09:58:42 changed by pwalker

Uploaded updated patch: Also corrects $ext->replace() and I found out that it is simpler than I thought.

04/22/11 15:57:10 changed by pwalker

  • attachment extensions.class.php.29.patch added.

Patch for FreePBX 2.9

(follow-up: ↓ 8 ) 09/17/11 21:31:17 changed by mbrevda

  • milestone changed from 2.10 to Undetermined.

pwalker, Patches need to be agains trunk

03/21/12 12:29:26 changed by pwalker

  • keywords changed from extensions.class.php remove to extensions.class.php remove replace.
  • description changed.
  • summary changed from $ext->remove() doesn't work correctly for numeric priority to extensions.class.php: remove() & replace() don't work correctly for numeric priority.

03/21/12 12:30:51 changed by pwalker

  • attachment extensions.class.php.svn.patch added.

Patch for svn as of 2012-03-21

(in reply to: ↑ 6 ) 03/21/12 12:34:13 changed by pwalker

Replying to mbrevda:

pwalker, Patches need to be agains trunk

Attaching patch against current SVN - it's the same as since 2.7 because the file hasn't really changed and the issue is still there in 2.10.

03/21/12 19:00:56 changed by p_lindheimer

  • milestone changed from Undetermined to 2.11.

03/21/12 20:53:22 changed by p_lindheimer

  • cc set to tonyclewis.
  • owner changed from p_lindheimer to mbrevda.

Assigning to Moshe to review and determine if this should be put in or not. Scanning the main FreePBX code, none of the standard FreePBX modules use these methods so I am inclined to apply pwalker's patch as is as it won't break FreePBX. However, this was originally added by Ethan so I suspect there may be some commercial modules that use these methods and want to make sure you have a chance to review before without just applying this so that it does not break something you may be using.

If you are ok with it go ahead and apply it or reassign it to me and I will do that.