[Home]

Summary:ASTERISK-15340: [patch] Serious problem with pattern matching (regression in #15421)
Reporter:Walter Doekes (wdoekes)Labels:
Date Opened:2009-12-21 03:55:50.000-0600Date Closed:2010-01-04 15:00:48.000-0600
Priority:MajorRegression?Yes
Status:Closed/CompleteComponents:PBX/pbx_config
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20091223__issue16482.diff.txt
( 1) astsvn-16482-betterfix.diff
( 2) astsvn-16482-simplefix.diff
Description:Hi, with the following dialplan:

[context]
exten => _[23],1,Set(category=NL-normal)
exten => _[45],1,Set(category=NL-normal)
exten => _[67],1,Set(category=NL-normal)
exten => _[89],1,Set(category=NL-normal)
exten => _X,1,Set(category=NL-unknown) ; (catch all)

_X sorts above [23] since asterisk 1.6.1.10.

Look at the following outputs:


[Asterisk 1.6.1.9]

*CLI> dialplan show context
[ Context 'context' created by 'pbx_config' ]
 '_[23]' =>        1. Set(category=NL-normal)                    [pbx_config]
 '_[45]' =>        1. Set(category=NL-normal)                    [pbx_config]
 '_[67]' =>        1. Set(category=NL-normal)                    [pbx_config]
 '_[89]' =>        1. Set(category=NL-normal)                    [pbx_config]
 '_X' =>           1. Set(category=NL-unknown)                   [pbx_config]

[Asterisk 1.6.1.10 and above, up to -svn]

*CLI> dialplan show context
[ Context 'context' created by 'pbx_config' ]
 '_[89]' =>        1. Set(category=NL-normal)                    [pbx_config]
 '_X' =>           1. Set(category=NL-unknown)                   [pbx_config]
 '_[67]' =>        1. Set(category=NL-normal)                    [pbx_config]
 '_[45]' =>        1. Set(category=NL-normal)                    [pbx_config]
 '_[23]' =>        1. Set(category=NL-normal)                    [pbx_config]


In my real world case:
exten => _+31X!,2,Set(category=NL-unknown) ; (catch all)
sorts above:
exten => _+31[2357]XXXXXXXX,2,Set(category=NL-normal)
yielding NL-unknown for normal Dutch phone numbers.


As far as I can see, this bug was introduced by the patch applied to fix ASTERISK-14398. (It touches the ext_cmp stuff and was introduced in 1.6.1.10.)

I'll try to come up with more detailed information. (But I've flagged it as Major in the mean time as I believe it to be a serious issue.)


Regards,
Walter Doekes
OSSO B.V.
Comments:By: Walter Doekes (wdoekes) 2009-12-21 18:28:19.000-0600

List of things I fixed:

(1) The old comment at the top of ext_cmp1 should have been removed.
(2) Moved the while() to a do()while as it only needed checking
   after the calls.
(3) Skipped past the underscores (unnecessary extra check).
(3) Moved the memset out of the ext_cmp1. This way it has to know
   less about the actual length of bitwise.
(4) "bitwise[ c1 / 8 ] & mask" was compared to 1 which fails unless
   mask is also 1. It should've been compared against mask (or just
   the truth value).
(5) Changed unsetting bits in the mask to setting bits so sort order
   works better ([2-4] before [246] makes more sense than the other
   way around).
By: Tilghman Lesher (tilghman) 2009-12-22 22:28:31.000-0600

(6) _2 and _[2] now compare as equal.  Previously, _2 would have sorted before _[2] and would not have flagged an error as being equal.

Additionally, as to (3), I've removed the memset altogether, as the structure can simply be initialized statically.  I've also restored the original syntax, as I think it is more clear, and that code already suffers from being a little difficult to read.
By: Tilghman Lesher (tilghman) 2009-12-22 22:29:56.000-0600

Additionally, when committed, this change should go into 1.4 and forward, not just into trunk.
By: Walter Doekes (wdoekes) 2009-12-23 00:39:51.000-0600

(6) Excellent
(memset) Okay, I wasn't sure about the portability of initializing arrays. Reducing the memory to one long strip saved me from ugly casts to one memset.

Also, since you've added 'z', 'n' and 'x', you can drop the toupper().
By: Tilghman Lesher (tilghman) 2009-12-23 01:28:39.000-0600

Oops.  I was thinking about doing just that, but I dropped it, since it's not backwards compatible with the current workings of released versions.  We can reconsider that in trunk, however.
By: Digium Subversion (svnbot) 2010-01-04 14:57:38.000-0600

Repository: asterisk
Revision: 237493

