Summary:ASTERISK-06897: [patch] fix for unnecessary dial delay if 2 or more early match patterns (i.e. with !) are present
Reporter:Michael Neuhauser (mneuhauser)Labels:
Date Opened:2006-05-04 15:36:26Date Closed:2006-05-16 05:41:07
Versions:Frequency of
Description:Case: a context has 2 (or more) early match patterns like these:
   _9X! =>
   _X! =>
When "90" is dialled, the first pattern matches only after a short delay (matchdigittimeout in chan_zap) instead of immediatly after the "0" was dialled. Immediate matching does happen if the 2nd pattern is not present.

I believe to have this tracked down to a small buglet in pbx_find_extension():
 if (action == HELPER_MATCHMORE && match == 2 && !earlymatch) {
/* It matched an extension ending in a '!' wildcard
  So ignore it for now, unless there's a better match */
earlymatch = eroot;
 } else {

The match on the first early pattern is ignored, but not the 2nd one (as "earlymatch" was already set). The problem is, that the 2nd pattern is handled in the else branch, and a value != NULL is returned. The solution (which works in the above mentioned case), is to ignore ALL early matches, not only the first one:

 if (action == HELPER_MATCHMORE && match == 2) {
/* It matched an extension ending in a '!' wildcard
  So ignore it for now, unless there's a better match */
earlymatch = eroot;
 } else {

I believe this change to be safe, as nothing ever is done with the value of "earlymatch", except returning NULL later on if earlymatch != NULL (maybe using a flag instead of a pointer, i.e. "int had_earlymatch; ... had_earlymatch = 1;" would be clearer).
Comments:By: Serge Vecher (serge-v) 2006-05-04 15:42:08

1) please get a disclaimer on file
2) upload the path in diff -u format


By: Michael Neuhauser (mneuhauser) 2006-05-04 15:46:07

Short bugfix -> no disclaimer necessary.

regarding patch: You gotta be kidding me. I've spend hours tracking this down, and you are not willing to delete 15 characters by hand?

By: Luigi Rizzo (rizzo) 2006-05-10 15:11:02

Although i independently put a similar comment in the code in
my branch (now in trunk), i am not 100% sure the patch you propose
implements the intended behaviour, and also, in this specific case,
there is another issue related to the sorting of patterns in the
contents. Since we recently changed the latter, i would be grateful
if you (mneuhauser) could let me know if the problem still occurs
with the version of pbx.c now in trunk - the sorting order has been
changed and now more specific ones come first, so you should
match the _9X! immediately.

By: Michael Neuhauser (mneuhauser) 2006-05-11 04:13:25

Due to various reasons I can not run trunk, sorry. But for the example presentedby me (_X!, _9X!, exten=90), the sort order does not matter at all as both
matches are early matches (and pbx_find_extension() keeps on matching for early
matches in MATCHMORE mode).

As I understand it, NULL should be returned for MATCHMORE if the exten is
already long enough and no more digits are needed. Think of it this way: if it
is OK to return NULL for exten=90 and only the pattern "_9X!" is present, why
should it not be OK to do so if "_X!" is also present? The second ! pattern
encountered is handled in the else branch (because earlymatch is already set)
and there a value != NULL is returned. I'm pretty sure that the else branch
must never be entered for MATCHMORE and match == 2.

But I'm not so sure that it is OK to return NULL immediatly (your comment in thetrunk) when a MATCHMORE/early match is encountered. I think that depends on the
sort order of the patterns and the intented behaviour of "!":
1) match as early as possible, even if there are longer/more specific patterns
  that could match if there were more digits
2) match as soon as it sure that no longer/more specific pattern could match if
  there were more digits (i.e. same as "." but without digittimeout)
I always assumed that 2) is the case, e.g. with only the patterns
"_1!" and "_12!" and exten="1", wait for another digit or timeout so that
the approbiate pattern can be mached, but for exten="13" match "_1!" immediatly
and do not wait for another digit/timeout (as it would be the case for "_1.").

So if 2) is the case => it is OK to immediatly return NULL for
MATCHMORE/early-match if the sort order of the patterns guarantees that no
longer/more specific pattern could be encountered later on.

I think the main point is: indepentend of pattern sort order, it must be
guaranteed that the else branch is not entered for early matches in MATCHMORE

By: Luigi Rizzo (rizzo) 2006-05-15 01:55:05

mneuhauser: your bottom sententence is absolutely correct and the code has been fixed. Unless you have objections, i think this bug can be closed.

For the rest, we specified (and documented in pbx.c) a sorting order of extensions in the dialplan so that it was clear to the users what to expect from their dialplans.
I cannot comment on whether the way ! is specified (in relation to its use with MATCHMORE or CANMATCH) is useful or not because i am not familiar with how CANMATCH and MATCHMORE are used in the code.

By: Michael Neuhauser (mneuhauser) 2006-05-16 05:33:32

rizzo: closing the bug is OK with me

By: Luigi Rizzo (rizzo) 2006-05-16 05:41:07

issue resolved. it would be good to put in the 1.2 branch
the change (remove !earlymatch from the test)
proposed in the original description, which
is so trivial that doesn't require a disclaimer.