Summary: | ASTERISK-12519: doesn't set ~~EXTEN~~ on the rigth place when a switch statement has been found | ||
Reporter: | Humberto Figuera (korihor) | Labels: | |
Date Opened: | 2008-08-04 18:33:20 | Date Closed: | 2008-08-07 19:07:03 |
Priority: | Major | Regression? | No |
Status: | Closed/Complete | Components: | PBX/pbx_ael |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ||
Description: | The instruction 'Set(~~EXTEN~~=${EXTEN})' created by pbx_ael is placed on a wrong location. I understand that the idea is to place it before the first 'switch', but if the first switch is found inside a conditional 'if' there is a problem, because ~~EXTEN~~ would only be set if this condition is 'true', otherwise 'Set(~~EXTEN~~=${EXTEN})' is never executed and ${EXTEN} is replaced in the rest of extension with ${~~EXTEN~~} which doesn't exists. Here is an example: Dialplan test: context test { 0123 => { NoOp("Here EXTEN = ${EXTEN} - good"); if ("${CALLERID(num):0:1}" = "2") { switch (${CALLERID(num):1:1}) { pattern [02]: goto blacklist; break; default: break; }; NoOp("Here EXTEN = ${EXTEN} - fortunately because first 'switch'"); } else if ("${CALLERID(num):0:1}" = "4") { switch (${CALLERID(num):1:1}) { pattern [03-9]: goto blacklist; break; default: break; }; NoOp("Here EXTEN = ${EXTEN} - unfortunately because not is first 'switch' - bad "); } else { goto blacklist; } Answer(); NoOp("Here EXTEN = ${EXTEN} - good if CALLERID(num):0:1 = 2\, bad otherwise"); Playback(test/${EXTEN}/welcome); Hangup(); blacklist: NoOp("Here EXTEN = ${EXTEN} - good if CALLERID(num):0:1 = 2\, bad otherwise"); Playback(test/${EXTEN}/you-are-on-blacklist|noanswer); Hangup(); }; }; [ Context 'test' created by 'pbx_ael' ] '0123' => 1. NoOp("Here EXTEN = ${EXTEN} - good") [pbx_ael] 2. GotoIf($["${CALLERID(num):0:1}" = "2"]?3:8) [pbx_ael] 3. Set(~~EXTEN~~=${EXTEN}) [pbx_ael] 4. Goto(sw-2-${CALLERID(num):1:1}|10) [pbx_ael] 5. NoOp(Finish switch-if-test-1-2) [pbx_ael] 6. NoOp("Here EXTEN = ${~~EXTEN~~} - fortunately because first 'switch'") [pbx_ael] 7. Goto(15) [pbx_ael] 8. GotoIf($["${CALLERID(num):0:1}" = "4"]?9:13) [pbx_ael] 9. Goto(sw-4-${CALLERID(num):1:1}|10) [pbx_ael] 10. NoOp(Finish switch-if-if-test-1-3-4) [pbx_ael] 11. NoOp("Here EXTEN = ${~~EXTEN~~} - unfortunately because not is first 'switch' - bad ") [pbx_ael] 12. Goto(14) [pbx_ael] 13. Goto(blacklist) [pbx_ael] 14. NoOp(Finish if-if-test-1-3) [pbx_ael] 15. NoOp(Finish if-test-1) [pbx_ael] 16. Answer() [pbx_ael] 17. NoOp("Here EXTEN = ${~~EXTEN~~} - good if CALLERID(num):0:1 = 2, bad otherwise") [pbx_ael] 18. Playback(test/${~~EXTEN~~}/welcome) [pbx_ael] 19. Hangup() [pbx_ael] [blacklist] 20. NoOp("Here EXTEN = ${~~EXTEN~~} - good if CALLERID(num):0:1 = 2, bad otherwise") [pbx_ael] 21. Playback(test/${~~EXTEN~~}/you-are-on-blacklist|noanswer) [pbx_ael] 22. Hangup() [pbx_ael] 'sw-2-' => 10. Goto(sw-2-.|10) [pbx_ael] 'sw-4-' => 10. Goto(sw-4-.|10) [pbx_ael] '_sw-2-[02]' => 10. Goto(0123|blacklist) [pbx_ael] 11. Goto(0123|5) [pbx_ael] '_sw-2-.' => 10. Goto(0123|5) [pbx_ael] '_sw-4-[03-9]' => 10. Goto(0123|blacklist) [pbx_ael] 11. Goto(0123|10) [pbx_ael] '_sw-4-.' => 10. Goto(0123|10) [pbx_ael] -= 7 extensions (30 priorities) in 1 context. =- ****** ADDITIONAL INFORMATION ****** The line 2792 on pbx/pbx_ael.c explains why the replacement is done: /* The following code will cause all priorities within an extension to have ${EXTEN} or ${EXTEN: replaced with ~~EXTEN~~, which is set just before the first switch in an exten. The switches will muck up the original ${EXTEN} value, so we save it away and the user accesses this copy instead. */ The line 3185 on pbx/pbx_ael.c sets ~~EXTEN~~ to ${EXTEN} on the first 'switch': if ((mother_exten && !mother_exten->has_switch)) { switch_set = new_prio(); switch_set->type = AEL_APPCALL; switch_set->app = strdup("Set"); switch_set->appargs = strdup("~~EXTEN~~=${EXTEN}"); linkprio(exten, switch_set, mother_exten); mother_exten->has_switch = 1; } else if ((exten && !exten->has_switch)) { switch_set = new_prio(); switch_set->type = AEL_APPCALL; switch_set->app = strdup("Set"); switch_set->appargs = strdup("~~EXTEN~~=${EXTEN}"); linkprio(exten, switch_set, exten); exten->has_switch = 1; } | ||
Comments: | By: Digium Subversion (svnbot) 2008-08-07 19:07:01 Repository: asterisk Revision: 136726 U branches/1.4/include/asterisk/ael_structs.h U branches/1.4/pbx/ael/ael-test/ref.ael-ntest10 U branches/1.4/pbx/ael/ael-test/ref.ael-test18 U branches/1.4/pbx/ael/ael-test/ref.ael-test8 U branches/1.4/pbx/ael/ael-test/ref.ael-vtest13 U branches/1.4/pbx/pbx_ael.c ------------------------------------------------------------------------ r136726 | murf | 2008-08-07 19:07:00 -0500 (Thu, 07 Aug 2008) | 32 lines (closes issue ASTERISK-12519) Reported by: korihor Wow, this one was a challenge! I regrouped and ran a new strategy for setting the ~~MACRO~~ value; I set it once per extension, up near the top. It is only set if there is a switch in the extension. So, I had to put in a chunk of code to detect a switch in the pval tree. I moved the code to insert the set of ~~exten~~ up to the beginning of the gen_prios routine, instead of down in the switch code. I learned that I have to push the detection of the switches down into the code, so everywhere I create a new exten in gen_prios, I make sure to pass onto it the values of the mother_exten first, and the exten next. I had to add a couple fields to the exten struct to accomplish this, in the ael_structs.h file. The checked field makes it so we don't repeat the switch search if it's been done. I also updated the regressions. ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=136726 |