| Summary: | ASTERISK-13218: Extensions configuration is not being sorted correctly | ||||
| Reporter: | TOC Jason (toc) | Labels: | |||
| Date Opened: | 2008-12-15 06:53:10.000-0600 | Date Closed: | 2008-12-16 14:04:43.000-0600 | ||
| Priority: | Minor | Regression? | No | ||
| Status: | Closed/Complete | Components: | Core/PBX | ||
| Versions: | Frequency of Occurrence | ||||
| Related Issues: |
| ||||
| Environment: | Attachments: | ||||
| Description: | The matching performed on extensions.conf is not sorting the items as expected. Take this example: [test-ext] exten => _.,1,NoOp(Match 1) exten => s-Nothing,1,NoOp(Match 3) exten => _s.,1,NoOp(Match 2) exten => _s-.,1,NoOp(Match 4) The result of: [test-ext] exten => _.,1,NoOp(Match 1) exten => s-Nothing,1,NoOp(Match 3) exten => _s.,1,NoOp(Match 2) exten => _s-.,1,NoOp(Match 4) Is: — Executing [s-Test@test-ext:1] NoOp("Local/s-Test@test-ext-0cbc;2", "Match 2") in new stack I would expect the match to occur on Match 4, this is the closed match based on the number of items to match on. Swap them around: [test-ext] exten => _.,1,NoOp(Match 1) exten => s-Nothing,1,NoOp(Match 3) exten => _s-.,1,NoOp(Match 4) exten => _s.,1,NoOp(Match 2) And this is the output: — Executing [s-Test@test-ext:1] NoOp("Local/s-Test@test-ext-5f64;2", "Match 4") in new stack It appears to be inconsistent in applying the matching as I would expect regardless of entry position, the same results to occur due to the matching. | ||||
| Comments: | By: TOC Jason (toc) 2008-12-15 07:23:01.000-0600 Even more strange is this match: [test-ext] exten => _s-.,1,NoOp(Match 4) exten => _.,1,NoOp(Match 1) exten => _s.,1,NoOp(Match 2) originate Local/sd@test-ext extension s-Nothing@test-ext -- Executing [sd@test-ext:1] NoOp("Local/sd@test-ext-dcaf;2", "Match 4") in new stack This shouldn't match at Match 4 at all - it should at either Match 1 or Match 2. Maybe the "-" is being evaluated in some way. By: Leif Madsen (lmadsen) 2008-12-15 09:33:31.000-0600 Assigned to myself as I'm going to verify with latest Asterisk. By: Leif Madsen (lmadsen) 2008-12-15 09:47:39.000-0600 Actually I get a separate error even than you with latest trunk. It seems the dialplan doesn't even allow us to load _s-. because it seems to think we've already loaded that extension: -- Registered extension context 'test-ext' (0xfa95fe0) in local table 0xfa98b60; registrar: pbx_config [Dec 15 06:45:17] WARNING[27102]: pbx_config.c:1550 pbx_load_config: The use of '_.' for an extension is strongly discouraged and can have unexpected behavior. Please use '_X.' instead at line 257 -- Added extension '_.' priority 1 to test-ext (0xfa95fe0) -- Added extension 's-Nothing' priority 1 to test-ext (0xfa95fe0) -- Added extension '_s.' priority 1 to test-ext (0xfa95fe0) [Dec 15 06:45:17] WARNING[27102]: pbx.c:7276 add_pri: Unable to register extension '_s-.', priority 1 in 'test-ext', already in use And the dialplan I copy/pasted: [test-ext] exten => _.,1,NoOp(Match 1) exten => s-Nothing,1,NoOp(Match 3) exten => _s.,1,NoOp(Match 2) exten => _s-.,1,NoOp(Match 4) So it would seem something has changed in the way the extensions are parsed in the dialplan. By: Leif Madsen (lmadsen) 2008-12-15 09:49:19.000-0600 Assigned to Corydon76 as I've reproduced an issue with the dialplan parser. By: Leif Madsen (lmadsen) 2008-12-15 10:00:35.000-0600 Reassigned to murf per discussion on IRC. By: Leif Madsen (lmadsen) 2008-12-16 08:58:47.000-0600 OK, after a discussion on IRC and a code review by murf, it seems this is actually expected behaviour (however it is not documented). I am going to create some documentation for it this afternoon and then this issue will be closed. Basically the issue is that hyphens (-) are ignored in pattern matches. So what is happening is that: _s-. and _s. Are actually the same pattern because the hyphen is ignored. This is the same reason why I was only seeing 3 matches load up in my dialplan when I was trying to duplicate your behaviour, and why I also got the "duplicate" warning when loading the dialplan. In this case order DOES matter because the 1st listed pattern is going to win, and the 2nd one will be thrown away. Hope that all makes sense, and I will expand upon this in the documention later this afternoon, and will probably write a blog post as well. Thanks! By: Steve Murphy (murf) 2008-12-16 09:01:53.000-0600 The pattern matcher, since about 2.5 years ago, and according to matching rules hashed out at a previous astridevcon, iirc (it was before *my* time), has the stipulation that spaces and dashes in patterns be ignored. Thus, _s-. and _s. are equivalent, and the first one will win, and the second will be interpreted as a duplicate and be tossed out. This explains the Match 2/4 difference. This behavior is pretty clearly explained in the code, but it doesn't say why. I'll add a little verbage to pbx.c to explain this. Personally, I'd not have put this sort of thing into the code. Sure, it makes it so you can directly feed stuff like NXX-XXX-XXXX into patterns and have them match 3078918888 into such a pattern but... the cost is that such small, not-often-mentioned exceptions confuse users and earn us bug reports. I've been working a lot on the dialplan code and I hadn't noticed this till now, either! But I can't just remove it, as there may be thousands of installations that depend on this behavior. By: Leif Madsen (lmadsen) 2008-12-16 09:02:47.000-0600 I'm keeping this open until some documentation has been merged into Asterisk. If this is closed I won't see it in My View. By: Digium Subversion (svnbot) 2008-12-16 14:04:42.000-0600 Repository: asterisk Revision: 164801 U trunk/main/pbx.c ------------------------------------------------------------------------ r164801 | murf | 2008-12-16 14:04:42 -0600 (Tue, 16 Dec 2008) | 36 lines (closes issue ASTERISK-13218) Reported by: toc Tested by: murf OK, Well this issue has had its share of flip-flopping. I found the following: 1. the code in question, in ext_cmp1 in pbx.c, would not allow two extensions that vary only by any dashes contained within them, to be defined in the same context. 2. for input dialstrings, dashes are NOT ignored. So, skipping them when sorting patterns seemed a bit silly. Thus, you might declare ext 891 in a context, but if you try dialing 8-9-1, it will NOT match 891. So, I proposed to remove the code from ext_cmp1 to skip the spaces and dashes. Just kept us from declaring 891 and 8-9-1 in the same context, forcing users to generate otherwise uselessly obfuscated dialplan code to get the same effect. Then, I tried out 1.4, and found that: 1. you can declare 891 and 8-9-1 in the same context! 2. You can't define 891, and have 8-9-1 match it! Nor can you define 8-9-1, and have 891 match it! So, it appears that my proposal simply restores the pbx to behaving as it did in 1.4. ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=164801 | ||||