U   branches/1.4/main/pbx.c

------------------------------------------------------------------------
r237493 | tilghman | 2010-01-04 14:57:36 -0600 (Mon, 04 Jan 2010) | 8 lines

Regression in issue ASTERISK-14398 - Pattern matching
(closes issue ASTERISK-15340)
Reported by: wdoekes
Patches:
      astsvn-16482-betterfix.diff uploaded by wdoekes (license 717)
      20091223__issue16482.diff.txt uploaded by tilghman (license 14)
Tested by: wdoekes, tilghman

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=237493
By: Digium Subversion (svnbot) 2010-01-04 14:59:03.000-0600

Repository: asterisk
Revision: 237494

_U  trunk/
U   trunk/main/pbx.c

------------------------------------------------------------------------
r237494 | tilghman | 2010-01-04 14:59:01 -0600 (Mon, 04 Jan 2010) | 15 lines

Merged revisions 237493 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r237493 | tilghman | 2010-01-04 14:57:35 -0600 (Mon, 04 Jan 2010) | 8 lines
 
 Regression in issue ASTERISK-14398 - Pattern matching
 (closes issue ASTERISK-15340)
  Reported by: wdoekes
  Patches:
        astsvn-16482-betterfix.diff uploaded by wdoekes (license 717)
        20091223__issue16482.diff.txt uploaded by tilghman (license 14)
  Tested by: wdoekes, tilghman
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=237494
By: Digium Subversion (svnbot) 2010-01-04 15:00:31.000-0600

Repository: asterisk
Revision: 237495

_U  branches/1.6.0/
U   branches/1.6.0/main/pbx.c

------------------------------------------------------------------------
r237495 | tilghman | 2010-01-04 15:00:29 -0600 (Mon, 04 Jan 2010) | 22 lines

Merged revisions 237494 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r237494 | tilghman | 2010-01-04 14:59:01 -0600 (Mon, 04 Jan 2010) | 15 lines
 
 Merged revisions 237493 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r237493 | tilghman | 2010-01-04 14:57:35 -0600 (Mon, 04 Jan 2010) | 8 lines
   
   Regression in issue ASTERISK-14398 - Pattern matching
   (closes issue ASTERISK-15340)
    Reported by: wdoekes
    Patches:
          astsvn-16482-betterfix.diff uploaded by wdoekes (license 717)
          20091223__issue16482.diff.txt uploaded by tilghman (license 14)
    Tested by: wdoekes, tilghman
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=237495
By: Digium Subversion (svnbot) 2010-01-04 15:00:39.000-0600

Repository: asterisk
Revision: 237496

_U  branches/1.6.1/
U   branches/1.6.1/main/pbx.c

------------------------------------------------------------------------
r237496 | tilghman | 2010-01-04 15:00:37 -0600 (Mon, 04 Jan 2010) | 22 lines

Merged revisions 237494 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r237494 | tilghman | 2010-01-04 14:59:01 -0600 (Mon, 04 Jan 2010) | 15 lines
 
 Merged revisions 237493 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r237493 | tilghman | 2010-01-04 14:57:35 -0600 (Mon, 04 Jan 2010) | 8 lines
   
   Regression in issue ASTERISK-14398 - Pattern matching
   (closes issue ASTERISK-15340)
    Reported by: wdoekes
    Patches:
          astsvn-16482-betterfix.diff uploaded by wdoekes (license 717)
          20091223__issue16482.diff.txt uploaded by tilghman (license 14)
    Tested by: wdoekes, tilghman
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=237496
By: Digium Subversion (svnbot) 2010-01-04 15:00:47.000-0600

Repository: asterisk
Revision: 237497

_U  branches/1.6.2/
U   branches/1.6.2/main/pbx.c

------------------------------------------------------------------------
r237497 | tilghman | 2010-01-04 15:00:45 -0600 (Mon, 04 Jan 2010) | 22 lines

Merged revisions 237494 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r237494 | tilghman | 2010-01-04 14:59:01 -0600 (Mon, 04 Jan 2010) | 15 lines
 
 Merged revisions 237493 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r237493 | tilghman | 2010-01-04 14:57:35 -0600 (Mon, 04 Jan 2010) | 8 lines
   
   Regression in issue ASTERISK-14398 - Pattern matching
   (closes issue ASTERISK-15340)
    Reported by: wdoekes
    Patches:
          astsvn-16482-betterfix.diff uploaded by wdoekes (license 717)
          20091223__issue16482.diff.txt uploaded by tilghman (license 14)
    Tested by: wdoekes, tilghman
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=237497