| Summary: | ASTERISK-16121: [patch] strange extension pattern matching | ||||
| Reporter: | Steve Murphy (murf) | Labels: | |||
| Date Opened: | 2010-05-20 12:57:25 | Date Closed: | 2010-10-04 18:23:59 | ||
| Priority: | Major | Regression? | No | ||
| Status: | Closed/Complete | Components: | PBX/General | ||
| Versions: | Frequency of Occurrence | ||||
| Related Issues: |
| ||||
| Environment: | Attachments: | ( 0) 20100727__issue17366.diff.txt ( 1) extension.conf | |||
| Description: | Maybe I'm just hallucinating, but if I goto(mysw-17-3,label) and have these extensions defined: _mysw-17-. _mysw-1-. mysw-17- mysw-17-* mysw-17-2 mysw-17-1 s The last match I would expect would be the second item in the list above, but that's the one that is chosen. ****** STEPS TO REPRODUCE ****** Define (in AEL or extensions.conf) : context test394 { s => goto mysw-15-OK|label; mysw-15-OK => { label: Set(zz=3); goto sw-17-${zz}|label2; } _mysw-1-. => { label2:NoOp(in exten _mysw-1-.); goto s|1;} _mysw-17-. => { label2:NoOp(in exten _mysw-17-.); goto s|1;} mysw-17- => { label2:NoOp(in exten mysw-1-); goto s|1;} mysw-17-* => { label2:NoOp(in exten mysw-1-*); goto s|1;} mysw-17-2 => { label2:NoOp(in exten mysw-1-2); goto s|1;} mysw-17-1 => { label2:NoOp(in exten mysw-1-1); goto s|1;} } load it into asterisk, and on the console: *CLI> dialplan show mysw-17-3@test394 [ Context 'test394' created by 'pbx_ael' ] '_mysw-1-.' => 1. NoOp(in exten _mysw-1-.) [pbx_ael] 2. Goto(s,1) [pbx_ael] '_mysw-17-.' => 1. NoOp(in exten _mysw-17-.) [pbx_ael] 2. Goto(s,1) [pbx_ael] -= 2 extensions (4 priorities) in 1 context. =- I really can't warp my brain to understand why _mysw-1-. is even in the list, because the - just before the dot in the mysw-1-. pattern should have disqualified that pattern! But, if I'm missing something (and I'm known to do that!), please let me know! ****** ADDITIONAL INFORMATION ****** BTW- This little tidbit of information has nothing to do with this bug, but the new pattern matcher selects the expected exten, mysw-17-. | ||||
| Comments: | By: Leif Madsen (lmadsen) 2010-05-25 11:19:49 Well I can certainly reproduce this and I have no idea either why this would match. Not a good thing! I'll ask Jared to see if there is something I'm also missing. By: Paul Belanger (pabelanger) 2010-05-26 11:22:03 For what its worth, here is an output from Asterisk using 'dialplan show' --- *CLI> dialplan show mysw-17-3@issue0017366 [ Context 'issue0017366' created by 'pbx_config' ] '_mysw-1-.' => 1. NoOp(2) [pbx_config] '_mysw-17-.' => 1. NoOp(1) [pbx_config] -= 2 extensions (2 priorities) in 1 context. =- By: Tilghman Lesher (tilghman) 2010-06-24 11:15:47 You should understand that in pattern matches, the "-" character is ignored as a separator character. This is so that you can do "_NXX-NXX-XXXX" for readability. Now it should be clear why it matches and also why the new pattern match algorithm (which is based upon best score, not first match) selects the item you want. By: Jared Smith (jsmith) 2010-06-24 14:19:11 I agree with Tilghman... the problem is that the - in the pattern is ignored. By: Steve Murphy (murf) 2010-06-30 18:02:56 Yes, I dimly remember that little feature of pattern matching now. And that means that the error isn't in the standard pattern matcher, it's in the fast pattern matcher and the AEL code generator. I'll see what I can do to upgrade those two to follow this convention. In the AEL, it'll simply be a matter of using the _ instead of - in generated extension names. In the fast pattern matcher its got to skip the dashes in the patterns and the strings, just like the standard algorithm does. Oh, joy! By: Tilghman Lesher (tilghman) 2010-07-27 15:26:46 I'm thinking something like this. This should work most of the time, but there's probably a battery of tests that could be run to verify. By: Steve Murphy (murf) 2010-07-28 07:46:15 Tilghman-- There *is* a battery of tests, and it would be very good to keep them up to date. look in pbx/ael/ael-test; run the 'runtests' script. I've been meaning to update, but perhaps you could verify that the current diffs are what they are meant to be, and update them, then run them on your patch, and see if that louses up anything. I'd add a new test or two with the patterns I described above, and maybe one more along that line, as regression tests...? By: Digium Subversion (svnbot) 2010-10-04 18:15:00 Repository: asterisk Revision: 290254 U branches/1.6.2/main/pbx.c U branches/1.6.2/pbx/ael/ael-test/ref.ael-ntest10 U branches/1.6.2/pbx/ael/ael-test/ref.ael-test1 U branches/1.6.2/pbx/ael/ael-test/ref.ael-test19 U branches/1.6.2/pbx/ael/ael-test/ref.ael-test2 U branches/1.6.2/pbx/ael/ael-test/ref.ael-test3 U branches/1.6.2/pbx/ael/ael-test/ref.ael-test4 U branches/1.6.2/pbx/ael/ael-test/ref.ael-test5 U branches/1.6.2/pbx/ael/ael-test/ref.ael-vtest13 U branches/1.6.2/pbx/ael/ael-test/ref.ael-vtest17 U branches/1.6.2/res/ael/pval.c ------------------------------------------------------------------------ r290254 | tilghman | 2010-10-04 18:15:00 -0500 (Mon, 04 Oct 2010) | 11 lines Change new pattern matcher to regard dashes the same as the old pattern matcher -- as visual candy to be ignored. Also change the AEL parser to not generate dashes within extensions, as those dashes would be ignored. Update the AEL tests to match this behavior. (closes issue ASTERISK-16121) Reported by: murf Patches: 20100727__issue17366.diff.txt uploaded by tilghman (license 14) Tested by: tilghman ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=290254 By: Digium Subversion (svnbot) 2010-10-04 18:23:12 Repository: asterisk Revision: 290255 _U branches/1.8/ U branches/1.8/main/pbx.c U branches/1.8/pbx/ael/ael-test/ref.ael-ntest10 U branches/1.8/pbx/ael/ael-test/ref.ael-test1 U branches/1.8/pbx/ael/ael-test/ref.ael-test19 U branches/1.8/pbx/ael/ael-test/ref.ael-test2 U branches/1.8/pbx/ael/ael-test/ref.ael-test3 U branches/1.8/pbx/ael/ael-test/ref.ael-test4 U branches/1.8/pbx/ael/ael-test/ref.ael-test5 U branches/1.8/pbx/ael/ael-test/ref.ael-vtest13 U branches/1.8/pbx/ael/ael-test/ref.ael-vtest17 U branches/1.8/res/ael/pval.c ------------------------------------------------------------------------ r290255 | tilghman | 2010-10-04 18:23:11 -0500 (Mon, 04 Oct 2010) | 18 lines Merged revisions 290254 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ........ r290254 | tilghman | 2010-10-04 18:14:59 -0500 (Mon, 04 Oct 2010) | 11 lines Change new pattern matcher to regard dashes the same as the old pattern matcher -- as visual candy to be ignored. Also change the AEL parser to not generate dashes within extensions, as those dashes would be ignored. Update the AEL tests to match this behavior. (closes issue ASTERISK-16121) Reported by: murf Patches: 20100727__issue17366.diff.txt uploaded by tilghman (license 14) Tested by: tilghman ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=290255 By: Digium Subversion (svnbot) 2010-10-04 18:23:58 Repository: asterisk Revision: 290256 _U trunk/ U trunk/main/pbx.c U trunk/pbx/ael/ael-test/ref.ael-ntest10 U trunk/pbx/ael/ael-test/ref.ael-test1 U trunk/pbx/ael/ael-test/ref.ael-test19 U trunk/pbx/ael/ael-test/ref.ael-test2 U trunk/pbx/ael/ael-test/ref.ael-test3 U trunk/pbx/ael/ael-test/ref.ael-test4 U trunk/pbx/ael/ael-test/ref.ael-test5 U trunk/pbx/ael/ael-test/ref.ael-vtest13 U trunk/pbx/ael/ael-test/ref.ael-vtest17 U trunk/res/ael/pval.c ------------------------------------------------------------------------ r290256 | tilghman | 2010-10-04 18:23:58 -0500 (Mon, 04 Oct 2010) | 25 lines Merged revisions 290255 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ................ r290255 | tilghman | 2010-10-04 18:23:11 -0500 (Mon, 04 Oct 2010) | 18 lines Merged revisions 290254 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ........ r290254 | tilghman | 2010-10-04 18:14:59 -0500 (Mon, 04 Oct 2010) | 11 lines Change new pattern matcher to regard dashes the same as the old pattern matcher -- as visual candy to be ignored. Also change the AEL parser to not generate dashes within extensions, as those dashes would be ignored. Update the AEL tests to match this behavior. (closes issue ASTERISK-16121) Reported by: murf Patches: 20100727__issue17366.diff.txt uploaded by tilghman (license 14) Tested by: tilghman ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=290256 | ||||