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:20Date Closed:2008-08-07 19:07:03
Versions:Frequency of
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;

                               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;

                               NoOp("Here EXTEN = ${EXTEN} - unfortunately because not is first 'switch' - bad ");

                       } else {
                               goto blacklist;

                       NoOp("Here EXTEN = ${EXTEN} - good if CALLERID(num):0:1 = 2\, bad otherwise");

                       NoOp("Here EXTEN = ${EXTEN} - good if CALLERID(num):0:1 = 2\, bad otherwise");


[ 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. =-


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.